-
Notifications
You must be signed in to change notification settings - Fork 12
Tabs: Keep track of descendant changes to determine if a tab panel has focusable elements #2888
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 15e1655 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
Size Change: +60 B (+0.06%) Total Size: 109 kB
ℹ️ View Unchanged
|
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (49ddfe1) and published all packages with changesets to npm. You can install the packages in ./dev/tools/deploy_wonder_blocks.js --tag="PR2888"Packages can also be installed manually by running: pnpm add @khanacademy/wonder-blocks-<package-name>@PR2888 |
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-zkpdojvfdo.chromatic.com/ Chromatic results:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## fix-popover-with-tabs #2888 +/- ##
=============================================
=============================================
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
…anel so that it can determine if it has focusable elements within it
…y set up the observer if the tab panel is active
…of descendant changes to determine if a tab panel has focusable elements
26fd6fd to
9a023f2
Compare
jandrade
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.
Looks great! This is a neat implementation! 🚀
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.
praise: This example is great to illustrate this point!
| /** | ||
| * When a tab panel has focusable elements, pressing `Tab` from the tablist | ||
| * will move focus to the first focusable element in the tab panel. If there | ||
| * are no focusable elements in the active tab panel, the tab panel will be | ||
| * focused instead. | ||
| */ |
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.
suggestion: It would be super helpful to explain this new behavior in the docs :)
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.
Updated, thanks for the suggestion!
…ing focusability when descendant elements change
Summary:
There was an issue reported by Level Access where a tabpanel had
tabindex=0in the Classroom Teacher Settings page, even though the tabpanel had focusable content. This was because the tab panel contents go from a loading state with no focusable elements, to UI with focusable elements.To address this, we set up a MutationObserver on the tab panel. When a tab panel is active, it will observe any changes in its subtree and re-determine if the tab panel contains focusable elements. This will make sure the tabindex is set properly depending on if there is or isn't focusable content in the panel
Issue: WB-2171
Test plan:
Storybook:
?path=/story/packages-tabs-tabs--with-focusable-contentBefore:
Screen.Recording.2025-12-03.at.4.51.15.PM.mov
After:
Screen.Recording.2025-12-03.at.4.49.50.PM.mov
Frontend znd using npm snapshot:
/teacher/class/<id>/settingsRegression testing:
/?path=/story/packages-tabs-tabs--with-focusable-content/?path=/docs/packages-tabs-tabs-testing-tabs-playtesting--docs