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.

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

DetailList: Screen Reader is reading wrong column header with associated cell value for single-select list #11365

Merged
merged 4 commits into from
Dec 9, 2019

Conversation

xugao
Copy link
Contributor

@xugao xugao commented Dec 4, 2019

Pull request checklist

Description of changes

aria-colindex is not being respected when narrator is reading the header name for each row value. (the multi-line cross-reference scenario).
This issue seems to something lacking by narrator. Here is a minimal repro in codepen to better demo this bug & my fix: https://codepen.io/xugao/pen/WNbNaVW
The fix I can find is to not hide the empty checkbox header column. Then narrator will count it as a column by default, and header name will be correct since header row has same number of columns as other rows.

Microsoft Reviewers: Open in CodeFlow

@size-auditor
Copy link

size-auditor bot commented Dec 4, 2019

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react DetailsList 208.231 kB 208.138 kB BelowBaseline     -93 bytes
office-ui-fabric-react ShimmeredDetailsList 218.748 kB 218.655 kB BelowBaseline     -93 bytes

ExceedsTolerance Over Tolerance (1024 B) ExceedsBaseline Over Baseline BelowBaseline Below Baseline New New Deleted  Removed 1 kB = 1000 B

Baseline commit: 953d773faf8a24bbcfecf9e39a2f5f371c9e5c49 (build)

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Dec 4, 2019

Component Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Status
BaseButton 827 776
BaseButton (experiments) 1007 1015
DefaultButton 1035 1058
DefaultButton (experiments) 1986 2015
DetailsRow 3352 3412
DetailsRow (fast icons) 3355 3274
DetailsRow without styles 3122 3121
DocumentCardTitle with truncation 34896 35067
MenuButton 1355 1343
MenuButton (experiments) 3606 3596
PrimaryButton 1219 1233
PrimaryButton (experiments) 2134 2144
SplitButton 2871 2840
SplitButton (experiments) 7220 7228
Stack 508 517
Stack with Intrinsic children 1158 1157
Stack with Text children 4553 4415
Text 410 403
Toggle 871 863
Toggle (experiments) 2320 2346
button 68 63

@ecraig12345
Copy link
Member

Can you share screenshots of what it looks like before and after? If there's a visual difference this may be something we need to run by design.

@xugao
Copy link
Contributor Author

xugao commented Dec 5, 2019

@ecraig12345 - there is no visual diff caused by this change. the element i removed style from is empty.
https://codepen.io/xugao/pen/QWwbNBB if you want to play real quick

Copy link
Member

@KevinTCoughlin KevinTCoughlin left a comment

Choose a reason for hiding this comment

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

Signing-off. The style change LGTM since there is no visual regression when manually removing style when selectAllVisibility={SelectAllVisibility.hidden} is provided via onRenderDetailsHeader, see: https://codepen.io/kevintcoughlin/pen/QWwyxRq?&editable=true.

@xugao xugao closed this Dec 9, 2019
@xugao xugao reopened this Dec 9, 2019
@xugao xugao merged commit 9f33910 into microsoft:master Dec 9, 2019
@msft-github-bot
Copy link
Contributor

🎉office-ui-fabric-react@v7.70.0 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Accessibility] Screen Reader is not reading ShimmeredDetailsList's column header with associated cell value
5 participants