Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.2] Minor patch for the currentModal #39450

Merged
merged 3 commits into from
Jan 10, 2023
Merged

Conversation

dgrammatiko
Copy link
Contributor

Pull Request for Issue # .

Summary of Changes

  • The current code saves the open modal reference to Joomla.current instead of Joomla.Modal.current
  • The current code works as the Joomla.current is not referenced by any other API, but it shouldn't be used for the modals

Testing Instructions

  • Create a new article and test adding an intro and a full image, selecting a user for the article and try using the XTD-buttons (insert Article, Menu, Module, Media).
  • All modals open and close correctly

Actual result BEFORE applying this Pull Request

Works

Expected result AFTER applying this Pull Request

Works

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.2-dev labels Dec 19, 2022
@Fedik
Copy link
Member

Fedik commented Dec 19, 2022

Please update also the incorrect comment

* USAGE (assuming that exampleId is the modal id)
* To get the current modal element:
* Joomla.Modal.current; // Returns node element, eg: document.getElementById('exampleId')
* To set the current modal element:
* Joomla.Modal.setCurrent(document.getElementById('exampleId'));

It should be used Joomla.Modal.getCurrent().

@dgrammatiko
Copy link
Contributor Author

@Fedik done

@carlitorweb
Copy link
Member

Remember for the other tester use after apply the patch, npm run build:js

@carlitorweb
Copy link
Member

I have tested this item ✅ successfully on 385efc0


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

1 similar comment
@Quy
Copy link
Contributor

Quy commented Jan 9, 2023

I have tested this item ✅ successfully on 385efc0


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

@Quy
Copy link
Contributor

Quy commented Jan 9, 2023

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 9, 2023
@fancyFranci fancyFranci merged commit 3a9ba35 into joomla:4.2-dev Jan 10, 2023
@fancyFranci fancyFranci added this to the Joomla! 4.2.7 milestone Jan 10, 2023
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jan 10, 2023
@fancyFranci
Copy link
Contributor

Thanks for cleaning this up!

@dgrammatiko dgrammatiko deleted the patch-5 branch January 10, 2023 22:43
charvimehradu pushed a commit to charvimehradu/joomla-cms that referenced this pull request Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NPM Resource Changed This Pull Request can't be tested by Patchtester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants