Inserted shortcodes are placed inside a Paragraph tag

[jonom]

This is totally appropriate for shortcodes that render inline content but undesirable for others, as block elements such as divs are not allowed within paragraphs.

A config setting of $block_element (boolean) for implementors of ShortCodable might provide a way forward?

If set to true the shortcode could be inserted in a div instead of a paragraph adjacent to the currently selected paragraph (or replacing it if empty). Think the div would be necessary to prevent TinyMCE trying to wrap the text in a paragraph later.

Taking it a step further it would be cool if the div had a class of shortcode on it. This would potentially allow for the extraneous div to be stripped out later during parsing and could also allow for some basic styling of shortcodes to make them stand out in the editor.

[wernerkrauss]

afaik this is an tinymce issue, as it wraps all text in a

. Any idea how to overcome this without changing to interpret a linebreak as
?

[anselmdk]

There's a long discussion about this from 2012, but unfortunately it didn't go anywhere: https://groups.google.com/forum/#!topic/silverstripe-dev/_EXlGEUFHmU

[anselmdk]

...and here's a pull request trying to fix that - not sure whether there's an easy way to get around it: https://github.com/silverstripe/silverstripe-framework/pull/838

[anselmdk]

maybe this module could help: https://github.com/nyeholt/silverstripe-cleancontent

[tomstgeorge]

I managed to get rid of the unwanted p tags with the following in my Page.php

function onBeforeWrite() { $this->Content = str_replace('<p>[', '[', $this->Content); $this->Content = str_replace(']</p>', ']', $this->Content); parent::onBeforeWrite(); }

[sheadawson]

What do you all think of implementing @tomstgeorge's solution in the module? Could make it a configurable option

[jonom]

It would break inline shortcodes wouldn't it :/ especially if you have something like <p>[someshortcode] some text</p> as only the first replacement would happen.

Maybe it would be better to use some regex to verify that the shortcode is the only thing in a paragraph tag before removing it?

[tomstgeorge]

Someone smarter than me could probably come up with a regex to do this better - so that it wouldn't strip out a <p>[ or ]</p> that was actually wanted content. e.g. someone ending a paragraph with a footnote [1]

[tomstgeorge]

possibly it would also need a whitelist of shortcodes that you would want to remove the surrounding p tags from

[sheadawson]

Yeah ideally each shortcodable object would have a boolean config setting to strip or not. And someone would write us a nice regex to remove the tags only if other text not sitting along side the shortcode inside the tag

[jonom]

Something like this <p>(\[.*\])<\/p> Test: https://regex101.com/r/bFtD9o/1

[Friksel]

For the record, I have the same problem with <p> tags automatically being put around shortcodes. And Internet Explorer and W3C complain about it, because for <div> tags aren't allowed inside <p> tags and those <div>s automatically closes the added <p> tags causing unwanted empty lines... and so on. I tried a lot of things, but nothing really feels right or doesn't work at all. There should be some way to avoid this crazy behaviour, right?

[amanpilgrim]

While it would be nice to avoid wrapping shortcodes within a block level element, TinyMCE requires one. Using a div as the block level element within the editor avoids validation issues and regex hell.

You'll need to add div as an option for TinyMCE - this can be configured within _config.php by setting either 'theme_advanced_blockformats' or 'style_formats'

[Friksel]

@amanpilgrim I thought a minute about changing the roottag in tinyMCE to <div> before too, but replacing <p>s with <div>s is not the right thing to do in my opinion. Cause there happens to be a <p> tag for semantic reasons. It's got a meaning, other then the general <div>. So I'd rather stick with a regex then, wich isn't perfect, but a 'backstage' problem then messing up the frontend semantics. (Next to this all CSS needs to be changed too after switching)

[amanpilgrim]

I'm suggesting setting something along the lines of HtmlEditorConfig::get('cwp')->setOption('theme_advanced_blockformats', 'p,h2,h3,h4,div');

This doesn't prevent the regular use of semantic markup, it simply provides an alternative block option in the 'formatselect' dropdown for scenarios like this where you don't want to insert rich content within a <p> tag. I personally prefer the 'styleselect' option as it gives you a finer grain of control (block type, classes, etc)

[jonom]

If we added extension hooks to ShortCodeParser::parse() we could strip out extraneous <p> wrappers without breaking the TinyMCE UX.

If we had a boolean config for shortcodable objects like $is_block_element, before parsing we could look at the shortcodes, check their type, and strip wrapper paragraph tags if they're block elements. No need to replace with a div I think, the shortcode template should render appropriate markup.

This wouldn't affect what is actually saved in the database which is nice - there's a bit of abstraction there so you could change the $is_block_element setting on a whim without having to go back and change p's to div's or vice versa in all your saved HTMLText fields.

[jonom]

Actually, replacing with a div means we can preserve any attributes such as css classes that may have been added to the wrapper element. I don't know how useful that will be but you never know. I've got this working locally, will open a PR once I've tested a bit more. Will depend on https://github.com/silverstripe/silverstripe-framework/pull/6302.