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..5d4525a6e64d5b --- /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 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/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..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,15 +1,27 @@ '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 } 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'; 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,6 +52,26 @@ export function useCarousel_unstable(options: UseCarouselOptions): { const { announce } = useAnnounce(); + const previousValueRef = React.useRef(value); + const focusRequestRef = React.useRef(null); + + // 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; + } + + 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 React.useEffect(() => { @@ -80,7 +112,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 +126,29 @@ export function useCarousel_unstable(options: UseCarouselOptions): { const previousNode = carouselWalker.prevPage(newNode?.value); store.insertValue(newValue, previousNode?.value ?? 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 === 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; + } + } } for (const removedNode of Array.from(mutation.removedNodes)) { @@ -112,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) => { @@ -137,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/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..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,7 +1,40 @@ import * as React from 'react'; -import { render } 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({ @@ -21,4 +54,368 @@ 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('keeps focus on the navigation tab when activating it', async () => { + render( + + + Step one + + + Step two + + + {value => } + + , + ); + + 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(); + }); + + 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",