[#19789] - [4.0] Fix the bad implementation for the XTD-buttons on tinyMCE
- Fixed in Code Base
- 9 Mar 2018
- Medium
- Build: 4.0-dev
- # 19789
- Diff
- dgt41:JS-API/xtd-buttons
User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Summary of Changes
Without breaking the current API for the XTD-Buttons this PR is dropping the js that was passed in the json (a very bad practice for which I'll take the blame!). So instead of rendering a list of buttons it renders a dropdown with all the XTD-Buttons as links (with their icons). This can be improved by separating the core xt-buttons from the ones from 3rd PD for clarity and also to have more manageable dropdowns (this is a TODO, if people agree on the idea).
Also since we don't have the jModalClose() which we hacked to somehow work with bootstrap and since we are not gonna use the Bootstrap modal in J4 I propose to use the Joomla in this way:
Joomla.currentModal = {
get: function() {
// check if nested
return this.currentModal;
},
set: function(element) {
// check if nested
this.currentModal = element;
},
currentModal: '',
parentModal: ''
}So how this thing works?
When a modal is called the setter is also called and it, what else, sets the element of the current modal.
Now wherever in the code (especially if you're not aware of the modal element) you can call:
jQuery(Joomla.currentModal.get()).modal('hide');
// or using the J4 UI modal
Joomla.currentModal.get().close();Preview
Testing Instructions
Apply patch and test all the buttons in tinyMCE
Expected result
Actual result
Documentation Changes Required
NO
@Fedik @dneukirchen what do you think?
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Layout JavaScript External Library Front End Plugins |
What is the ordering of these?
I have no clue, but I agree this doesn't look nice
| Labels |
Added:
?
|
||
What is the ordering of these? It should default to be alphabetical imho
I have no clue, but I agree this doesn't look nice
It seems to be the ordering that plugins have in plugin manager when you filter via editors-xtd type
| Title |
|
||||||
I meant the part with modal and it`s id:
var modal = document.getElementById(name.name.replace(' ', '') + 'Modal');
They need to be unique for a11y as well
@Fedik the editors, not only tinyMCE will eventually become Custom Elements because frankly they are fields... So we can implement a lot more things once we do the transition. Basically this PR was done just as a POC and also to get some feedback on the needed API change for closing the current modal, which is something that we miss in J4 and thus a lot of scripts are failing...
| Category | Layout JavaScript External Library Front End Plugins | ⇒ | Administration Language & Strings Layout JavaScript External Library Front End Plugins |
Ok, multiple instances get modals with unique ids
So:
looks good but the icons will need to match the icons in the menu for those components ;)
I have tested this item
Notice: Undefined index: native in C:\htdocs\cms4\plugins\editors\tinymce\tinymce.php on line 531
Warning: array_merge(): Argument #2 is not an array in C:\htdocs\cms4\plugins\editors\tinymce\tinymce.php on line 531
Notice: Undefined index: paths in C:\htdocs\cms4\plugins\editors\tinymce\tinymce.php on line 534
Warning: array_merge(): Argument #2 is not an array in C:\htdocs\cms4\plugins\editors\tinymce\tinymce.php on line 534
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19789.
| Labels |
Added:
?
|
||
@brianteeman thanks, now should be fine
PS about the icons:
This is the before
this is the after
So in essence those icons are not changed. Maybe in the new skin that is proposed by the design team we can use the font awesome (or a part of it, don't forget that tiny is still in an iframe) to get some more meaningful icons
I have tested this item
no xtd-buttons at all now
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19789.
@brianteeman please clear your browser's cache
@brianteeman just curious here, did you used patchtester for this one?
Probably I don't remember
now that I have escaped scotland I will try again with git this week
must have been a patchtester issue as installing your fork works correctly as expected
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19789.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19789.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19789.
| Labels |
Added:
?
|
||
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-03-09 22:57:18 |
| Closed_By | ⇒ | wilsonge |
@regularlabs can you have a look on this one and provide some feedback?
I haven't tested this. But I can give you my opinion.
Placing all these editor button plugins in a separate dropdown feels a bit weird to me.
I would leave this to be controlled by the administrator. Let him/her decide if stuff needs to be grouped, and which buttons should get grouped.
Having stuff you use a lot and want there at-the-ready - like the read more button - is now hidden away in some undescriptive bucket full of random stuff that most likely have nothing to do with each other.
Also there are editor buttons that will show a dropdown/submenu thing themselves (at least one, Content Templater ;) ). So that will pose an issue for that extension.
Personally I was not very happy with the editor buttons getting forced into the editor in the first place. It caused all sorts of issues (styling, scripts, etc). I expect the same thing with this new approach.
I still prefer just using JCE (like everyone else) which shows the editor buttons under the editor (in Joomla 3 anyway), like we were use to.
But I am not very bothered about any of it.
As an extension developer I will just work my way around any new issues or workflows Joomla throws at me.
Based on the screenshot
What is the ordering of these? It should default to be alphabetical imho