fix: react-popover compatibility with screen reader #28172

emmayjiang · 2023-06-07T18:30:48Z

Allows the popover created by react-popover to be read by screen readers despite having no focusable content.

fabricteam · 2023-06-07T18:36:31Z

📊 Bundle size report

Package & Exports	Baseline (minified/GZIP)	PR	Change
`react-components` react-components: Accordion, Button, FluentProvider, Image, Menu, Popover	`211.151 kB` `58.715 kB`	`211.165 kB` `58.722 kB`	`14 B` `7 B`
`react-infobutton` InfoButton	`132.502 kB` `40.405 kB`	`132.516 kB` `40.411 kB`	`14 B` `6 B`
`react-infobutton` InfoLabel	`135.967 kB` `41.466 kB`	`135.981 kB` `41.472 kB`	`14 B` `6 B`
`react-popover` Popover	`119.464 kB` `36.745 kB`	`119.478 kB` `36.751 kB`	`14 B` `6 B`

Unchanged fixtures

Package & Exports	Size (minified/GZIP)
`react-alert` Alert	`93.624 kB` `22.563 kB`
`react-avatar` Avatar	`57.872 kB` `15.115 kB`
`react-avatar` AvatarGroup	`15.682 kB` `6.306 kB`
`react-avatar` AvatarGroupItem	`74.048 kB` `19.605 kB`
`react-components` react-components: Button, FluentProvider & webLightTheme	`64.935 kB` `17.918 kB`
`react-components` react-components: FluentProvider & webLightTheme	`36.168 kB` `11.964 kB`
`react-datepicker-compat` DatePicker Compat	`222.522 kB` `59.185 kB`
`react-persona` Persona	`64.793 kB` `17.036 kB`
`react-portal-compat` PortalCompatProvider	`6.473 kB` `2.196 kB`
`react-table` DataGrid	`150.941 kB` `41.535 kB`
`react-table` Table (Primitives only)	`45.147 kB` `12.577 kB`
`react-table` Table as DataGrid	`133.392 kB` `34.008 kB`
`react-table` Table (Selection only)	`79.161 kB` `19.386 kB`
`react-table` Table (Sort only)	`78.491 kB` `19.195 kB`
`react-tags` Tag	`22.04 kB` `7.94 kB`

_{🤖 This report was generated against 517b807cd97ee89a470ac5e9dec48a6f42b69532}

smhigley · 2023-06-07T18:39:48Z

packages/react-components/react-popover/src/components/PopoverSurface/usePopoverSurface.ts

