[#33216] - [4.0] Xtd modals done right
- Fixed in Code Base
- 22 Apr 2021
- Medium
- Build: 4.0-dev
- # 33216
- Diff
- dgrammatiko:xtd_modals_done_right_4.0-dev
User tests: Successful: Unsuccessful:
Pull Request for Issue #33210 .
Summary of Changes
- Modals should not break when debug language is on
- Modals should have unique ids (since there is an option for multiple instances of an editor)
Testing Instructions
- Enable debug and the language debug and also the display (the 2 switches that appear after enabling debug)
- Switch between Tinymce, Codemirror and none editor in the Global configuration
- Create an article and test ALL the xtd-buttons
Actual result BEFORE applying this Pull Request
Broken
Expected result AFTER applying this Pull Request
Fixed
Documentation Changes Required
NO
~~Actually, yes! The PR #19789 from 2018 introduced the property realName which basically is the untranslated string for the button and this is a B/C break and although buttons without it should still work the J4 implementation of the buttons should dictate for it for a11y and also to ensure proper functionality of the multiple instances of a browser. ~~
Rework the code so it should be B/C although J4 XTD Buttons should have a property name and another icon to ensure there won't be accidental conflicts
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | JavaScript Repository NPM Change Layout Front End Plugins |
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33216.
| Labels |
Added:
NPM Resource Changed
?
|
||
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33216.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33216.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33216.
| Status | Pending | ⇒ | Ready to Commit |
rtc
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33216.
@dgrammatiko Is the "Documentation Changes Required" still up to date, i.e. it needs documentation changes? Or has that become obsolete after your recent changes? I ask because if it needs documentation changes, I have to set the corresponding label before merging, so it later won't be forgotten.
Or has that become obsolete after your recent changes?
It should be B/C unless I got totally wrong. Let me explain: I introduce here a new property icon but the fallback is the property name (as it was before): $icon = ($button->get('icon')) ? $button->get('icon') : $button->get('name');
That said, it should be communicated that ALL the J4 xtd-button SHOULD use these properties not only for the error reported with the debug language strings but mainly because the button MIGHT NOT work as expected when there are multiple instances of editors in a page (eg using custom fields)
So does it need doc changes or not? That's still not clear to me. A simple "yes" or "no" is sufficient for me, any answer will not block RTC.
So does it need doc changes or not?
Yes, If https://docs.joomla.org/Special:MyLanguage/Editors_form_field_type this is all the docs about the editor field someone should totally rewrite that and include also the xtd-buttons and the expected properties for the button.
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-04-22 17:00:49 |
| Closed_By | ⇒ | richard67 | |
| Labels |
Added:
?
?
|
||
Thanks!
Appveyor failure is not related to this PR.