User tests: Successful: Unsuccessful:
Pull Request for Issue # .
A redo of #14510
This a PR that fixes an old bad implementation of the theming for tinyMCE (I'm the one that did that)
In J3 to use a custom skin you had to upload it in the media/editors/tinymce/skins
That's utterly wrong. The skin is part of templating so the right path is templates/mytemplate/
Also the only ways to reach media/editors/tinymce/skins
are either ftp or ssh. By using the template we can use the editor in the template styles at the end the skin is just plain css so it belongs to the rest of our current css...
Go to node_modules/tinyMCE/skin
s and copy the folder oxide_dark
to administrator/templates/atum/css/vendor/tinymce/skins/ui
. Rename it or leave it as is.
Check it by editing an article.
Mild B/C break as we're removing a very idiomatic implementation of tinyMCE theming
This is not conflicting with #29257 That PR is adjusting the inner part of the editor
Status | New | ⇒ | Pending |
Category | ⇒ | Front End Plugins |
This unnecessarily removes the ability to select skin per set.
Yes this is right! the skin should be in sync with the template, it's a css, no need for different sets. Unless you like circus looking sites
Also it's not quite accurate you can still do whatever you want by overriding the tinyMCE options, a known way for getting other options your way...
Labels |
Added:
?
|
Category | Front End Plugins | ⇒ | Administration Language & Strings Front End Plugins |
Also remove "skin":"0","skin_admin":"0"
in base.sql in the installation folder.
Labels |
Added:
?
|
Category | Front End Plugins Administration Language & Strings | ⇒ | Administration Language & Strings SQL Installation Postgresql Front End Plugins |
I have tested this item
I have tested this item
I have tested this item
I have tested this item
@Quy please hold your test
@richard67 I think I have messed up the sql's here. Let me fix that
Edit: No the changes from NULL
to ' '
is what's in the current branch
@Quy this is fine you can proceed with the testing (thanks)
@dgrammatiko Looks ok now.
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
RTC
So I'm in favour of this change overall for overriding a skin at the template level rather than placing into the media directory.
However I think that probably we want to keep the options in the UI.
For two reasons - first of all we've lost the ability to use the oxide-dark default tiny theme. Which for dark skinned templates is probably just annoying that they would then have to keep things synched. Secondly I guess it allows different themings for the different levels (90% of the time probably not a major issue but when you're showing limited options to people I guess situationally could be useful)
Secondly I guess it allows different themings for the different levels
From a consistency and design integrity point of view people shouldn't roll different skins in a single template. The idea is that the skin is an integral part of the used template. But this is still possible, @Fedik already showcased how someone can override any (that also covers the skin) tinyMCE option. Personally I'm in favour of options that:
Which for dark skinned templates is probably just annoying that they would then have to keep things synched
I'm pretty sure (I had a little chat with Charlie on this one) proper front enders they will use http://skin.tiny.cloud/t5/ to provide a concrete skin that aligns with their templates colours, etc. So there isn't much to worry about here. (Their tool allows to import JSON data for the compilation not to mention that you can have the source and run it as a step of the rest of the building tools)
Anyways, that was my personal take here, if you insist on having back the options then let's get them back...
Status | Ready to Commit | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-07-29 18:18:46 |
Closed_By | ⇒ | dgrammatiko | |
Labels |
Added:
?
|
This unnecessarily removes the ability to select skin per set.