Skip to content

Conversation

@HenriqueLimas
Copy link
Member

  • This PR contains CSS changes
  • This PR does not contain CSS changes

Description

Add an observer on the dialog element that checks for nodes removal/addition and refresh the keyboard trap so it make sure the first and last focusable element are always up to date. This issue is visible when data fetching is executed inside the child component and the dialog component doesn't re-render.

Notes

The same issue is possible to happen in Marko, although data fetching on component level might not be common. @agliga let me know if you want me to implement the same on marko

Screenshots

Checklist

  • I verify the build is in a non-broken state
  • I verify all changes are within scope of the linked issue
  • I verify the markup will not be a breaking change (if not a major release)
  • I verify the MIND pattern for the component has been created/revised
  • I regenerated all CSS files under dist folder
  • I tested the UI in all supported browsers
  • I did a visual regression check of the components impacted by doing a Percy build and approved the build
  • I tested the UI in dark mode and RTL mode
  • I added/updated/removed Storybook coverage as appropriate

@HenriqueLimas HenriqueLimas self-assigned this Aug 12, 2025
Copilot AI review requested due to automatic review settings August 12, 2025 18:33
@HenriqueLimas HenriqueLimas added package: ebayui-core-react P3 Needs to be fixed relatively soon, but no rush labels Aug 12, 2025
@changeset-bot
Copy link

changeset-bot bot commented Aug 12, 2025

🦋 Changeset detected

Latest commit: 05109f5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@ebay/ui-core-react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

Copilot AI left a 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 keyboard trap functionality in the EbayLightboxDialog component where dynamic content changes weren't properly updating the focusable elements. The fix adds a MutationObserver to monitor DOM changes and refresh the keyboard trap accordingly.

Key changes:

  • Added MutationObserver to track child node changes in dialog elements
  • Refactored test story to better demonstrate the lazy loading scenario
  • Updated keyboard trap to refresh when DOM structure changes

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
packages/ebayui-core-react/src/ebay-dialog-base/components/dialogBase.tsx Added MutationObserver to monitor DOM changes and refresh keyboard trap when child nodes are added/removed
packages/ebayui-core-react/src/ebay-lightbox-dialog/__tests__/index.stories.tsx Refactored LazyContent story to use a separate component for cleaner demonstration of dynamic content loading
.changeset/plain-tigers-stay.md Added changeset entry documenting the keyboard trap fix

@agliga
Copy link
Collaborator

agliga commented Aug 12, 2025

@HenriqueLimas
I created a similar change on makeup side. Lets go with that and just upgrade makeup to the latest
makeup/makeup-js#215

@HenriqueLimas
Copy link
Member Author

Sounds good! I will close this one then! Thanks for adding it

@HenriqueLimas HenriqueLimas deleted the fix-dialog-focus-trap branch August 12, 2025 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Needs to be fixed relatively soon, but no rush package: ebayui-core-react

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants