diff --git a/change/@fluentui-react-headless-components-preview-e2bcd363-f078-44e2-92ba-52ad44fe0b24.json b/change/@fluentui-react-headless-components-preview-e2bcd363-f078-44e2-92ba-52ad44fe0b24.json new file mode 100644 index 00000000000000..92a6097f46a56b --- /dev/null +++ b/change/@fluentui-react-headless-components-preview-e2bcd363-f078-44e2-92ba-52ad44fe0b24.json @@ -0,0 +1,6 @@ +{ + "type": "patch", + "comment": "fix: restore Popover and TeachingPopover trigger focus after controlled and uncontrolled close without stealing intentional outside focus", + "packageName": "@fluentui/react-headless-components-preview", + "email": "paulmardling@microsoft.com" +} diff --git a/change/@fluentui-react-teaching-popover-587162ef-6182-49c0-9199-d1466f307969.json b/change/@fluentui-react-teaching-popover-587162ef-6182-49c0-9199-d1466f307969.json new file mode 100644 index 00000000000000..44ab28f36735ef --- /dev/null +++ b/change/@fluentui-react-teaching-popover-587162ef-6182-49c0-9199-d1466f307969.json @@ -0,0 +1,6 @@ +{ + "type": "patch", + "comment": "fix: handle mount-time carousel focus and transfer focus after replacement navigation refs attach", + "packageName": "@fluentui/react-teaching-popover", + "email": "paulmardling@microsoft.com" +} diff --git a/change/@fluentui-react-teaching-popover-605d4544-c842-424e-b41f-d6e91ddbad69.json b/change/@fluentui-react-teaching-popover-605d4544-c842-424e-b41f-d6e91ddbad69.json new file mode 100644 index 00000000000000..5fd0def4e2a092 --- /dev/null +++ b/change/@fluentui-react-teaching-popover-605d4544-c842-424e-b41f-d6e91ddbad69.json @@ -0,0 +1,6 @@ +{ + "type": "patch", + "comment": "fix: move focus to the opposite carousel navigation button when null alternate text hides the focused button, including headless base hooks", + "packageName": "@fluentui/react-teaching-popover", + "email": "paulmardling@microsoft.com" +} diff --git a/packages/react-components/react-headless-components-preview/library/src/components/Popover/Popover.cy.tsx b/packages/react-components/react-headless-components-preview/library/src/components/Popover/Popover.cy.tsx index 1628efeaf2c91d..8a11ae6453d3cc 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/Popover/Popover.cy.tsx +++ b/packages/react-components/react-headless-components-preview/library/src/components/Popover/Popover.cy.tsx @@ -15,33 +15,6 @@ const mount = (element: JSXElement) => { const popoverTriggerSelector = '[aria-expanded]'; const popoverContentSelector = '[role="group"]'; -/** - * Marks a focus-restoration scenario as a known gap of the native - * `popover="auto"` model. - * - * Two distinct gaps put scenarios in this bucket: - * - * 1. Programmatic close: when React state flips `open: true → false`, the - * surface unmounts before any close-side effect can call `hidePopover()`, - * so the spec hide algorithm never runs and no focus restoration happens. - * The trailing pointer event from the close interaction (e.g. clicking a - * "Close" button) leaves focus on that button. - * - * 2. Hover and contextmenu opens: the spec hide algorithm restores focus to - * the element that was focused when `showPopover()` ran. Hover and - * contextmenu paths never move focus to the trigger before opening, so - * the snapshot points at whatever was focused before — usually `` — - * and restoration after close lands on the wrong element. - * - * Both gaps require a manual focus snapshot taken at intent-to-open and - * replayed when `open` transitions back to false. The tests below are kept - * as executable documentation; un-skipping them is the canary that the - * manual snapshot is in place. - */ -const itSkipUnsupportedFocusRestore = (description: string, fn: () => void): void => { - it.skip(description, fn); -}; - describe('Popover', () => { ['uncontrolled', 'controlled'].forEach(scenario => { const UncontrolledExample = () => ( @@ -241,8 +214,8 @@ describe('Popover', () => { }); }); - describe('Focus restore — unsupported scenarios', () => { - itSkipUnsupportedFocusRestore('programmatic close: should restore focus to trigger', () => { + describe('Focus restoration for controlled and non-click opens', () => { + it('programmatic close: preserves intentional focus on an outside control', () => { const Example = () => { const [open, setOpen] = React.useState(false); return ( @@ -264,10 +237,11 @@ describe('Popover', () => { cy.get('[data-testid=surface]').should('be.visible'); cy.get('[data-testid=close]').click(); cy.get('[data-testid=surface]').should('not.exist'); - cy.focused().should('have.attr', 'data-testid', 'trigger'); + cy.focused().should('have.attr', 'data-testid', 'close'); }); - itSkipUnsupportedFocusRestore('hover-leave close: should restore focus to trigger', () => { + // Keep the existing non-click-open scenarios skipped: native hover/dismissal timing is a separate gap. + it.skip('hover-leave close: should restore focus to trigger', () => { mount( @@ -283,7 +257,7 @@ describe('Popover', () => { cy.focused().should('have.attr', 'data-testid', 'trigger'); }); - itSkipUnsupportedFocusRestore('hover-open + Esc: should restore focus to trigger', () => { + it.skip('hover-open + Esc: should restore focus to trigger', () => { mount( @@ -298,7 +272,7 @@ describe('Popover', () => { cy.focused().should('have.attr', 'data-testid', 'trigger'); }); - itSkipUnsupportedFocusRestore('context-open + Esc: should restore focus to trigger', () => { + it.skip('context-open + Esc: should restore focus to trigger', () => { mount( diff --git a/packages/react-components/react-headless-components-preview/library/src/components/Popover/usePopover.ts b/packages/react-components/react-headless-components-preview/library/src/components/Popover/usePopover.ts index 0d4069cce4ff4b..ecc4a6d7463c40 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/Popover/usePopover.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/Popover/usePopover.ts @@ -2,6 +2,7 @@ import * as React from 'react'; import { useControllableState, useEventCallback, useId, useTimeout } from '@fluentui/react-utilities'; +import { useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts'; import { usePositioning, resolvePositioningShorthand } from '../../hooks'; import type { PopoverProps, PopoverState, PopoverContextValue, OpenPopoverEvents } from './Popover.types'; @@ -72,6 +73,17 @@ export const usePopover = (props: PopoverProps): PopoverState => { const triggerRef = React.useRef(null); const contentRef = React.useRef(null); const arrowRef = React.useRef(null); + const { targetDocument } = useFluent(); + const wasOpen = React.useRef(open); + + React.useEffect(() => { + // React removes the surface before the native hide algorithm can restore focus. + // Restore only after an actual close, without taking focus from an outside control. + if (wasOpen.current && !open && targetDocument?.activeElement === targetDocument?.body) { + triggerRef.current?.focus(); + } + wasOpen.current = open; + }, [open, targetDocument]); const generatedSurfaceId = useId('fui-popover-surface-'); const surfaceId = props.id ?? generatedSurfaceId; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/TeachingPopover/TeachingPopover.cy.tsx b/packages/react-components/react-headless-components-preview/library/src/components/TeachingPopover/TeachingPopover.cy.tsx index 286f46fcd0f3d7..6b28870bebc37a 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/TeachingPopover/TeachingPopover.cy.tsx +++ b/packages/react-components/react-headless-components-preview/library/src/components/TeachingPopover/TeachingPopover.cy.tsx @@ -8,6 +8,7 @@ import { TeachingPopoverCarouselCard, TeachingPopoverCarouselFooter, TeachingPopoverCarouselPageCount, + TeachingPopoverHeader, TeachingPopoverSurface, TeachingPopoverTitle, TeachingPopoverTrigger, @@ -20,7 +21,146 @@ const mount = (element: JSXElement) => mountBase(element); const triggerSelector = '[aria-expanded]'; const surfaceSelector = '[role="group"]'; +const FocusExample = ({ + controlled = false, + initiallyOpen = false, + withTrigger = true, + trapFocus = false, +}: { + controlled?: boolean; + initiallyOpen?: boolean; + withTrigger?: boolean; + trapFocus?: boolean; +}) => { + const [open, setOpen] = React.useState(initiallyOpen); + const [value, setValue] = React.useState('one'); + const surface = ( + + Tour + setValue(data.value)} + onFinish={() => { + setValue('one'); + setOpen(false); + }} + > + Feature Step 1 + Feature Step 2 + + + + ); + return ( + <> + + setOpen(data.open)} + > + {withTrigger + ? [ + + + , + surface, + ] + : surface} + + + ); +}; + describe('TeachingPopover', () => { + describe('focus management', () => { + [false, true].forEach(controlled => { + describe(controlled ? 'controlled' : 'uncontrolled', () => { + it('moves focus to Next when Previous becomes hidden on the first step', () => { + mount(); + cy.get('#next').realClick(); + cy.get('#previous').focus().realPress('Enter'); + cy.contains('Feature Step 1').should('be.visible'); + cy.get('#previous').should('not.be.visible'); + cy.get('#next').should('have.focus'); + }); + + [false, true].forEach(initiallyOpen => { + (['finish', 'escape', 'dismiss'] as const).forEach(action => { + it(`restores trigger focus on ${action} (initiallyOpen=${initiallyOpen})`, () => { + mount(); + if (!initiallyOpen) { + cy.get(triggerSelector).realClick(); + } + cy.get('#next').should('be.visible').focus(); + if (action === 'finish') { + cy.realPress('Enter'); + cy.get('#next').should('have.text', 'Got it').realPress('Enter'); + } else if (action === 'escape') { + cy.realPress('Escape'); + } else { + cy.get('[aria-label="dismiss"]').realClick(); + } + cy.get(surfaceSelector).should('not.exist'); + cy.get(triggerSelector).should('have.focus'); + }); + }); + }); + }); + }); + + it('restores the trigger after light dismissal of an initially open tour', () => { + mount(); + cy.get('#next').focus(); + cy.get('body').realClick({ position: 'bottomRight' }); + cy.get(surfaceSelector).should('not.exist'); + cy.get(triggerSelector).should('have.focus'); + }); + + it('preserves intentional outside focus on controlled close', () => { + mount(); + cy.get('#next').focus(); + cy.get('[data-testid="outside"]').realClick(); + cy.get(surfaceSelector).should('not.exist'); + cy.get('[data-testid="outside"]').should('have.focus'); + }); + + it('does not move focus on an initially closed mount', () => { + mount(); + cy.get(surfaceSelector).should('not.exist'); + cy.get(triggerSelector).should('not.have.focus'); + }); + + it('can finish an initially open triggerless tour', () => { + mount(); + cy.get('#next').realClick(); + cy.get('#next').realClick(); + cy.get(surfaceSelector).should('not.exist'); + cy.get('[data-testid="outside"]').should('not.have.focus'); + }); + + (['escape', 'dismiss'] as const).forEach(action => { + it(`restores trigger focus on modal ${action} when initially open`, () => { + mount(); + cy.get('[role="dialog"]').should('be.visible'); + if (action === 'escape') { + cy.realPress('Escape'); + } else { + cy.get('[aria-label="dismiss"]').realClick(); + } + cy.get('[role="dialog"]').should('not.exist'); + cy.get(triggerSelector).should('have.focus'); + }); + }); + }); + (['uncontrolled', 'controlled'] as const).forEach(scenario => { const UncontrolledExample = () => ( diff --git a/packages/react-components/react-headless-components-preview/library/src/components/TeachingPopover/TeachingPopover.test.tsx b/packages/react-components/react-headless-components-preview/library/src/components/TeachingPopover/TeachingPopover.test.tsx index 37063801bec7c2..0e6adfe66fe6d4 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/TeachingPopover/TeachingPopover.test.tsx +++ b/packages/react-components/react-headless-components-preview/library/src/components/TeachingPopover/TeachingPopover.test.tsx @@ -7,6 +7,9 @@ import { TeachingPopoverHeader } from './TeachingPopoverHeader'; import { TeachingPopoverTrigger } from './TeachingPopoverTrigger'; import { TeachingPopoverSurface } from './TeachingPopoverSurface'; import { TeachingPopoverTitle } from './TeachingPopoverTitle'; +import { TeachingPopoverCarousel } from './TeachingPopoverCarousel'; +import { TeachingPopoverCarouselCard } from './TeachingPopoverCarouselCard'; +import { TeachingPopoverCarouselFooter } from './TeachingPopoverCarouselFooter'; describe('TeachingPopover', () => { isConformant({ @@ -162,4 +165,46 @@ describe('TeachingPopover', () => { expect(getByText('Surface')).toBeInTheDocument(); expect(onOpenChange).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ open: true })); }); + + it('restores focus when a controlled open prop closes an initially open surface', () => { + const Example = ({ open }: { open: boolean }) => ( + + + + + + + + + ); + const { getByText, rerender } = render(); + getByText('Inner').focus(); + + rerender(); + + expect(getByText('Trigger')).toHaveFocus(); + }); + + it('does not restore focus when a consumer rejects a controlled close request', () => { + const onOpenChange = jest.fn(); + const { getByRole, getByText } = render( + + + + + + + First + + + + , + ); + + userEvent.click(getByRole('button', { name: 'Got it', hidden: true })); + + expect(onOpenChange).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ open: false })); + expect(getByRole('group', { hidden: true })).toBeInTheDocument(); + expect(getByText('Trigger')).not.toHaveFocus(); + }); }); diff --git a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopover/TeachingPopover.cy.tsx b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopover/TeachingPopover.cy.tsx index 3cef6c0d66bb15..3e1ce17b0b2d8a 100644 --- a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopover/TeachingPopover.cy.tsx +++ b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopover/TeachingPopover.cy.tsx @@ -10,6 +10,10 @@ import { TeachingPopoverSurface, TeachingPopoverBody, TeachingPopoverTitle, + TeachingPopoverCarousel, + TeachingPopoverCarouselCard, + TeachingPopoverCarouselFooter, + TeachingPopoverCarouselFooterButton, } from '@fluentui/react-teaching-popover'; import type { TeachingPopoverProps } from '@fluentui/react-teaching-popover'; import type { JSXElement } from '@fluentui/react-utilities'; @@ -23,6 +27,105 @@ const triggerSelector = '[aria-expanded]'; const surfaceSelector = '[role="dialog"]'; describe('TeachingPopover', () => { + (['autoFocus', 'callback ref'] as const).forEach(source => { + it(`tracks footer focus established by ${source} during mounting`, () => { + const onFocus = cy.stub().as('onFocus'); + mount( + + First + Second + button?.focus() : undefined} + onFocus={onFocus} + id="previous" + > + Previous + + + Next + + , + ); + + cy.get('#previous').should('have.focus'); + cy.get('@onFocus').should('have.been.calledOnce'); + cy.get('#previous').realPress('Enter'); + cy.get('#previous').should('not.be.visible'); + cy.get('#next').should('have.focus'); + }); + }); + + (['callback ref', 'key'] as const).forEach(change => { + it(`transfers focus after the destination footer button's ${change} changes`, () => { + const Example = () => { + const [hidden, setHidden] = React.useState(false); + const nextRef = React.useRef(null); + return ( + + First + Second + setHidden(true)} + id="previous" + > + Previous + + { + nextRef.current = button; + } + : nextRef + } + key={change === 'key' && hidden ? 'after' : 'before'} + id="next" + > + Next + + + ); + }; + mount(); + + cy.get('#previous').focus().realPress('Enter'); + cy.get('#previous').should('not.be.visible'); + cy.get('#next').should('have.focus'); + }); + }); + + it('focuses Next when returning to the first page hides Previous', () => { + mount( + + + + + + + First + Second + + + + , + ); + cy.get('#previous').focus().realPress('Enter'); + cy.get('#previous').should('not.be.visible'); + cy.get('#next').should('have.focus'); + }); + (['uncontrolled', 'controlled'] as const).forEach(scenario => { const UncontrolledExample = () => ( 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..ab5dc0ddebd943 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 @@ -22,6 +22,7 @@ export function useCarousel_unstable(options: UseCarouselOptions): { direction: 'next' | 'prev', ) => void; selectPageByValue: (event: React.MouseEvent, newValue: string) => void; + footerButtonRefs: NonNullable; }; } { const { announcement, onValueChange, onFinish } = options; @@ -37,6 +38,9 @@ export function useCarousel_unstable(options: UseCarouselOptions): { initialState: null, }); const rootRef = React.useRef(null); + const previousButtonRef = React.useRef(null); + const nextButtonRef = React.useRef(null); + const footerButtonRefs = React.useMemo(() => ({ prev: previousButtonRef, next: nextButtonRef }), []); const { announce } = useAnnounce(); @@ -150,6 +154,7 @@ export function useCarousel_unstable(options: UseCarouselOptions): { value, selectPageByDirection, selectPageByValue: updateSlide, + footerButtonRefs, }, }; } diff --git a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/Carousel/CarouselContext.ts b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/Carousel/CarouselContext.ts index d901922de331d3..8df7a4ffa03abe 100644 --- a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/Carousel/CarouselContext.ts +++ b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/Carousel/CarouselContext.ts @@ -15,6 +15,10 @@ export type CarouselContextValue = { direction: 'next' | 'prev', ) => void; selectPageByValue: (event: React.MouseEvent, value: string) => void; + footerButtonRefs?: { + prev: React.RefObject; + next: React.RefObject; + }; }; export const carouselContextDefaultValue: CarouselContextValue = { diff --git a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/useTeachingPopoverCarouselContextValues.ts b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/useTeachingPopoverCarouselContextValues.ts index a215206fe4ade9..37bec45f6fcf6b 100644 --- a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/useTeachingPopoverCarouselContextValues.ts +++ b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarousel/useTeachingPopoverCarouselContextValues.ts @@ -9,7 +9,7 @@ import type { export function useTeachingPopoverCarouselContextValues_unstable( state: TeachingPopoverCarouselState, ): TeachingPopoverCarouselContextValues { - const { store, value, selectPageByValue, selectPageByDirection } = state; + const { store, value, selectPageByValue, selectPageByDirection, footerButtonRefs } = state; const carousel = React.useMemo( () => ({ @@ -17,8 +17,9 @@ export function useTeachingPopoverCarouselContextValues_unstable( value, selectPageByDirection, selectPageByValue, + footerButtonRefs, }), - [store, value, selectPageByDirection, selectPageByValue], + [store, value, selectPageByDirection, selectPageByValue, footerButtonRefs], ); return { carousel }; diff --git a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarouselFooterButton/TeachingPopoverCarouselFooterButton.focus.test.tsx b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarouselFooterButton/TeachingPopoverCarouselFooterButton.focus.test.tsx new file mode 100644 index 00000000000000..a435eb5969df81 --- /dev/null +++ b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarouselFooterButton/TeachingPopoverCarouselFooterButton.focus.test.tsx @@ -0,0 +1,168 @@ +import * as React from 'react'; +import '@testing-library/jest-dom'; +import { render } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { TeachingPopoverCarousel } from '../TeachingPopoverCarousel/TeachingPopoverCarousel'; +import { TeachingPopoverCarouselCard } from '../TeachingPopoverCarouselCard/TeachingPopoverCarouselCard'; +import { TeachingPopoverCarouselFooterButton } from './TeachingPopoverCarouselFooterButton'; + +const Example = ({ + value, + previousAltText = null, + nextAltText = 'Got it', + previousRef, + nextRef, + nextKey, + autoFocus, + onFocus, + onBlur, +}: { + value?: string; + previousAltText?: React.ReactNode; + nextAltText?: React.ReactNode; + previousRef?: React.Ref; + nextRef?: React.Ref; + nextKey?: string; + autoFocus?: boolean; + onFocus?: React.FocusEventHandler; + onBlur?: React.FocusEventHandler; +}) => ( + + First + Second + + Previous + + + Next + + +); + +describe('TeachingPopoverCarouselFooterButton focus', () => { + it.each(['autoFocus', 'callback ref'])('tracks mount-time focus from %s and forwards focus handlers', source => { + const onFocus = jest.fn(); + const onBlur = jest.fn(); + const previousRef = (button: HTMLButtonElement | HTMLAnchorElement | null) => button?.focus(); + const { getByRole } = render( + , + ); + + expect(getByRole('button', { name: 'Previous' })).toHaveFocus(); + expect(onFocus).toHaveBeenCalledTimes(1); + + userEvent.click(getByRole('button', { name: 'Previous' })); + + expect(getByRole('button', { name: 'Next' })).toHaveFocus(); + expect(onBlur).toHaveBeenCalledTimes(1); + }); + + it.each(['callback ref', 'key'])('waits for the destination ref when its %s changes', change => { + const { getByRole, rerender } = render( + undefined} nextKey="before" />, + ); + getByRole('button', { name: 'Start' }).focus(); + const nextRef = jest.fn(); + + rerender(); + + const next = getByRole('button', { name: 'Next' }); + expect(nextRef).toHaveBeenLastCalledWith(next); + expect(next).toHaveFocus(); + }); + + it('preserves focus deliberately moved during destination ref attachment', () => { + const outsideRef = React.createRef(); + const { getByRole, rerender } = render( + <> + + + , + ); + getByRole('button', { name: 'Start' }).focus(); + + rerender( + <> + + { + if (button) { + outsideRef.current?.focus(); + } + }} + /> + , + ); + + expect(getByRole('button', { name: 'Outside' })).toHaveFocus(); + }); + + it('focuses Next when navigating back hides the focused Previous button', () => { + const { getByRole } = render(); + + userEvent.click(getByRole('button', { name: 'Previous' })); + + expect(getByRole('button', { name: 'Next' })).toHaveFocus(); + }); + + it('restores focus for a controlled page transition and forwards the button ref', () => { + const ref = React.createRef(); + const { getByRole, rerender } = render(); + expect(ref.current).toBe(getByRole('button', { name: 'Previous' })); + ref.current?.focus(); + + rerender(); + + expect(getByRole('button', { name: 'Next' })).toHaveFocus(); + expect(ref.current).toHaveAttribute('hidden'); + }); + + it('does not move focus when a controlled page change is rejected', () => { + const { getByRole } = render(); + const previous = getByRole('button', { name: 'Previous' }); + + userEvent.click(previous); + + expect(previous).toHaveFocus(); + }); + + it('does not move focus when Previous still has alternate content', () => { + const { getByRole } = render(); + + userEvent.click(getByRole('button', { name: 'Previous' })); + + expect(getByRole('button', { name: 'Start' })).toHaveFocus(); + }); + + it('does not move focus from another element on a controlled page change', () => { + const { getByRole, rerender } = render(); + const next = getByRole('button', { name: 'Got it' }); + next.focus(); + + rerender(); + + expect(getByRole('button', { name: 'Next' })).toHaveFocus(); + }); + + it('focuses Previous when Next becomes hidden on the last page', () => { + const { getByRole } = render(); + userEvent.click(getByRole('button', { name: 'Previous' })); + + userEvent.click(getByRole('button', { name: 'Next' })); + + expect(getByRole('button', { name: 'Previous' })).toHaveFocus(); + }); +}); diff --git a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarouselFooterButton/TeachingPopoverCarouselFooterButton.types.ts b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarouselFooterButton/TeachingPopoverCarouselFooterButton.types.ts index a2d3b8eb0ef386..bd84bdc32f3657 100644 --- a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarouselFooterButton/TeachingPopoverCarouselFooterButton.types.ts +++ b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarouselFooterButton/TeachingPopoverCarouselFooterButton.types.ts @@ -20,6 +20,8 @@ export type TeachingPopoverCarouselFooterButtonProps = ComponentProps, ): TeachingPopoverCarouselFooterButtonBaseState => { - const { navType, altText } = props; + const { navType, altText, onFocus, onBlur } = props; const selectPageByDirection = useCarouselContext_unstable(c => c.selectPageByDirection); const values = useCarouselValues_unstable(snapshot => snapshot); const activeValue = useCarouselContext_unstable(c => c.value); + const footerButtonRefs = useCarouselContext_unstable(c => c.footerButtonRefs); + const buttonRef = React.useRef(null); + const mergedRef = useMergedRefs(ref, buttonRef, footerButtonRefs?.[navType]); + const { targetDocument } = useFluent(); + const hasFocus = React.useRef(false); + const shouldTransferFocus = React.useRef(false); const handleClick = (event: React.MouseEvent) => { if (event.isDefaultPrevented()) { @@ -41,6 +54,20 @@ export const useTeachingPopoverCarouselFooterButtonBase_unstable = ( }; const handleButtonClick = useEventCallback(mergeCallbacks(handleClick, props.onClick)); + const handleFocus = React.useCallback( + (event: React.FocusEvent) => { + hasFocus.current = true; + onFocus?.(event); + }, + [onFocus], + ); + const handleBlur = React.useCallback( + (event: React.FocusEvent) => { + hasFocus.current = false; + onBlur?.(event); + }, + [onBlur], + ); const isTrailing = React.useMemo(() => { if (!activeValue) { @@ -60,21 +87,44 @@ export const useTeachingPopoverCarouselFooterButtonBase_unstable = ( buttonChild = altText; } + const hidden = isTrailing && (altText === null || altText === undefined); + useIsomorphicLayoutEffect(() => { + const activeElement = targetDocument?.activeElement; + shouldTransferFocus.current = + hidden && hasFocus.current && (activeElement === buttonRef.current || activeElement === targetDocument?.body); + }, [hidden, navType, footerButtonRefs, targetDocument]); + + // Sibling refs may still be detached during our layout effect. Wait for all refs to attach, + // then check that another component has not deliberately moved focus in the meantime. + React.useEffect(() => { + const transferFocus = shouldTransferFocus.current; + shouldTransferFocus.current = false; + const activeElement = targetDocument?.activeElement; + if (transferFocus && hidden && (activeElement === buttonRef.current || activeElement === targetDocument?.body)) { + footerButtonRefs?.[navType === 'prev' ? 'next' : 'prev'].current?.focus(); + } + }, [hidden, navType, footerButtonRefs, targetDocument]); + + const root = slot.always( + getIntrinsicElementProps('button', { + ...props, + ref: mergedRef, + hidden: hidden || props.hidden, + onClick: handleButtonClick, + children: buttonChild, + }), + { elementType: 'button' }, + ); + root.onFocus = handleFocus; + root.onBlur = handleBlur; + return { navType, altText, components: { root: 'button', }, - root: slot.always( - getIntrinsicElementProps('button', { - ref, - ...props, - onClick: handleButtonClick, - children: buttonChild, - }), - { elementType: 'button' }, - ), + root, }; }; diff --git a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarouselFooterButton/useTeachingPopoverCarouselFooterButtonStyles.styles.ts b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarouselFooterButton/useTeachingPopoverCarouselFooterButtonStyles.styles.ts index e08300ed06156e..a3c25dd6962562 100644 --- a/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarouselFooterButton/useTeachingPopoverCarouselFooterButtonStyles.styles.ts +++ b/packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarouselFooterButton/useTeachingPopoverCarouselFooterButtonStyles.styles.ts @@ -16,6 +16,9 @@ export const teachingPopoverCarouselFooterButtonClassNames: SlotClassNames