Silverstripe-framework: [BUG] TinyMCE text alignment and image floating relies on "simple" theme to work (both in CMS and User Frontend)

[zzdjk6]

Affected Version

SS 4.3

Description

If simple theme is removed, the text alignment and image floating will not work on both CMS and user-frontend for HTMLText field (which is using TinyMCE).

The CSS that makes text alignment and image floating work is stored in themes/simple/css/typography.css.

Steps to Reproduce

1. Remove simple theme (from both file system and config)
2. Edit the HTML content of a page, try to use text align buttons to align text (e.g. right align), the text will not align properly (but the class has been added, this is different from SS 4.2.1 as the later add style directly instead of class) (notice: the class been added is text-right, which is not included in the simple theme too, so the TinyMCE alignment will not display the correct result even with simple theme installed)
3. Edit the HTML content of a page, try to insert an image and use right wrap, the image will not wrapped to the right
4. The page displays in user land will not work either.

My workaround

1. Add an CSS file like below and modify the config to make TinyMCE use that (modified from the simple theme)
2. Require this CSS file in the frontend page
3. In templates, add typography class in the container tag of corresponding content

Suggested Solution

Include the CSS in core

[kinglozzer]

I’m not sure that this is a bug, or even a missing feature. We purposely don’t provide styling for things like this out-the-box, otherwise developers have to override existing CSS or block core CSS files from loading. Anyone disagree with my assessment @silverstripe/core-team?

[zzdjk6]

@kinglozzer Well, maybe at least include a default "editor.css" in core to cover the basic function of TinyMCE editor in the CMS? (Not cover the user-side frontend is OK but need documentation)

Otherwise the TinyMCE editor for "HTMLText" field will not work properly if "simple" theme not installed and enabled. Then we have to include an "editor.css" for every future project to make the editor in CMS work.

The client catch this bug at the first glance in the CMS while I am demonstrating (the right alignment text doesn't align right in the CMS). It was so embarrassing and I had to say "there is at least a highlight in the toolbar"...

Please see the screenshot:

[maxime-rainville]

I'm with @kinglozzer. I think it's the developer responsibility to make sure their theme works with WYSIWYG classes. Probably worth documenting this somewhere thought and spell out what classes the WYSIWYG will add out of the box.

[kinglozzer]

Well, maybe at least include a default "editor.css" in core to cover the basic function of TinyMCE editor in the CMS?

That may be worth discussing. The only problem I can see is that while it might work well for simple things like text alignment, we also provide things like image captions, media embedding etc which tend to have more complex HTML/styling (which people will need to override). Maybe we could provide some basic styles (with minimal specificity CSS rules) for the CMS only, but offer a config flag to switch them off?

Either way, updating the WYSIWYG docs is a good idea!

[chillu]

I agree with @zzdjk6 that we shouldn't ship a product with broken functionality by default. We're in control and in charge of how the CMS transfers user actions into views it presents (e.g. right alignment). The developer is in charge on how that looks like on their own website. With that line of thinking, I'd recommend:

1. Add editor.css to core, include in CMS
2. Point to default editor.css everywhere that devs could look for this info: https://docs.silverstripe.org/en/4/developer_guides/customising_the_admin_interface/typography/, https://docs.silverstripe.org/en/4/developer_guides/templates/, https://docs.silverstripe.org/en/4/developer_guides/templates/themes/, https://docs.silverstripe.org/en/4/developer_guides/templates/requirements/, https://www.silverstripe.org/learn/lessons/v4/migrating-static-templates-into-your-theme-1

I don't think we should include an editor.css by default on the frontend, and force devs to hunt down where to disable that. We'll have to ensure that duplicate of editor.css doesn't get out of sync with our default themes, but there seems to be little changes in those files.

[clarkepaul]

Interesting idea @zzdjk6, most repeat SilverStripe devs know which base files/styles to copy over to their new theme but there will be those new to SilverStripe who will get quite confused with the basic set up.

[xini]

I'm with @chillu here, but instead of the CMS, I would include that css in the framework. The HTMLEditorField is in framework, so should the css.

[robbieaverill]

@xini yeah it'll get moved in SS5 though

[xini]

@robbieaverill fine. Then move the css then too. For now it's in framework.

[davejtoews]

As someone who's made a number of SilverStripe sites and just now realized I've shipped broken text editors I want to express my agreement with everyone here who says this shouldn't be broken out of the box.

If there's a feeling that imposing specific styles on the editor by default is too opinionated or imposing on developers who then need to override it, then the default TinyMCE config shouldn't include buttons that apply classes with no associated styles. If the styles are to be explicitly defined by the developer, these editor features should require an explicit opt-in as well.

There's a mis-match between providing an editor which applies classnames to markup, and not having any styles defined for them.

[sminnee]

When you shipped broken sites, was it just the CMS that was broken or was it the front-end too?

If we bundle a default editor.css I disagree that it shouldn't be included in the front end. Otherwise you'd have a situation where the alignment works on the CMS but is broken in the template, which seems like a footgun.

We'll also have to consider backwards collar though. Perhaps some config to inject a minimal default editor.css into both the CMS and into the front end, as two separate config statements, can be provided in the default yaml provided by the installer.

"Delete this line in the default config to DIY your styles" seems fair.

[davejtoews]

Broken on both, though the front end was less surprising to me than the CMS part.

I definitely see your point about being consistent with what's included in the CMS and the front end. However, I _expect_ to be responsible for front-end CSS but _don't expect_ to have to write admin/editor CSS unless I'm adding some custom feature. But yeah, inconsistency there is a potential pitfall too.

My complaint is inconsistency between features enabled in TinyMCE and having the styles needed to make those features work in the CMS. If those are made consistent I've got no complaint with also keeping the styles needed on the front-end consistent as well.

However since my front end markup may be different it seems like there's still a pretty good chance of some default front-end styles failing. If I didn't know enough to add in TinyMCE styles to my front end myself, I probably won't know to make sure the markup I output is wrapped in the .typography class.

The simplest way to keep things consistent between the classes the editor adds, the styles needed in the CMS, and the styles needed in the front end might very well be to have a default TinyMCE config that doesn't add any classes at all.

[sminnee]

I'm not sure that I would describe "hide the left/right align buttons by default" as a fix so much as a new bug ;-)

Whatever we do we should also review the relevant docs to make them harder to gloss over.

Your comment "I expect X but I don't expect Y" - the challenge we have is that everyone's expectations are different, so relying on this kind of intuition can be tricky.

But I agree that some changes are needed.

[davejtoews]

I'm not sure that hiding the left/right align buttons would be a bug, but I see what you mean, it would be hiding a commonly used feature. But having that feature missing might just be a good enough reason to check the docs page to find how to enable it, where of course you should also find instructions for making the styles work.

If it's a bug, it's at least a readily apparent bug, rather than something that's gonna fly under the radar as can happen in the current situation.

[jpbecotte]

I had to migrate a site that have been constructed in 2010 from 3.x to 4.x. Then client call me:
«

• I can't center text.
• What do you mean you can't center text?
• Well, I am in the admin panel, I click on the center button, and the text doesn't center in the editor.
• What???? It has been working for 10 years and now it is suddenly broken? Let me check.
  »

Then I fall on this thread. Waoooohh! Really? Some of you think there's no bug, here? Of course, everyone will get rid of the simple theme. They don't need it. They'll throw it away.

[michalkleiner]

And that's how we live in the SS community.

[chillu]

So many participants here, agreement on the issue, and yet I don't see a pull request for this relatively simple functional fix with some lightweight docs?