-
Notifications
You must be signed in to change notification settings - Fork 280
fix(OpenUI5Support): fix closing popups with escape key #12278
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an issue with closing popups when the escape key is pressed in a mixed environment of OpenUI5 and UI5 Web Components. The fix ensures proper event handling and prevents interference between OpenUI5 and Web Component popup escape key behavior.
Key changes:
- Added event marking utilities to prevent conflicts between popup systems
- Modified popup registry to check for marked events before handling escape
- Updated ComboBox escape key handling to be more selective about propagation
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/main/test/pages/DialogAndOpenUI5Dialog.html | Added comprehensive test scenarios with mixed OpenUI5 and Web Component popups |
| packages/main/src/popup-utils/OpenedPopupsRegistry.ts | Added event marking check to prevent handling already-processed escape events |
| packages/main/src/ComboBox.ts | Refined escape key handling to only stop propagation when value is reset |
| packages/main/cypress/specs/OpenUI5andWebCPopups.cy.tsx | Added Cypress tests for mixed popup scenarios and escape key behavior |
| packages/base/src/util/openui5support/eventMarking.ts | New utility for marking events to prevent duplicate handling |
| packages/base/src/features/patchPopup.ts | Added popup control patching to prevent escape handling when Web Components are above |
| packages/base/src/features/OpenUI5Support.ts | Updated to include Dialog and Popover controls in the patching process |
Comments suppressed due to low confidence (1)
packages/base/src/features/patchPopup.ts:1
- There's a mismatch between the parameter type and the property being patched. Line 96 should patch
PopupControl.prototype.onsapescapenotPopup.prototype.onsapescape.
// OpenUI5's Control.js subset
alexandar-mitsev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Open WebC Dialog -> Open UI5 With shortcut -> Open WebC Dialog. Then press ESC several times - not all dialogs are closed. Same happens with OpenUI5 dialog and WebC responsive popover. ESC does not close the openui5 dialog
- Open OpenUI5 Dilaog -> the webcomponents combobox does not work - the suggestions never appear, neither if you click the arrow, nor if you start writing. There is a related console error
The following are bugs reported in the Same BLI, but can be solved in different change:
- Open OpenUI5 Dilaog -> press "Tab" you can never reach the webcomponents content. Also initial focus skips all webcomponents and focuses the first openui5 control
- Open WebC Dialog with keyboard the select is directly opened when the dialog is opened
- OpenUI5 Dialog -> Open WebC PR with no intial focus -> keyboard focus can never reach the opened popover
-- This is an OpenUI5 Dialog issue. I reported it - SNOW: DINC0654974
-- Fixed in the test page
-- In OpenUI5 project Focusable util doesn't find focusable Web Components. We should decide how and where the fix should be.
-- sap.m.Select issue
-- this is by design. "no initial focus" functionality is used in ComboBox and the focus never goes to the popover |
alexandar-mitsev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Team Rila should review the changes in ComboBox
LGTM
Team Rila should review the changes in ComboBox
|
🎉 This PR is included in version v2.16.0-rc.0 🎉 The release is available on v2.16.0-rc.0 Your semantic-release bot 📦🚀 |
Fixes issues with closing popups when the escape key is pressed in a mixed environment of OpenUI5 and UI5 Web Components. The fix ensures proper event handling and prevents interference between OpenUI5 and Web Component popup escape key behavior.
JIRA: BGSOFUIRODOPI-3523