@@ -45,10 +46,13 @@ export const usePopoverSurface_unstable = (
      root: 'div',
    },
    root: getNativeElementProps('div', {
+      id,
+      'aria-labelledby': id,


ah sorry, I think I forgot to mention -- I think the id + aria-labelledby should go in the examples, rather than in the base component. The reason is that I think having a popover be labelled by all its content is too opinionated as a default. It works well for small popovers like our examples, but not for longer popovers (which should probably be labelled just by the title).

We'll add docs about how to properly label, but for now I think this bit should be in the examples

change/@fluentui-react-popover-54e46725-d634-43de-961b-a174b511fa9a.json

packages/react-components/react-popover/src/components/PopoverSurface/usePopoverSurface.ts

fabricteam · 2023-06-07T18:41:58Z

Perf Analysis (`@fluentui/react-components`)

Scenario	Render type	Master Ticks	PR Ticks	Iterations	Status
FluentProviderWithTheme	mount	81	86	10	Possible regression

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations	Status
Avatar	mount	588	580	5000
Button	mount	303	296	5000
Field	mount	1056	1025	5000
FluentProvider	mount	640	654	5000
FluentProviderWithTheme	mount	81	86	10	Possible regression
FluentProviderWithTheme	virtual-rerender	74	74	10
FluentProviderWithTheme	virtual-rerender-with-unmount	76	77	10
InfoButton	mount	17	13	5000
MakeStyles	mount	863	862	50000
Persona	mount	1592	1605	5000
SpinButton	mount	1238	1272	5000

size-auditor · 2023-06-07T18:46:49Z

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 517b807cd97ee89a470ac5e9dec48a6f42b69532 (build)

codesandbox-ci · 2023-06-07T18:48:11Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 061fc3e:

Sandbox	Source
@fluentui/react 8 starter	Configuration
@fluentui/react-components 9 starter	Configuration

fabricteam · 2023-06-07T19:18:09Z

🕵 fluentuiv9 No visual regressions between this PR and main

ling1726 · 2023-06-08T06:38:40Z

packages/react-components/react-popover/src/components/PopoverSurface/usePopoverSurface.ts

@@ -49,6 +49,7 @@ export const usePopoverSurface_unstable = (
      role: trapFocus ? 'dialog' : 'group',
      'aria-modal': trapFocus ? true : undefined,
      ...modalAttributes,
+      tabIndex: '-1',


in cases where popovers don't contain focusable elemnts, wouldn't users just be able to use inline?

There's nothing in this component that will actually focus this element if it's open. Unless the conclusion behind this PR is that we should be doing that?

@smhigley probably has more insight to share

@ling1726 the popover does actually get focus when tabIndex="-1" is present -- my assumption was that tabster was doing that 😅

PopoverSurface gets focus when tabIndex=-1 because of this useEffect

fluentui/packages/react-components/react-popover/src/components/Popover/usePopover.ts

Line 129 in 517b807

const firstFocusable = isNaN(containerTabIndex)

When the popover was built, we never expected the focusing effect to work when users don't put focusable elements inside popovers. Our guidance has always been to treat popovers as dialogs when this happens use trapFocus in that case.

@smhigley if popovers have no focusable content, would inline be enough? if we result to focusing the entire popover by default always, I don't see a reason to use inline

smhigley · 2023-06-12T19:23:21Z

/azp run

azure-pipelines · 2023-06-12T19:23:43Z

Azure Pipelines successfully started running 4 pipeline(s).

…1fa9a.json Co-authored-by: Esteban Munoz Facusse <esteban.230@hotmail.com>

emmayjiang requested a review from a team as a code owner June 7, 2023 18:30

msft-fluent-ui-bot assigned Hotell Jun 7, 2023

smhigley reviewed Jun 7, 2023

View reviewed changes

sopranopillow reviewed Jun 7, 2023

View reviewed changes

sopranopillow assigned emmayjiang and unassigned Hotell Jun 7, 2023

ling1726 reviewed Jun 8, 2023

View reviewed changes

emmayjiang and others added 4 commits June 13, 2023 09:18

popover popup screenreader fix

878016d

changefile

c06359a

fix stories

10db230

Update change/@fluentui-react-popover-54e46725-d634-43de-961b-a174b51…

061fc3e

…1fa9a.json Co-authored-by: Esteban Munoz Facusse <esteban.230@hotmail.com>

emmayjiang force-pushed the react-popover-screenreader-fix branch from 21b5773 to 061fc3e Compare June 13, 2023 16:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: react-popover compatibility with screen reader #28172

fix: react-popover compatibility with screen reader #28172

emmayjiang commented Jun 7, 2023

fabricteam commented Jun 7, 2023 •

edited

smhigley Jun 7, 2023

fabricteam commented Jun 7, 2023 •

edited

size-auditor bot commented Jun 7, 2023 •

edited

codesandbox-ci bot commented Jun 7, 2023 •

edited

fabricteam commented Jun 7, 2023 •

edited

ling1726 Jun 8, 2023

emmayjiang Jun 8, 2023

smhigley Jun 12, 2023

sopranopillow Jun 13, 2023 •

edited

ling1726 Jun 14, 2023

smhigley commented Jun 12, 2023

azure-pipelines bot commented Jun 12, 2023

fix: react-popover compatibility with screen reader #28172

Are you sure you want to change the base?

fix: react-popover compatibility with screen reader #28172

Conversation

emmayjiang commented Jun 7, 2023

fabricteam commented Jun 7, 2023 • edited

📊 Bundle size report

smhigley Jun 7, 2023

Choose a reason for hiding this comment

fabricteam commented Jun 7, 2023 • edited

Perf Analysis (@fluentui/react-components)

size-auditor bot commented Jun 7, 2023 • edited

Asset size changes

codesandbox-ci bot commented Jun 7, 2023 • edited

fabricteam commented Jun 7, 2023 • edited

🕵 fluentuiv9 No visual regressions between this PR and main

ling1726 Jun 8, 2023

Choose a reason for hiding this comment

emmayjiang Jun 8, 2023

Choose a reason for hiding this comment

smhigley Jun 12, 2023

Choose a reason for hiding this comment

sopranopillow Jun 13, 2023 • edited

Choose a reason for hiding this comment

ling1726 Jun 14, 2023

Choose a reason for hiding this comment

smhigley commented Jun 12, 2023

azure-pipelines bot commented Jun 12, 2023

fabricteam commented Jun 7, 2023 •

edited

fabricteam commented Jun 7, 2023 •

edited

Perf Analysis (`@fluentui/react-components`)

size-auditor bot commented Jun 7, 2023 •

edited

codesandbox-ci bot commented Jun 7, 2023 •

edited

fabricteam commented Jun 7, 2023 •

edited

sopranopillow Jun 13, 2023 •

edited