From d4bf05828f355197879be7cf12c3b9f8f38f968b Mon Sep 17 00:00:00 2001 From: PaulGMardling Date: Fri, 28 Aug 2026 11:04:54 +0200 Subject: [PATCH 1/2] fix(react-teaching-popover): move focus to page title on carousel step 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> --- ...-d7c78e6e-6bf1-499a-9c7c-7e5e3534747b.json | 7 +++ .../library/config/tests.cjs | 2 + .../Carousel/Carousel.tsx | 18 +++++- .../Carousel/constants.ts | 7 +++ .../TeachingPopoverCarousel.test.tsx | 60 ++++++++++++++++++- .../TeachingPopoverTitle.test.tsx.snap | 2 + .../useTeachingPopoverTitleBase.tsx | 5 ++ .../library/tsconfig.spec.json | 2 +- 8 files changed, 99 insertions(+), 4 deletions(-) create mode 100644 change/@fluentui-react-teaching-popover-d7c78e6e-6bf1-499a-9c7c-7e5e3534747b.json diff --git a/change/@fluentui-react-teaching-popover-d7c78e6e-6bf1-499a-9c7c-7e5e3534747b.json b/change/@fluentui-react-teaching-popover-d7c78e6e-6bf1-499a-9c7c-7e5e3534747b.json new file mode 100644 index 00000000000000..0e5f13db745c84 --- /dev/null +++ b/change/@fluentui-react-teaching-popover-d7c78e6e-6bf1-499a-9c7c-7e5e3534747b.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: move focus to the active page's TeachingPopoverTitle when a TeachingPopoverCarousel step changes, so assistive technology announces the new heading and step count in a single pass (fixes screen reader not announcing step changes on Next/Previous)", + "packageName": "@fluentui/react-teaching-popover", + "email": "paulmardling@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-components/react-teaching-popover/library/config/tests.cjs b/packages/react-components/react-teaching-popover/library/config/tests.cjs index 2e211ae9e21420..c6c67de97059e8 100644 --- a/packages/react-components/react-teaching-popover/library/config/tests.cjs +++ b/packages/react-components/react-teaching-popover/library/config/tests.cjs @@ -1 +1,3 @@ /** Jest test setup file. */ + +require('@testing-library/jest-dom'); diff --git a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/Carousel/Carousel.tsx b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/Carousel/Carousel.tsx index 9fa6bc54f4a129..a8aa975952409f 100644 --- a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/Carousel/Carousel.tsx +++ b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/Carousel/Carousel.tsx @@ -4,7 +4,7 @@ import * as React from 'react'; import { isHTMLElement, useMergedRefs, useControllableState, useEventCallback } from '@fluentui/react-utilities'; import { useAnnounce, useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts'; -import { CAROUSEL_ITEM } from './constants'; +import { CAROUSEL_ITEM, CAROUSEL_TITLE } from './constants'; import { useCarouselWalker_unstable } from './useCarouselWalker'; import { createCarouselStore } from './createCarouselStore'; import type { CarouselStore, UseCarouselOptions } from './Carousel.types'; @@ -80,7 +80,11 @@ export function useCarousel_unstable(options: UseCarouselOptions): { const callback: MutationCallback = mutationList => { for (const mutation of mutationList) { for (const addedNode of Array.from(mutation.addedNodes)) { - if (isHTMLElement(addedNode) && addedNode.hasAttribute(CAROUSEL_ITEM)) { + if (!isHTMLElement(addedNode)) { + continue; + } + + if (addedNode.hasAttribute(CAROUSEL_ITEM)) { const newValue = addedNode.getAttribute(CAROUSEL_ITEM)!; const newNode = carouselWalker.find(newValue); if (!newNode?.value) { @@ -90,6 +94,16 @@ export function useCarousel_unstable(options: UseCarouselOptions): { const previousNode = carouselWalker.prevPage(newNode?.value); store.insertValue(newValue, previousNode?.value ?? null); } + + // Move focus to the new page's title (if present) once it has actually mounted in the DOM, so + // assistive technology can announce the updated heading (and any aria-describedby'd step count) in a + // single pass, instead of relying solely on the `announcement` live region. Because this only runs for + // nodes added after the observer starts, the initial page's title is left untouched on mount. + const titleEl = addedNode.matches(`[${CAROUSEL_TITLE}]`) + ? addedNode + : addedNode.querySelector(`[${CAROUSEL_TITLE}]`); + + titleEl?.focus({ preventScroll: true }); } for (const removedNode of Array.from(mutation.removedNodes)) { diff --git a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/Carousel/constants.ts b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/Carousel/constants.ts index 068a09d0e3c1ff..576bed53266de9 100644 --- a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/Carousel/constants.ts +++ b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/Carousel/constants.ts @@ -1,2 +1,9 @@ export const CAROUSEL_ITEM = 'data-carousel-item'; export const CAROUSEL_ACTIVE_ITEM = 'data-carousel-active-item'; + +/** + * Marks the heading (TeachingPopoverTitle) belonging to a carousel page, so that focus can be moved to it + * when the active page changes. This lets assistive technology announce the new step's accessible name/role + * in a single pass, instead of relying solely on a separate live region announcement. + */ +export const CAROUSEL_TITLE = 'data-carousel-title'; diff --git a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/TeachingPopoverCarousel.test.tsx b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/TeachingPopoverCarousel.test.tsx index 8abb9be461ea5d..b87262d2b501e7 100644 --- a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/TeachingPopoverCarousel.test.tsx +++ b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/TeachingPopoverCarousel.test.tsx @@ -1,7 +1,10 @@ import * as React from 'react'; -import { render } from '@testing-library/react'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; import { isConformant } from '../../testing/isConformant'; import { TeachingPopoverCarousel } from './TeachingPopoverCarousel'; +import { TeachingPopoverCarouselCard } from '../TeachingPopoverCarouselCard/TeachingPopoverCarouselCard'; +import { TeachingPopoverCarouselFooter } from '../TeachingPopoverCarouselFooter/TeachingPopoverCarouselFooter'; +import { TeachingPopoverTitle } from '../TeachingPopoverTitle/TeachingPopoverTitle'; describe('TeachingPopoverCarousel', () => { isConformant({ @@ -21,4 +24,59 @@ describe('TeachingPopoverCarousel', () => { ); expect(result.container).toMatchSnapshot(); }); + + it('moves focus to the new page title when navigating to the next page', async () => { + render( + + + Step one + + + Step two + + + Footer + + , + ); + + fireEvent.click(screen.getByRole('button', { name: 'Next' })); + + await waitFor(() => expect(screen.getByText('Step two')).toHaveFocus()); + }); + + it('moves focus to the new page title when navigating to the previous page', async () => { + render( + + + Step one + + + Step two + + + Footer + + , + ); + + fireEvent.click(screen.getByRole('button', { name: 'Previous' })); + + await waitFor(() => expect(screen.getByText('Step one')).toHaveFocus()); + }); + + it('does not move focus to the title on initial render', async () => { + render( + + + Step one + + , + ); + + // Flush any pending microtasks (e.g. the carousel's mutation observer) before asserting. + await Promise.resolve(); + + expect(screen.getByText('Step one')).not.toHaveFocus(); + }); }); diff --git a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverTitle/__snapshots__/TeachingPopoverTitle.test.tsx.snap b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverTitle/__snapshots__/TeachingPopoverTitle.test.tsx.snap index a4ce6d3da671ea..dc79c5ced3f45d 100644 --- a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverTitle/__snapshots__/TeachingPopoverTitle.test.tsx.snap +++ b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverTitle/__snapshots__/TeachingPopoverTitle.test.tsx.snap @@ -4,6 +4,8 @@ exports[`TeachingPopoverTitle renders a default state 1`] = `

Default TeachingPopoverTitle

diff --git a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverTitle/useTeachingPopoverTitleBase.tsx b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverTitle/useTeachingPopoverTitleBase.tsx index d949ab831ee877..1336c6a1063ced 100644 --- a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverTitle/useTeachingPopoverTitleBase.tsx +++ b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverTitle/useTeachingPopoverTitleBase.tsx @@ -3,6 +3,7 @@ import type * as React from 'react'; import { usePopoverContext_unstable } from '@fluentui/react-popover'; import { getIntrinsicElementProps, slot, useEventCallback } from '@fluentui/react-utilities'; +import { CAROUSEL_TITLE } from '../TeachingPopoverCarousel/Carousel/constants'; import type { TeachingPopoverTitleBaseProps, TeachingPopoverTitleBaseState } from './TeachingPopoverTitle.types'; /** @@ -38,6 +39,10 @@ export const useTeachingPopoverTitleBase_unstable = ( root: slot.always( getIntrinsicElementProps('h2', { ref, + // Not in the tab sequence, but programmatically focusable so a TeachingPopoverCarousel can move focus + // here when the active page changes, letting assistive technology announce the new title in one pass. + tabIndex: -1, + [CAROUSEL_TITLE]: true, ...props, }), { elementType: 'h2' }, diff --git a/packages/react-components/react-teaching-popover/library/tsconfig.spec.json b/packages/react-components/react-teaching-popover/library/tsconfig.spec.json index 911456fe4b4d91..0e881941843de8 100644 --- a/packages/react-components/react-teaching-popover/library/tsconfig.spec.json +++ b/packages/react-components/react-teaching-popover/library/tsconfig.spec.json @@ -3,7 +3,7 @@ "compilerOptions": { "module": "CommonJS", "outDir": "dist", - "types": ["jest", "node"] + "types": ["jest", "node", "@testing-library/jest-dom"] }, "include": [ "**/*.spec.ts", From a3ab716ddbfe4b2d7bc84728dc2dabcef657363a Mon Sep 17 00:00:00 2001 From: PaulGMardling Date: Fri, 28 Aug 2026 11:52:13 +0200 Subject: [PATCH 2/2] fix(react-teaching-popover): scope carousel title focus to actual navigations 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> --- .../Carousel/Carousel.tsx | 43 +++++++++++++++---- .../TeachingPopoverCarousel.test.tsx | 31 +++++++++++++ 2 files changed, 65 insertions(+), 9 deletions(-) diff --git a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/Carousel/Carousel.tsx b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/Carousel/Carousel.tsx index a8aa975952409f..471b9ad359068f 100644 --- a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/Carousel/Carousel.tsx +++ b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/Carousel/Carousel.tsx @@ -40,6 +40,21 @@ export function useCarousel_unstable(options: UseCarouselOptions): { const { announce } = useAnnounce(); + // Tracks the value of a carousel page that is in the process of becoming active, so that focus is only moved + // to a page's title when its DOM node mounts *because of* a navigation - not whenever any + // `[data-carousel-title]` node happens to be added anywhere under the carousel (e.g. unrelated async content). + const pendingFocusValueRef = React.useRef(null); + const isInitialRenderRef = React.useRef(true); + + React.useEffect(() => { + if (isInitialRenderRef.current) { + isInitialRenderRef.current = false; + return; + } + + pendingFocusValueRef.current = value; + }, [value]); + if (process.env.NODE_ENV !== 'production') { // eslint-disable-next-line react-hooks/rules-of-hooks React.useEffect(() => { @@ -95,15 +110,25 @@ export function useCarousel_unstable(options: UseCarouselOptions): { store.insertValue(newValue, previousNode?.value ?? null); } - // Move focus to the new page's title (if present) once it has actually mounted in the DOM, so - // assistive technology can announce the updated heading (and any aria-describedby'd step count) in a - // single pass, instead of relying solely on the `announcement` live region. Because this only runs for - // nodes added after the observer starts, the initial page's title is left untouched on mount. - const titleEl = addedNode.matches(`[${CAROUSEL_TITLE}]`) - ? addedNode - : addedNode.querySelector(`[${CAROUSEL_TITLE}]`); - - titleEl?.focus({ preventScroll: true }); + // Move focus to a page's title only when it mounts as part of an actual navigation to it (tracked via + // `pendingFocusValueRef`), so assistive technology announces the updated heading (and any + // aria-describedby'd step count) in a single pass. A page's own root element (marked with + // `data-carousel-item`) is never removed/re-added on navigation - only its children toggle - so the + // title's *owning* item is resolved via the closest `[data-carousel-item]` ancestor and compared + // against the pending value. This ensures unrelated title mounts elsewhere in the carousel - e.g. async + // content added to a page that isn't the one just navigated to - never steal focus. + if (pendingFocusValueRef.current !== null) { + const titleEl = addedNode.matches(`[${CAROUSEL_TITLE}]`) + ? addedNode + : addedNode.querySelector(`[${CAROUSEL_TITLE}]`); + + const owningItemValue = titleEl?.closest(`[${CAROUSEL_ITEM}]`)?.getAttribute(CAROUSEL_ITEM); + + if (titleEl && owningItemValue === pendingFocusValueRef.current) { + titleEl.focus({ preventScroll: true }); + pendingFocusValueRef.current = null; + } + } } for (const removedNode of Array.from(mutation.removedNodes)) { diff --git a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/TeachingPopoverCarousel.test.tsx b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/TeachingPopoverCarousel.test.tsx index b87262d2b501e7..62f2fa5f58a391 100644 --- a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/TeachingPopoverCarousel.test.tsx +++ b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/TeachingPopoverCarousel.test.tsx @@ -79,4 +79,35 @@ describe('TeachingPopoverCarousel', () => { expect(screen.getByText('Step one')).not.toHaveFocus(); }); + + it('does not move focus when a title mounts on the active page outside of a navigation', async () => { + const AsyncTitle = () => { + const [loaded, setLoaded] = React.useState(false); + + React.useEffect(() => { + setLoaded(true); + }, []); + + return loaded ? Async step one : null; + }; + + render( + + + + + + , + ); + + const button = screen.getByRole('button', { name: 'Focus me' }); + button.focus(); + + // Wait for the async title to mount, plus any pending microtasks (e.g. the carousel's mutation observer). + await waitFor(() => expect(screen.getByText('Async step one')).toBeInTheDocument()); + await Promise.resolve(); + + expect(button).toHaveFocus(); + expect(screen.getByText('Async step one')).not.toHaveFocus(); + }); });