fix(OpenUI5Support): improve focus handling for OpenUI5 and Web Component popups #12230
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the closing behavior of UI5 popups by improving the popup registry system to properly distinguish between OpenUI5 and Web Component popups. The fix ensures proper focus event handling and popup management when multiple popup types are mixed.
Key changes:
- Enhanced popup registry to track popup types (OpenUI5 vs WebComponent) instead of just instances
- Improved focus event handling logic to determine when OpenUI5 popups should handle focus events
- Updated test page to better demonstrate the mixed popup scenarios
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/base/src/features/patchPopup.ts | Core logic changes to track popup types and implement proper focus event handling |
| packages/base/src/features/OpenUI5Support.ts | Updated method signature to accept PopupInfo objects |
| packages/main/src/popup-utils/OpenedPopupsRegistry.ts | Modified to use PopupInfo type and pass popup type information |
| packages/main/test/pages/DialogAndOpenUI5Dialog.html | Enhanced test page with shortcut functionality and corrected component usage |
TeodorTaushanov
changed the title
There was a problem hiding this comment.
Bugs:
- "Open WebC Dialog" -> press "openUI5 with shortcut button", press esc, press esc again - the webc dialog is not closed
- "Open Openui5 dialog" -> "Open WebC Responsive Popover", there are few issues:
- (from this change) the focus is not moved in the webc popover
- with keyboard the focus never reaches the popover
- esc sometimes closes the openui5 dialog first, before the webc popover
- if you put a select or combobox inside the openui5 dialog it is not reachable with keyboard and it acts strangely if operated with mouse
| // If the popup is the topmost one, we call the original focus event handler from the OpenUI5 Popup, | ||
| // otherwise the focus event is handled by the Web Component Popup. | ||
| if (this === getTopmostPopup()) { | ||
| if (shouldCallOpenUI5FocusEvent(this)) { |
There was a problem hiding this comment.
an alternative is to patch only the logic for topmost popup - see 54781ca and https://git.wdf.sap.corp/c/openui5/+/6794573
unfortunately most issues still remain, but seems better patching for long term solution as it directly fixes the initial problem
| @@ -21,13 +21,14 @@ | |||
| data-sap-ui-compatVersion="edge"></script> | |||
There was a problem hiding this comment.
we need more samples
- openui5 dialog with several ui5-webcomponents inputs like select, combobox and input
- webc dialog with several openui5 inputs
- openui5 popover with webc inputs
- webc popover with openui5 inputs
|
🎉 This PR is included in version v2.14.0-rc.7 🎉 The release is available on v2.14.0-rc.7 Your semantic-release bot 📦🚀 |
SNOW: DINC0613861