Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{

@github-actions github-actions Bot Sep 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🕵🏾‍♀️ visual changes to review in the Visual Change Report

vr-tests-react-components/Menu Converged - submenuIndicator slotted content 2 screenshots
Image Name Diff(in Pixels) Image Type
vr-tests-react-components/Menu Converged - submenuIndicator slotted content.default - RTL.submenus open.chromium.png 404 Changed
vr-tests-react-components/Menu Converged - submenuIndicator slotted content.default.submenus open.chromium.png 413 Changed
vr-tests-react-components/Positioning 2 screenshots
Image Name Diff(in Pixels) Image Type
vr-tests-react-components/Positioning.Positioning end.chromium.png 10 Changed
vr-tests-react-components/Positioning.Positioning end.updated 2 times.chromium.png 139 Changed
vr-tests-react-components/ProgressBar converged 3 screenshots
Image Name Diff(in Pixels) Image Type
vr-tests-react-components/ProgressBar converged.Indeterminate + thickness - Dark Mode.default.chromium.png 43 Changed
vr-tests-react-components/ProgressBar converged.Indeterminate + thickness - High Contrast.default.chromium.png 67 Changed
vr-tests-react-components/ProgressBar converged.Indeterminate + thickness.default.chromium.png 147 Changed
vr-tests-react-components/TagPicker 2 screenshots
Image Name Diff(in Pixels) Image Type
vr-tests-react-components/TagPicker.disabled - RTL.chromium.png 635 Changed
vr-tests-react-components/TagPicker.disabled.disabled input hover.chromium.png 677 Changed

There were 2 duplicate changes discarded. Check the build logs for more information.

"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"
}
Original file line number Diff line number Diff line change
@@ -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"
}
Original file line number Diff line number Diff line change
@@ -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"
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<body>` —
* 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 = () => (
Expand Down Expand Up @@ -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 (
Expand All @@ -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(
<Popover openOnHover mouseLeaveDelay={0}>
<PopoverTrigger disableButtonEnhancement>
Expand All @@ -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(
<Popover openOnHover>
<PopoverTrigger disableButtonEnhancement>
Expand All @@ -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(
<Popover openOnContext>
<PopoverTrigger disableButtonEnhancement>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -72,6 +73,17 @@ export const usePopover = (props: PopoverProps): PopoverState => {
const triggerRef = React.useRef<HTMLElement>(null);
const contentRef = React.useRef<HTMLElement>(null);
const arrowRef = React.useRef<HTMLDivElement>(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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
TeachingPopoverCarouselCard,
TeachingPopoverCarouselFooter,
TeachingPopoverCarouselPageCount,
TeachingPopoverHeader,
TeachingPopoverSurface,
TeachingPopoverTitle,
TeachingPopoverTrigger,
Expand All @@ -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<string | undefined>('one');
const surface = (
<TeachingPopoverSurface key="surface">
<TeachingPopoverHeader>Tour</TeachingPopoverHeader>
<TeachingPopoverCarousel
value={controlled ? value : undefined}
defaultValue={controlled ? undefined : 'one'}
onValueChange={(_, data) => setValue(data.value)}
onFinish={() => {
setValue('one');
setOpen(false);
}}
>
<TeachingPopoverCarouselCard value="one">Feature Step 1</TeachingPopoverCarouselCard>
<TeachingPopoverCarouselCard value="two">Feature Step 2</TeachingPopoverCarouselCard>
<TeachingPopoverCarouselFooter
previous={{ navType: 'prev', altText: null, children: 'Previous', id: 'previous' }}
next={{ navType: 'next', altText: 'Got it', children: 'Next', id: 'next' }}
/>
</TeachingPopoverCarousel>
</TeachingPopoverSurface>
);
return (
<>
<button data-testid="outside" onClick={() => setOpen(false)}>
Outside
</button>
<TeachingPopover
open={controlled ? open : undefined}
defaultOpen={controlled ? undefined : initiallyOpen}
trapFocus={trapFocus}
onOpenChange={(_, data) => setOpen(data.open)}
>
{withTrigger
? [
<TeachingPopoverTrigger key="trigger">
<button>Open tour</button>
</TeachingPopoverTrigger>,
surface,
]
: surface}
</TeachingPopover>
</>
);
};

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(<FocusExample controlled={controlled} initiallyOpen />);
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(<FocusExample controlled={controlled} initiallyOpen={initiallyOpen} />);
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(<FocusExample controlled initiallyOpen />);
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(<FocusExample controlled initiallyOpen />);
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(<FocusExample controlled />);
cy.get(surfaceSelector).should('not.exist');
cy.get(triggerSelector).should('not.have.focus');
});

it('can finish an initially open triggerless tour', () => {
mount(<FocusExample controlled initiallyOpen withTrigger={false} />);
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(<FocusExample controlled initiallyOpen trapFocus />);
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 = () => (
<TeachingPopover>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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 }) => (
<TeachingPopover open={open}>
<TeachingPopoverTrigger>
<button>Trigger</button>
</TeachingPopoverTrigger>
<TeachingPopoverSurface>
<button>Inner</button>
</TeachingPopoverSurface>
</TeachingPopover>
);
const { getByText, rerender } = render(<Example open />);
getByText('Inner').focus();

rerender(<Example open={false} />);

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(
<TeachingPopover open onOpenChange={onOpenChange}>
<TeachingPopoverTrigger>
<button>Trigger</button>
</TeachingPopoverTrigger>
<TeachingPopoverSurface>
<TeachingPopoverCarousel defaultValue="one">
<TeachingPopoverCarouselCard value="one">First</TeachingPopoverCarouselCard>
<TeachingPopoverCarouselFooter next={{ navType: 'next', altText: 'Got it', children: 'Next' }} />
</TeachingPopoverCarousel>
</TeachingPopoverSurface>
</TeachingPopover>,
);

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();
});
});
Loading
Loading