Fixed issue when escape key is pressed to close prompt

[konarshankar07]

Related Pull Requests

https://github.com/magento/partners-magento2b2b/pull/49

Description (*)

This PR will fix the issue with prompt. Currently, If escape key is pressed then prompt is not closing but instead of closing it try to submit the prompt

Fixed Issues (if relevant)

1. Pressing escape key confirms the save/license confirmation popup adobe-stock-integration#572: Pressing escape key confirms the save/license confirmation popup

Manual testing scenarios (*)

1. Login to admin panel
2. Open Magento Media Gallery (i.e. go to Cms -> Pages, edit the page and insert image)
3. Click "New Folder" button to open prompt
4. Enter Magento in the textbox to create folder
5. Press "Escape" key on the keyboard

Actual result

Trying to create new folder with magento

Expected result

Popup should be closed and new folder should not be created

Questions or comments

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @konarshankar07. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

[magento-engcom-team]

Hi @sivaschenko, thank you for the review.
ENGCOM-6284 has been created to process this Pull Request
✳️ @sivaschenko, could you please add one of the following labels to the Pull Request?

Label	Description
Auto-Tests: Covered	All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered	Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required	Changes in Pull Request does not require coverage by auto-tests

[engcom-Delta]

Hi @konarshankar07 During testing PR I found this issue: "New Folder Name" only hides behind "Media Gallery" popup if press "Escape" button but it is not closed

Could you take a look?

[konarshankar07]

Hello @engcom-Delta ,
Thanks for raising this issue. I have fixed the above mention issue. Please test and let me know your feedback

[engcom-Golf]

@magento run all tests

[engcom-Golf]

Related PR with B2B fixes https://github.com/magento/partners-magento2b2b/pull/49

[magento-engcom-team]

Hi @slavvka, thank you for the review.
ENGCOM-6284 has been created to process this Pull Request

[m2-assistant]

Hi @konarshankar07, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

[naydav]

@konarshankar07 @slavvka @sivaschenko
Hello guys,

Unfortunately, we need to revert these changes. In the scope of this PR we changed the base UI components ("modal", "modal/promt"). Thus it can lead to a huge impact on the current existed implementations based on this component.

We can accept some backward-incompatible changes but in this case, it is unreasonably (I mean not functionality but implementation way that leads to huge BIC).
We need to reimplement the fix in another way.

Thanks

[davemacaulay]

@naydav & others, this PR broke Page Builder's usage of the modal and prompt substantially. I agree this BiC change is maybe too severe as it has potential to break other consumers of these libraries.

[konarshankar07]

Hello @naydav ,
Sorry for the trouble caused by this PR but can you please explain in detail? So I can fix this issue in another PR. Honestly, I don't understand how this PR caused the issue after passing all the tests.
Thanks

[slavvka]

@konarshankar07 You changed the signature of methods in BIC way. Please look the guide https://devdocs.magento.com/guides/v2.3/contributor-guide/backward-compatible-development/. All that rules apply to all code (both PHP and JS). But unfortunately our SVC doesn't check that for JS files. As you could see that already made your fix incompatible with B2B (that required changes there as well). Also we have some extensions that aren't used to run tests - so after running the builds with them we discovered the issue with PageBuilder. As therefore this could be a huge issue for all other marketplace extensions that could use those methods. That is why we ask you to reimplement the fix in BC way according to the guide and create a new PR. This PR has been reverted.

[sivaschenko]

@naydav @slavvka @davemacaulay this pull request is fixing a major issue of popups functionality as well as eliminating the interface inconsistency. I'd recommend reviewing the possibility to include this fix in minor release (2.4)

[naydav]

@sivaschenko
We can make fix in compatible way and deliver it in 2.4