Skip to content

Comments

[5.0] Use Joomla dialog for confirmation dialog, instead of old school confirm()#41434

Merged
HLeithner merged 17 commits intojoomla:5.0-devfrom
Fedik:toolbar-confirm-dialog
Sep 4, 2023
Merged

[5.0] Use Joomla dialog for confirmation dialog, instead of old school confirm()#41434
HLeithner merged 17 commits intojoomla:5.0-devfrom
Fedik:toolbar-confirm-dialog

Conversation

@Fedik
Copy link
Member

@Fedik Fedik commented Aug 23, 2023

Summary of Changes

Use Joomla dialog for confirmation dialog, instead of old school confirm()
Also an example how it could be used in other places.

Testing Instructions

Apply patch, run npm install.
Trash some Article, and then try empty the trash.

Actual result BEFORE applying this Pull Request

You get old school confirm() dialog

Expected result AFTER applying this Pull Request

You get Joomla dialog

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

Reference:

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.0-dev labels Aug 23, 2023
@Fedik Fedik added Feature Small A PR which only has a small change labels Aug 23, 2023
@HLeithner
Copy link
Member

image

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on ee1e199


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

@heelc29
Copy link
Contributor

heelc29 commented Aug 23, 2023

I have tested this item 🔴 unsuccessfully on ee1e199

Translation (JYES, JNO) not loaded for footer buttons
image


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

@heelc29
Copy link
Contributor

heelc29 commented Aug 23, 2023

system tests are failing
image

@Fedik
Copy link
Member Author

Fedik commented Aug 23, 2023

system tests are failing

Yeap, I did not checked, but I think cypress expecting old confirm()
Need to debug

@Fedik Fedik requested a review from chmst as a code owner August 24, 2023 10:24
@Fedik
Copy link
Member Author

Fedik commented Aug 24, 2023

Test can be fixed only after updating Firefox for Cypress

@HLeithner
Copy link
Member

Test can be fixed only after updating Firefox for Cypress

Test running now with

  • Cypress: 12.17.4
  • Browser: Firefox 114 (headless)
  • Node Version: v20.5.0

@Fedik
Copy link
Member Author

Fedik commented Aug 24, 2023

After removing failed tests it is working now.

@HLeithner HLeithner merged commit d7a0022 into joomla:5.0-dev Sep 4, 2023
@HLeithner
Copy link
Member

thanks

@HLeithner HLeithner added this to the Joomla! 5.0 milestone Sep 4, 2023
@HLeithner
Copy link
Member

I think we need a migration documentation?

@brianteeman
Copy link
Contributor

I think we need a migration documentation?

yes please as there are other places we can use this

@Fedik Fedik deleted the toolbar-confirm-dialog branch September 4, 2023 09:45
@Fedik
Copy link
Member Author

Fedik commented Sep 4, 2023

It is part of

Or need something more specific?

@heelc29 heelc29 mentioned this pull request Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature NPM Resource Changed This Pull Request can't be tested by Patchtester Small A PR which only has a small change Unit/System Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants