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

[Nav][Accessibility] Selected items are not announced by narrator #14982

Closed
wants to merge 7 commits into from

Conversation

cimoli-dev
Copy link

@cimoli-dev cimoli-dev commented Sep 10, 2020

Pull request checklist

Description of changes

To get narrator to announced the selected nav item, aria-selected attributed must be added to the list item. However, that attribute is only supported on the following roles: gridcell, option, row, tab.

Because of that, change the role of the items from 'listitem' to 'treeitem' and set the value to true/false depending on if the link is selected.

However, there is a known issue in both narrator and JAWS where the selected state is not being announced. I've added a comment in the code about this.
Aria-selected and aria-checked are not output correctly for trees #432

image

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 10, 2020

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 bcae018:

Sandbox Source
Fluent UI Button Configuration
microsoft/fluentui: codesandbox-react-template Configuration
microsoft/fluentui: codesandbox-react-next-template Configuration
microsoft/fluentui: codesandbox-react-northstar-template Configuration

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Sep 10, 2020

Perf Analysis

Scenario Render type Master Ticks PR Ticks Iterations Status
button mount 104 110 5000 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 903 895 5000
ButtonNext mount 687 702 5000
Checkbox mount 1692 1597 5000
CheckboxBase mount 1367 1336 5000
CheckboxNext mount 1545 1557 5000
ChoiceGroup mount 5035 5041 5000
ChoiceGroupNext mount 5058 5043 5000
ComboBox mount 916 919 1000
CommandBar mount 7415 7345 1000
ContextualMenu mount 12634 12401 1000
DefaultButton mount 1115 1115 5000
DetailsRow mount 3584 3601 5000
DetailsRowFast mount 3609 3523 5000
DetailsRowNoStyles mount 3435 3407 5000
Dialog mount 1489 1495 1000
DocumentCardTitle mount 1738 1735 1000
Dropdown mount 2681 2625 5000
FocusZone mount 1810 1791 5000
IconButton mount 1975 1789 5000
Label mount 325 337 5000
Link mount 453 452 5000
LinkNext mount 484 474 5000
MenuButton mount 1494 1517 5000
MessageBar mount 2022 1998 5000
MessageBarNext mount 2048 2034 5000
Nav mount 3202 3262 1000
OverflowSet mount 1409 1399 5000
OverflowSetNext mount 1031 1025 5000
Panel mount 1395 1458 1000
Persona mount 855 826 1000
Pivot mount 1384 1420 1000
PivotNext mount 1385 1360 1000
PrimaryButton mount 1267 1268 5000
Rating mount 7722 7791 5000
RatingNext mount 7841 7694 5000
SearchBox mount 1308 1334 5000
SearchBoxNext mount 1376 1371 5000
Shimmer mount 2631 2642 5000
ShimmerNext mount 2601 2622 5000
Slider mount 1535 1519 5000
SliderNext mount 1989 1957 5000
SpinButton mount 4962 4944 5000
SpinButtonNext mount 5489 5057 5000
Spinner mount 408 412 5000
SplitButton mount 3158 3110 5000
Stack mount 537 527 5000
StackWithIntrinsicChildren mount 1852 1891 5000
StackWithTextChildren mount 5042 5010 5000
SwatchColorPicker mount 10304 10249 5000
SwatchColorPickerNext mount 11174 10211 5000
TagPicker mount 2792 2703 5000
TeachingBubble mount 48297 48028 5000
TeachingBubbleNext mount 48412 48286 5000
Text mount 442 425 5000
TextField mount 1391 1392 5000
ThemeProvider mount 4247 4207 5000
ThemeProvider virtual-rerender 430 445 5000
Toggle mount 851 852 5000
ToggleNext mount 803 836 5000
button mount 104 110 5000 Possible regression

Perf Analysis (Fluent)

⚠️ 5 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
ButtonOverridesMissPerf.default 1690 46 36.74:1 analysis
ButtonUseCssNestingPerf.default 1069 41 26.07:1 analysis
ButtonUseCssPerf.default 837 44 19.02:1 analysis
ChatWithPopoverPerf.default 485 461 1.05:1 analysis
ListNestedPerf.default 641 866 0.74:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.46 0.49 0.94:1 2000 915
🦄 Button.Fluent 0.12 0.2 0.6:1 5000 617
🔧 Checkbox.Fluent 0.67 0.37 1.81:1 1000 665
🎯 Dialog.Fluent 0.16 0.22 0.73:1 5000 822
🔧 Dropdown.Fluent 2.83 0.5 5.66:1 1000 2826
🔧 Icon.Fluent 0.15 0.06 2.5:1 5000 746
🦄 Image.Fluent 0.08 0.12 0.67:1 5000 418
🔧 Slider.Fluent 1.54 0.37 4.16:1 1000 1540
🔧 Text.Fluent 0.08 0.03 2.67:1 5000 388
🦄 Tooltip.Fluent 0.11 14.86 0.01:1 5000 572

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
PortalMinimalPerf.default 170 115 1.48:1
AttachmentMinimalPerf.default 202 150 1.35:1
RefMinimalPerf.default 245 193 1.27:1
BoxMinimalPerf.default 406 331 1.23:1
FlexMinimalPerf.default 324 273 1.19:1
SegmentMinimalPerf.default 410 345 1.19:1
StatusMinimalPerf.default 780 655 1.19:1
GridMinimalPerf.default 381 322 1.18:1
AnimationMinimalPerf.default 432 373 1.16:1
DividerMinimalPerf.default 418 362 1.15:1
LabelMinimalPerf.default 456 397 1.15:1
RadioGroupMinimalPerf.default 487 424 1.15:1
Text.Fluent 388 336 1.15:1
AccordionMinimalPerf.default 168 148 1.14:1
ImageMinimalPerf.default 420 367 1.14:1
ReactionMinimalPerf.default 443 390 1.14:1
VideoMinimalPerf.default 700 614 1.14:1
Image.Fluent 418 366 1.14:1
Tooltip.Fluent 572 502 1.14:1
HeaderMinimalPerf.default 406 358 1.13:1
LayoutMinimalPerf.default 439 389 1.13:1
SkeletonMinimalPerf.default 465 410 1.13:1
TextMinimalPerf.default 396 349 1.13:1
Button.Fluent 617 544 1.13:1
ChatMinimalPerf.default 676 604 1.12:1
FormMinimalPerf.default 479 428 1.12:1
PopupMinimalPerf.default 712 642 1.11:1
TableMinimalPerf.default 448 405 1.11:1
TooltipMinimalPerf.default 829 745 1.11:1
HeaderSlotsPerf.default 886 802 1.1:1
TextAreaMinimalPerf.default 525 476 1.1:1
AlertMinimalPerf.default 328 300 1.09:1
AvatarMinimalPerf.default 500 459 1.09:1
ButtonMinimalPerf.default 187 171 1.09:1
ItemLayoutMinimalPerf.default 1340 1244 1.08:1
TreeMinimalPerf.default 931 865 1.08:1
Avatar.Fluent 915 846 1.08:1
ProviderMinimalPerf.default 920 863 1.07:1
IconMinimalPerf.default 725 675 1.07:1
Dialog.Fluent 822 769 1.07:1
Icon.Fluent 746 698 1.07:1
DialogMinimalPerf.default 817 769 1.06:1
ListMinimalPerf.default 511 483 1.06:1
MenuMinimalPerf.default 895 847 1.06:1
Checkbox.Fluent 665 625 1.06:1
AttachmentSlotsPerf.default 1192 1135 1.05:1
CardMinimalPerf.default 600 572 1.05:1
ProviderMergeThemesPerf.default 1875 1782 1.05:1
ToolbarMinimalPerf.default 998 955 1.05:1
CarouselMinimalPerf.default 468 449 1.04:1
EmbedMinimalPerf.default 1984 1916 1.04:1
InputMinimalPerf.default 1298 1249 1.04:1
LoaderMinimalPerf.default 746 717 1.04:1
MenuButtonMinimalPerf.default 1636 1579 1.04:1
DropdownManyItemsPerf.default 796 774 1.03:1
SplitButtonMinimalPerf.default 3834 3731 1.03:1
CustomToolbarPrototype.default 3682 3562 1.03:1
ButtonSlotsPerf.default 605 591 1.02:1
ChatDuplicateMessagesPerf.default 422 414 1.02:1
CheckboxMinimalPerf.default 2879 2831 1.02:1
DropdownMinimalPerf.default 2871 2819 1.02:1
TableManyItemsPerf.default 2305 2269 1.02:1
Slider.Fluent 1540 1510 1.02:1
SliderMinimalPerf.default 1513 1495 1.01:1
Dropdown.Fluent 2826 2798 1.01:1
ListWith60ListItems.default 979 1065 0.92:1
TreeWith60ListItems.default 198 223 0.89:1
ListCommonPerf.default 696 945 0.74:1

@size-auditor
Copy link

size-auditor bot commented Sep 10, 2020

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-next-Nav 180.363 kB 180.407 kB ExceedsBaseline     44 bytes
office-ui-fabric-react office-ui-fabric-react-Nav 180.363 kB 180.407 kB ExceedsBaseline     44 bytes

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

Baseline commit: f199d137f3aa5cb03989937e6db547463c009d71 (build)

@cimoli-dev cimoli-dev changed the title User/cimoli/again [Nav][Accessibility] Selected items are not announced by narrator Sep 10, 2020
Copy link
Member

@ecraig12345 ecraig12345 left a comment

Choose a reason for hiding this comment

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

I don't think tab is the right role here. aria-selected is also supported on several other roles including treeitem, which I think would be more appropriate here. In that case the role of the <ul> would need to be changed to tree.

@cimoli-dev
Copy link
Author

@ecraig12345 I'm using tree and treeitem now. However, narrator and JAWS are not announcing the selected property. It appears to be an open issue for both ATs. Because of that i've added a comment about it near the change.

@ecraig12345
Copy link
Member

ecraig12345 commented Sep 15, 2020

@ecraig12345 I'm using tree and treeitem now. However, narrator and JAWS are not announcing the selected property. It appears to be an open issue for both ATs. Because of that i've added a comment about it near the change.

Thanks for the update! If I remember right from when I started to look into this issue several months ago, the problem may be that aria-current is not actually the correct or only attribute to use, and aria-selected should be used instead or in addition. But I'd have to do some more reading to refresh my memory on that. (Would also welcome your input if you're more familiar with the correct way to handle things here.)

@Pangamma
Copy link

Pangamma commented Sep 17, 2020

Checking this PR once per day as it would fix one of the bugs currently experienced by one of the projects I work on.

Copy link
Member

@khmakoto khmakoto left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this fix, but due to work we're currently doing to prepare master for our version 8 beta release, we're asking contributors to either wait a couple weeks to submit fixes (if it's not urgent) or submit to the new 7.0 branch (if it's urgent). See #15222 for more details.

@msft-github-bot
Copy link
Contributor

This pull request has been automatically marked as stale because it was marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 5 days of this comment. Thank you for your contributions to Fluent UI!

@Weffe
Copy link

Weffe commented Dec 8, 2021

This is still an issue and should get fixed because as the user traverses to the "selected" nav item, then the AT doesn't announce it.

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