fix(react-teaching-popover): move focus to page title on carousel step change - #36643
Draft
Paul Mardling (PaulGMardling) wants to merge 2 commits into
Draft
Conversation
…p change Screen readers (Narrator/NVDA) did not reliably announce the new step's title/content when navigating a TeachingPopoverCarousel via Next/Previous, since the existing live-region announcement depends entirely on the consumer-supplied `announcement` callback. This adds the spec-preferred fix: every TeachingPopoverTitle now renders with tabIndex=-1 and a data-carousel-title marker, and the Carousel's existing MutationObserver moves focus to the new page's title once it mounts. This lets assistive technology announce the new heading directly, independent of the live-region text, without introducing new public props or cross-component context plumbing. Also adds @testing-library/jest-dom to the package's tsconfig.spec.json types (and requires it in the jest setup) so toHaveFocus() works both at runtime and under type-check, matching react-headless-components-preview. Fixes ADO #39651. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📊 Bundle size report
Unchanged fixtures
|
|
Pull request demo site: URL |
…igations Previously the MutationObserver moved focus to *any* [data-carousel-title] node added anywhere under the carousel, regardless of why it was added. Since TeachingPopoverTitle always carries that marker, a consumer rendering/async-loading a title anywhere in the carousel (e.g. content loaded after the active page already mounted) could unexpectedly steal focus, even though no Next/Previous navigation occurred. Track which page value is expected to become active (set when the carousel's value changes, cleared after use) and only move focus to a title when it belongs to that page - resolved via the closest [data-carousel-item] ancestor, since an item's root element persists in the DOM across navigation (only its children are added/removed). Adds a regression test covering a title mounting on the active page outside of a navigation, and confirms existing focus-on-navigation behavior still passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-actions[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Screen readers (Narrator/NVDA) did not reliably announce the new step's title/content when navigating a
TeachingPopoverCarouselvia Next/Previous. The existing accessibility mechanism relies entirely on a live-regionannounce()call driven by a consumer-suppliedannouncementcallback, which only receives the raw carousel item value (not the step index/total or title), so in practice announcements were often missing or unhelpful.This is the "spec preferred" fix referenced in the underlying a11y bug report: move focus to the new page's title on step change, so assistive technology announces the heading directly — independent of (and in addition to) any live-region announcement text.
What changed
TeachingPopoverTitlenow renders withtabIndex={-1}and adata-carousel-titlemarker by default (overridable via props).Carousel's existingMutationObserver(already used to track carousel item add/remove) now also detects when a[data-carousel-title]node mounts and calls.focus({ preventScroll: true })on it.Testing
yarn nx run react-teaching-popover:test— all 312 tests pass (23 suites), including 3 new tests.yarn nx run react-teaching-popover:lint— passes.yarn nx run react-teaching-popover:type-check— passes (added@testing-library/jest-domtotsconfig.spec.jsontypes sotoHaveFocus()type-checks, matchingreact-headless-components-preview).yarn nx run react-teaching-popover:build— passes.yarn nx run react-headless-components-preview:test— all 1032 tests pass (no regressions in the package that re-exports this one).Screen.Recording.2026-08-28.at.14.11.58.mov
Fixes
ADO #39651
Related PR for flutentui-modern #5636212