Skip to content

[4.2] Minor patch for the currentModal#39450

Merged
fancyFranci merged 3 commits intojoomla:4.2-devfrom
dgrammatiko:patch-5
Jan 10, 2023
Merged

[4.2] Minor patch for the currentModal#39450
fancyFranci merged 3 commits intojoomla:4.2-devfrom
dgrammatiko:patch-5

Conversation

@dgrammatiko
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

@Fedik done

@carlitorweb
Copy link
Copy Markdown
Member

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

@carlitorweb
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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