Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Table and DataGrid should not remove cells from the accessibility tree #31068

Merged
merged 5 commits into from
Apr 19, 2024
Merged

fix: Table and DataGrid should not remove cells from the accessibility tree #31068

merged 5 commits into from
Apr 19, 2024

Conversation

smhigley
Copy link
Contributor

Previous Behavior

The header cell for the radio selection column was removed from the a11y tree via two separate mechanisms:

  1. The HTML hidden attribute was applied to the cell root, because of the Fluent hidden prop of the same name
  2. A visibility: hidden style was applied based on the hidden state

Removing a cell from a table breaks the table structure, as well as screen reader table navigation, which resulted in a high-severity a11y bug.

New Behavior

While we could have made the Fluent hidden prop not apply that HTML hidden prop, that would also have meant that authors who intended to add the HTML hidden prop (e.g. to hide an entire column, and not just the header) would have been unable to.

Instead, I deprecated hidden and added an invisible prop to do the same thing.

Related Issue(s)

@smhigley smhigley requested a review from a team as a code owner April 16, 2024 17:55
@github-actions github-actions bot added this to the April Project Cycle Q1 2024 milestone Apr 16, 2024
@codesandbox-ci CodeSandbox CI
Copy link

codesandbox-ci bot commented Apr 16, 2024 •
edited
Loading

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.

@fabricteam
Copy link
Collaborator

fabricteam commented Apr 16, 2024 •
edited
Loading

Perf Analysis (@fluentui/react-components)

Scenario Render type Master Ticks PR Ticks Iterations Status
FluentProviderWithTheme virtual-rerender 37 36 10 Possible regression
FluentProviderWithTheme virtual-rerender-with-unmount 66 78 10 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 632 654 5000
Button mount 301 307 5000
Field mount 1145 1133 5000
FluentProvider mount 700 717 5000
FluentProviderWithTheme mount 77 78 10
FluentProviderWithTheme virtual-rerender 37 36 10 Possible regression
FluentProviderWithTheme virtual-rerender-with-unmount 66 78 10 Possible regression
MakeStyles mount 852 879 50000
Persona mount 1779 1771 5000
SpinButton mount 1422 1408 5000
SwatchPicker mount 1507 1540 5000

@fabricteam
Copy link
Collaborator

fabricteam commented Apr 16, 2024 •
edited
Loading

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-components
react-components: entire library		 1.056 MB
257.506 kB		 1.056 MB
257.496 kB		 10 B
-10 B
react-table
DataGrid		 168.498 kB
46.732 kB		 168.508 kB
46.729 kB		 10 B
-3 B
react-table
Table as DataGrid		 137.918 kB
37.149 kB		 137.912 kB
37.14 kB		 -6 B
-9 B
react-table
Table (Selection only)		 76.334 kB
20.553 kB		 76.328 kB
20.545 kB		 -6 B
-8 B
react-table
Table (Sort only)		 74.977 kB
20.155 kB		 74.971 kB
20.148 kB		 -6 B
-7 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Button, FluentProvider & webLightTheme		 71.104 kB
20.52 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover		 221.098 kB
62.406 kB
react-components
react-components: FluentProvider & webLightTheme		 43.591 kB
14.356 kB
react-portal-compat
PortalCompatProvider		 7.944 kB
2.588 kB
react-table
Table (Primitives only)		 45.324 kB
14.116 kB
🤖 This report was generated against 7181c431a13178dafbf09ad3cbc4ab7c9740bbcf

@fabricteam
Copy link
Collaborator

fabricteam commented Apr 16, 2024 •
edited
Loading

🕵 fluentuiv9 No visual regressions between this PR and main

@smhigley smhigley merged commit e9fcf5b into microsoft:master Apr 19, 2024
19 checks passed
@smhigley smhigley deleted the table-hidden-cell branch April 19, 2024 19:16
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Apr 23, 2024
@marcosmoura
Merge branch 'master' into feat/react-drawer/move-scroll-to-contex
456e2bf 
* master: (416 commits)
  fix: remove relative imports within stories which are invalid for creating 'show docs' and circular dep imports from cypress files (microsoft#31087)
  Fix overlapping bars on continuous axes (microsoft#31035)
  feat(scripts-tasks): implement ~36% faster type-check task (microsoft#31116)
  feat(workspace-plugin): implement split-library-in-two migration generator (microsoft#31086)
  applying package updates
  Sankey diagram: Support number formatting (microsoft#31113)
  Fix wrong position of hover callout in case of single data AreaChart (microsoft#30256)
  Combobox filtering bug fix (microsoft#31141)
  chore(react-tag-picker): adds text elliptical clipping example (microsoft#31114)
  docs: command cheat sheet (microsoft#30685)
  chore: refactor tests for createPresenceComponent() (microsoft#31137)
  Stable Release: TeachingPopover (microsoft#31112)
  applying package updates
  fix: SpinButton buttons now show correct visuals at bounds (microsoft#31126)
  feat: add accessibility docs to Link about color/underlines (microsoft#31121)
  fix: Table and DataGrid should not remove cells from the accessibility tree (microsoft#31068)
  Add shadow DOM support to `@fluentui/react` (Fluent v8) (microsoft#30689)
  fix(react-swatch-picker): fixes after bug bash (microsoft#31097)
  feat: unify v9 babel preset in all packages (microsoft#31088)
  applying package updates
  ...
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Apr 23, 2024
@marcosmoura
Merge branch 'master' into feat/react-drawer/move-scroll-to-context
033678b 
* master: (416 commits)
  fix: remove relative imports within stories which are invalid for creating 'show docs' and circular dep imports from cypress files (microsoft#31087)
  Fix overlapping bars on continuous axes (microsoft#31035)
  feat(scripts-tasks): implement ~36% faster type-check task (microsoft#31116)
  feat(workspace-plugin): implement split-library-in-two migration generator (microsoft#31086)
  applying package updates
  Sankey diagram: Support number formatting (microsoft#31113)
  Fix wrong position of hover callout in case of single data AreaChart (microsoft#30256)
  Combobox filtering bug fix (microsoft#31141)
  chore(react-tag-picker): adds text elliptical clipping example (microsoft#31114)
  docs: command cheat sheet (microsoft#30685)
  chore: refactor tests for createPresenceComponent() (microsoft#31137)
  Stable Release: TeachingPopover (microsoft#31112)
  applying package updates
  fix: SpinButton buttons now show correct visuals at bounds (microsoft#31126)
  feat: add accessibility docs to Link about color/underlines (microsoft#31121)
  fix: Table and DataGrid should not remove cells from the accessibility tree (microsoft#31068)
  Add shadow DOM support to `@fluentui/react` (Fluent v8) (microsoft#30689)
  fix(react-swatch-picker): fixes after bug bash (microsoft#31097)
  feat: unify v9 babel preset in all packages (microsoft#31088)
  applying package updates
  ...
miroslavstastny pushed a commit to miroslavstastny/fluentui that referenced this pull request Jun 14, 2024
@smhigley @miroslavstastny
fix: Table and DataGrid should not remove cells from the accessibilit…
6ef258b 
…y tree (microsoft#31068)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ling1726 ling1726 ling1726 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
April Project Cycle Q1 2024
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@smhigley @fabricteam @ling1726