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,7 @@
{
"type": "patch",
"comment": "fix: add aria-controls to PopoverTrigger and name the Default story surface",
"packageName": "@fluentui/react-headless-components-preview",
"email": "vgenaev@gmail.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,36 @@ describe('Popover', () => {
});
});

describe('Focus on open', () => {
it('browser focuses a text-only surface on open', () => {
mount(
<Popover>
<PopoverTrigger>
<button data-testid="trigger">Trigger</button>
</PopoverTrigger>
<PopoverSurface data-testid="surface">Text only</PopoverSurface>
</Popover>,
);
cy.get('[data-testid=trigger]').focus().realPress('Enter');
cy.get('[data-testid=surface]').should('have.focus');
});

it('browser focuses the first focusable element inside the surface on open', () => {
mount(
<Popover>
<PopoverTrigger>
<button data-testid="trigger">Trigger</button>
</PopoverTrigger>
<PopoverSurface data-testid="surface">
<button data-testid="inner">Inner</button>
</PopoverSurface>
</Popover>,
);
cy.get('[data-testid=trigger]').focus().realPress('Enter');
cy.get('[data-testid=inner]').should('have.focus');
});
});

describe('Focus restoration', () => {
it('should restore focus to trigger on close', () => {
mount(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,25 @@ describe('Popover', () => {
expect(trigger).toHaveAttribute('aria-expanded', 'true');
});

it('sets aria-controls on trigger only while open', () => {
const { getByText, getByRole } = render(
<Popover id="my-id">
<PopoverTrigger>
<button>Trigger</button>
</PopoverTrigger>
<PopoverSurface>Surface</PopoverSurface>
</Popover>,
);

const trigger = getByText('Trigger');
expect(trigger).not.toHaveAttribute('aria-controls');

userEvent.click(trigger);

expect(trigger).toHaveAttribute('aria-controls', 'my-id');
expect(getByRole('group', { hidden: true })).toHaveAttribute('id', 'my-id');
});

it('sets data-open on trigger when open', () => {
const { getByText } = render(
<Popover defaultOpen>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export type PopoverProps = {
/**
* Optional id for the popover surface. When provided, this id is applied to
* the `PopoverSurface` element and referenced by `PopoverTrigger`
* (`aria-details`). If omitted, a generated id is used.
* (`aria-details` and `aria-controls`). If omitted, a generated id is used.
*/
id?: string;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type { PopoverTriggerChildProps as CanonicalPopoverTriggerChildProps } fr
type HeadlessPopoverTriggerChildExtras = {
'aria-haspopup'?: 'true';
'aria-details'?: string;
'aria-controls'?: string;
'data-open'?: string;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export const usePopoverTrigger = (props: PopoverTriggerProps): PopoverTriggerSta
'aria-expanded': `${open}` as 'true' | 'false',
'aria-haspopup': 'true' as const,
'aria-details': open ? surfaceId : undefined,
'aria-controls': open ? surfaceId : undefined,
'data-open': toDataAttributeValue(open),
...child?.props,
onMouseEnter: useEventCallback(mergeCallbacks(child?.props.onMouseEnter, onMouseEnter)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
### Do

- Create nested `Popover`s as separate components.
- If there are no interactive items in the `Popover` content, set `tabIndex={-1}` on the `PopoverSurface`.
- If there are no interactive items in the `Popover` content, the browser focuses the `PopoverSurface` itself on open. Give it an accessible name (`aria-label` or `aria-labelledby`) and point `aria-describedby` at the content, so screen readers announce the popover and its text.
- Use `Popover` to reduce screen clutter and host non-essential information.

### Don't
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,27 @@
import * as React from 'react';
import { useId } from '@fluentui/react-components';
import { Popover, PopoverTrigger, PopoverSurface } from '@fluentui/react-headless-components-preview/popover';

import styles from './popover.module.css';

export const Default = (): React.ReactNode => (
<Popover>
<PopoverTrigger>
<button className={styles.trigger}>Show popover</button>
</PopoverTrigger>
<PopoverSurface className={styles.surface}>
<h3 className={styles.heading}>Popover title</h3>
<p className={styles.body}>
This is the content of the popover. Click the trigger again or press Escape to close.
</p>
</PopoverSurface>
</Popover>
);
export const Default = (): React.ReactNode => {
const headingId = useId('popover-heading-');
const bodyId = useId('popover-body-');

return (
<Popover>
<PopoverTrigger>
<button className={styles.trigger}>Show popover</button>
</PopoverTrigger>
{/* Text-only content: the surface itself receives focus on open, so it needs a name and description. */}
<PopoverSurface className={styles.surface} aria-labelledby={headingId} aria-describedby={bodyId}>
<h3 id={headingId} className={styles.heading}>
Popover title
</h3>
<p id={bodyId} className={styles.body}>
This is the content of the popover. Click the trigger again or press Escape to close.
</p>
</PopoverSurface>
</Popover>
);
};