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..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 @@ -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'; @@ -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(() => { @@ -80,7 +95,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 +109,26 @@ export function useCarousel_unstable(options: UseCarouselOptions): { const previousNode = carouselWalker.prevPage(newNode?.value); store.insertValue(newValue, previousNode?.value ?? null); } + + // 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/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..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 @@ -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,90 @@ 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(); + }); + + 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(); + }); }); 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",