From d4bf05828f355197879be7cf12c3b9f8f38f968b Mon Sep 17 00:00:00 2001 From: PaulGMardling Date: Fri, 28 Aug 2026 11:04:54 +0200 Subject: [PATCH 1/3] 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/3] 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(); + }); }); From c8f9a1a46639efe3b763067e2114e6ca698ff349 Mon Sep 17 00:00:00 2001 From: PaulGMardling Date: Wed, 9 Sep 2026 16:10:56 +0200 Subject: [PATCH 3/3] fix(react-teaching-popover): preserve carousel focus ownership Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...-d7c78e6e-6bf1-499a-9c7c-7e5e3534747b.json | 2 +- .../Carousel/Carousel.tsx | 78 +++-- .../TeachingPopoverCarousel.test.tsx | 316 +++++++++++++++++- 3 files changed, 367 insertions(+), 29 deletions(-) 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 index 0e5f13db745c84..5d4525a6e64d5b 100644 --- a/change/@fluentui-react-teaching-popover-d7c78e6e-6bf1-499a-9c7c-7e5e3534747b.json +++ b/change/@fluentui-react-teaching-popover-d7c78e6e-6bf1-499a-9c7c-7e5e3534747b.json @@ -1,6 +1,6 @@ { "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)", + "comment": "fix: move focus to the active page's TeachingPopoverTitle after Next/Previous navigation while preserving navigation-tab focus and newer focus choices", "packageName": "@fluentui/react-teaching-popover", "email": "paulmardling@microsoft.com", "dependentChangeType": "patch" 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 471b9ad359068f..a04b9a75922cf4 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 @@ -1,7 +1,13 @@ 'use client'; import * as React from 'react'; -import { isHTMLElement, useMergedRefs, useControllableState, useEventCallback } from '@fluentui/react-utilities'; +import { + isHTMLElement, + useMergedRefs, + useControllableState, + useEventCallback, + useIsomorphicLayoutEffect, +} from '@fluentui/react-utilities'; import { useAnnounce, useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts'; import { CAROUSEL_ITEM, CAROUSEL_TITLE } from './constants'; @@ -10,6 +16,12 @@ import { createCarouselStore } from './createCarouselStore'; import type { CarouselStore, UseCarouselOptions } from './Carousel.types'; import type { CarouselContextValue } from './CarouselContext'; +type CarouselFocusRequest = { + requestedValue: string; + origin: Element | null; + committed: boolean; +}; + // TODO: Migrate this into an external @fluentui/carousel component // For now, we won't export this publicly, is only for internal TeachingPopover use until stabilized. export function useCarousel_unstable(options: UseCarouselOptions): { @@ -40,20 +52,25 @@ 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); + const previousValueRef = React.useRef(value); + const focusRequestRef = React.useRef(null); - React.useEffect(() => { - if (isInitialRenderRef.current) { - isInitialRenderRef.current = false; + // A controlled value does not carry the request that caused it, so delayed acceptance is recognized by matching + // the latest directional request while its captured focus origin still owns focus. Any different committed value, + // direct tab activation, or newer directional request supersedes it. + useIsomorphicLayoutEffect(() => { + if (previousValueRef.current === value) { return; } - pendingFocusValueRef.current = value; - }, [value]); + previousValueRef.current = value; + + if (focusRequestRef.current?.requestedValue === value) { + focusRequestRef.current.committed = true; + } else { + focusRequestRef.current = null; + } + }); if (process.env.NODE_ENV !== 'production') { // eslint-disable-next-line react-hooks/rules-of-hooks @@ -110,23 +127,26 @@ export function useCarousel_unstable(options: UseCarouselOptions): { 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 focusRequest = focusRequestRef.current; + + if (focusRequest?.committed) { 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; + if (titleEl && owningItemValue === focusRequest.requestedValue) { + const activeElement: Element | null = targetDocument?.activeElement ?? null; + const originWasRemoved = focusRequest.origin !== null && !focusRequest.origin.isConnected; + const requestStillOwnsFocus = + activeElement === focusRequest.origin || (originWasRemoved && activeElement === targetDocument?.body); + + if (requestStillOwnsFocus) { + titleEl.focus({ preventScroll: true }); + } + + focusRequestRef.current = null; } } } @@ -151,7 +171,7 @@ export function useCarousel_unstable(options: UseCarouselOptions): { return () => { observer.disconnect(); }; - }, [carouselWalker, store, win]); + }, [carouselWalker, store, targetDocument, win]); const updateSlide = useEventCallback( (event: React.MouseEvent, newValue: string) => { @@ -176,19 +196,29 @@ export function useCarousel_unstable(options: UseCarouselOptions): { direction === 'prev' ? carouselWalker.prevPage(active.value) : carouselWalker.nextPage(active.value); if (newPage) { + focusRequestRef.current = { + requestedValue: newPage.value, + origin: targetDocument?.activeElement ?? null, + committed: false, + }; updateSlide(event, newPage?.value); } else { onFinish?.(event, { event, type: 'click', value: active?.value }); } }); + const selectPageByValue: CarouselContextValue['selectPageByValue'] = useEventCallback((event, newValue) => { + focusRequestRef.current = null; + updateSlide(event, newValue); + }); + return { carouselRef: useMergedRefs(rootRef, carouselRef), carousel: { store, value, selectPageByDirection, - selectPageByValue: updateSlide, + selectPageByValue, }, }; } 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 62f2fa5f58a391..38f370495bb682 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,11 +1,41 @@ import * as React from 'react'; -import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { act, 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 { TeachingPopoverCarouselNav } from '../TeachingPopoverCarouselNav/TeachingPopoverCarouselNav'; +import { TeachingPopoverCarouselNavButton } from '../TeachingPopoverCarouselNavButton/TeachingPopoverCarouselNavButton'; import { TeachingPopoverTitle } from '../TeachingPopoverTitle/TeachingPopoverTitle'; +const DeferredSecondPage = (props: { + autoFocus?: boolean; + removeNavigationOrigin?: boolean; + revealTitleRef: React.RefObject<(() => void) | null>; +}) => { + const [showTitle, setShowTitle] = React.useState(false); + props.revealTitleRef.current = () => setShowTitle(true); + const footer = ( + + Footer + + ); + + return ( + + + Step one + {props.removeNavigationOrigin && footer} + + + {showTitle && Step two} + + + {!props.removeNavigationOrigin && footer} + + ); +}; + describe('TeachingPopoverCarousel', () => { isConformant({ Component: TeachingPopoverCarousel, @@ -65,18 +95,296 @@ describe('TeachingPopoverCarousel', () => { await waitFor(() => expect(screen.getByText('Step one')).toHaveFocus()); }); - it('does not move focus to the title on initial render', async () => { + it('keeps focus on the navigation tab when activating it', async () => { render( Step one + + Step two + + + {value => } + , ); - // Flush any pending microtasks (e.g. the carousel's mutation observer) before asserting. - await Promise.resolve(); + const stepTwoTab = screen.getByRole('tab', { name: 'Step two' }); + stepTwoTab.focus(); + fireEvent.click(stepTwoTab); + + await waitFor(() => expect(screen.getByText('Step two')).toBeVisible()); + expect(stepTwoTab).toHaveFocus(); + }); + + it('does not move focus to a deferred title after the user moves focus', async () => { + const revealTitleRef: React.RefObject<(() => void) | null> = { current: null }; + render(); + + const nextButton = screen.getByRole('button', { name: 'Next' }); + nextButton.focus(); + fireEvent.click(nextButton); + + const input = await screen.findByRole('textbox', { name: 'Step two input' }); + input.focus(); + expect(input).toHaveFocus(); + act(() => revealTitleRef.current?.()); + + await waitFor(() => expect(screen.getByText('Step two')).toBeVisible()); + expect(input).toHaveFocus(); + }); + + it('does not override autofocus when a deferred title mounts', async () => { + const revealTitleRef: React.RefObject<(() => void) | null> = { current: null }; + render(); + + const nextButton = screen.getByRole('button', { name: 'Next' }); + nextButton.focus(); + fireEvent.click(nextButton); + + const input = await screen.findByRole('textbox', { name: 'Step two input' }); + expect(input).toHaveFocus(); + act(() => revealTitleRef.current?.()); + + await waitFor(() => expect(screen.getByText('Step two')).toBeVisible()); + expect(input).toHaveFocus(); + }); + + it('moves focus to a deferred title when the navigation origin still has focus', async () => { + const revealTitleRef: React.RefObject<(() => void) | null> = { current: null }; + render(); + + const nextButton = screen.getByRole('button', { name: 'Next' }); + nextButton.focus(); + fireEvent.click(nextButton); + expect(nextButton).toHaveFocus(); + act(() => revealTitleRef.current?.()); + + await waitFor(() => expect(screen.getByText('Step two')).toHaveFocus()); + }); + + it('moves focus to a deferred title when the navigation origin was removed', async () => { + const revealTitleRef: React.RefObject<(() => void) | null> = { current: null }; + render(); + + const nextButton = screen.getByRole('button', { name: 'Next' }); + nextButton.focus(); + fireEvent.click(nextButton); + + await waitFor(() => expect(nextButton).not.toBeInTheDocument()); + expect(document.body).toHaveFocus(); + act(() => revealTitleRef.current?.()); + + await waitFor(() => expect(screen.getByText('Step two')).toHaveFocus()); + }); + + it('does not move focus when the controlled value changes directly', async () => { + const ControlledCarousel = () => { + const [value, setValue] = React.useState('one'); + + return ( + <> + + + + Step one + + + Step two + + + + ); + }; + + render(); + + const control = screen.getByRole('button', { name: 'Show step two' }); + control.focus(); + fireEvent.click(control); + + await waitFor(() => expect(screen.getByText('Step two')).toBeVisible()); + expect(control).toHaveFocus(); + }); + + it('does not reuse a rejected controlled navigation request for a later external change', async () => { + const ControlledCarousel = () => { + const [value, setValue] = React.useState('one'); + + return ( + <> + + + + Step one + + + Step two + + + Footer + + + + ); + }; + + render(); + + const nextButton = screen.getByRole('button', { name: 'Next' }); + nextButton.focus(); + fireEvent.click(nextButton); + expect(screen.getByText('Step one')).toBeVisible(); + + const control = screen.getByRole('button', { name: 'Show step two' }); + control.focus(); + fireEvent.click(control); + + await waitFor(() => expect(screen.getByText('Step two')).toBeVisible()); + expect(control).toHaveFocus(); + }); + + it('clears a pending directional focus request when the active navigation tab is selected', async () => { + let showStepTwo: (() => void) | undefined; + + const ControlledCarousel = () => { + const [value, setValue] = React.useState('one'); + showStepTwo = () => setValue('two'); + + return ( + + + Step one + + + Step two + + + {pageValue => } + + + Footer + + + ); + }; + + render(); + + const nextButton = screen.getByRole('button', { name: 'Next' }); + nextButton.focus(); + fireEvent.click(nextButton); + + const activeTab = screen.getByRole('tab', { name: 'Step one' }); + activeTab.focus(); + fireEvent.click(activeTab); + act(() => showStepTwo?.()); + + await waitFor(() => expect(screen.getByText('Step two')).toBeVisible()); + expect(activeTab).toHaveFocus(); + }); + + it('moves focus to the title after directional navigation in controlled mode', async () => { + const ControlledCarousel = () => { + const [value, setValue] = React.useState('one'); + + return ( + setValue(data.value!)}> + + Step one + + + Step two + + + Footer + + + ); + }; + + render(); + + fireEvent.click(screen.getByRole('button', { name: 'Next' })); + + await waitFor(() => expect(screen.getByText('Step two')).toHaveFocus()); + }); + + it('supports delayed controlled acceptance while the navigation origin retains focus', async () => { + let acceptNavigation: (() => void) | undefined; + + const ControlledCarousel = () => { + const [value, setValue] = React.useState('one'); + + return ( + { + acceptNavigation = () => setValue(data.value!); + }} + > + + Step one + + + Step two + + + Footer + + + ); + }; + + render(); + + const nextButton = screen.getByRole('button', { name: 'Next' }); + nextButton.focus(); + fireEvent.click(nextButton); + expect(screen.getByText('Step one')).toBeVisible(); + + act(() => acceptNavigation?.()); + + await waitFor(() => expect(screen.getByText('Step two')).toHaveFocus()); + }); + + it('does not move focus to a deferred initial title in StrictMode', async () => { + let revealInitialTitle: (() => void) | undefined; + + const StrictModeCarousel = () => { + const [showTitle, setShowTitle] = React.useState(false); + revealInitialTitle = () => setShowTitle(true); + + return ( + + + {showTitle && Step one} + + + ); + }; + + render( + + + , + ); + + expect(document.body).toHaveFocus(); + act(() => revealInitialTitle?.()); + await waitFor(() => expect(screen.getByText('Step one')).toBeVisible()); + expect(document.body).toHaveFocus(); expect(screen.getByText('Step one')).not.toHaveFocus(); });