? NPM Resource Changed ? ? Pending

User tests: Successful: Unsuccessful:

avatar dgrammatiko
dgrammatiko
21 Apr 2021

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

avatar dgrammatiko dgrammatiko - open - 21 Apr 2021
avatar dgrammatiko dgrammatiko - change - 21 Apr 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 21 Apr 2021
Category JavaScript Repository NPM Change Layout Front End Plugins
avatar richard67
richard67 - comment - 21 Apr 2021

Appveyor failure is not related to this PR.

avatar infograf768 infograf768 - test_item - 21 Apr 2021 - Tested successfully
avatar infograf768
infograf768 - comment - 21 Apr 2021

I have tested this item successfully on e65da44


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33216.

avatar dgrammatiko dgrammatiko - change - 21 Apr 2021
The description was changed
avatar dgrammatiko dgrammatiko - edited - 21 Apr 2021
avatar dgrammatiko dgrammatiko - change - 21 Apr 2021
Labels Added: NPM Resource Changed ?
9096add 21 Apr 2021 avatar dgrammatiko oops
avatar dgrammatiko dgrammatiko - change - 21 Apr 2021
The description was changed
avatar dgrammatiko dgrammatiko - edited - 21 Apr 2021
avatar infograf768 infograf768 - test_item - 22 Apr 2021 - Tested successfully
avatar infograf768
infograf768 - comment - 22 Apr 2021

I have tested this item successfully on 55e6ca9


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33216.

avatar Quy Quy - test_item - 22 Apr 2021 - Tested successfully
avatar Quy
Quy - comment - 22 Apr 2021

I have tested this item successfully on 3c89ba0


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33216.

avatar infograf768 infograf768 - test_item - 22 Apr 2021 - Tested successfully
avatar infograf768
infograf768 - comment - 22 Apr 2021

I have tested this item successfully on 3c89ba0


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33216.

avatar infograf768 infograf768 - change - 22 Apr 2021
Status Pending Ready to Commit
avatar infograf768
infograf768 - comment - 22 Apr 2021

rtc


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33216.

avatar richard67
richard67 - comment - 22 Apr 2021

@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.

avatar dgrammatiko
dgrammatiko - comment - 22 Apr 2021

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)

avatar richard67
richard67 - comment - 22 Apr 2021

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.

avatar dgrammatiko
dgrammatiko - comment - 22 Apr 2021

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.

avatar richard67 richard67 - close - 22 Apr 2021
avatar richard67 richard67 - merge - 22 Apr 2021
avatar richard67 richard67 - change - 22 Apr 2021
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: ? ?
avatar richard67
richard67 - comment - 22 Apr 2021

Thanks!

Add a Comment

Login with GitHub to post a comment