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

Dropdown: Removing aria-required from multiselect scenarios since it's not allowed for role='button' #14369

Merged

Conversation

khmakoto
Copy link
Member

@khmakoto khmakoto commented Aug 5, 2020

Pull request checklist

Description of changes

We were placing aria-required on Dropdowns, but this clashed with the role='button' we were placing on multiselect Dropdowns and creating accessibility warnings. We're fixing this issue by setting aria-required only for single select Dropdowns.

Focus areas to test

(optional)

@khmakoto
Copy link
Member Author

khmakoto commented Aug 5, 2020

@msft-github-bot merge in 1 minute

@msft-github-bot
Copy link
Contributor

Hello @khmakoto!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

  • I won't merge this pull request until after the UTC date Wed, 05 Aug 2020 22:02:54 GMT, which is in 1 minute

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 5, 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 4916bfe:

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
competent-rosalind-uirgy Issue #14355

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 5, 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 b47a230:

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
nervous-galois-bvjsi Issue #14355

@msft-github-bot
Copy link
Contributor

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 939 923 5000
ButtonNext mount 591 593 5000
Checkbox mount 1612 1633 5000
CheckboxBase mount 1450 1359 5000
CheckboxNext mount 1702 1719 5000
ChoiceGroup mount 5151 5240 5000
ComboBox mount 963 929 1000
CommandBar mount 7935 8011 1000
ContextualMenu mount 14250 14158 1000
DefaultButton mount 1163 1166 5000
DetailsRow mount 3733 3844 5000
DetailsRowFast mount 3671 3743 5000
DetailsRowNoStyles mount 3554 3513 5000
Dialog mount 1567 1571 1000
DocumentCardTitle mount 1956 1899 1000
Dropdown mount 2637 2716 5000
FocusZone mount 1949 1901 5000
IconButton mount 1783 1790 5000
Label mount 356 351 5000
Link mount 443 455 5000
LinkNext mount 503 504 5000
MenuButton mount 1507 1470 5000
Nav mount 3370 3382 1000
Panel mount 1539 1505 1000
Persona mount 881 883 1000
Pivot mount 1437 1425 1000
PivotNext mount 1428 1460 1000
PrimaryButton mount 1325 1302 5000
SearchBox mount 1311 1312 5000
SearchBoxNext mount 1387 1398 5000
Slider mount 1604 1566 5000
SliderNext mount 2029 2032 5000
SpinButton mount 5123 5168 5000
SpinButtonNext mount 5318 5299 5000
Spinner mount 449 433 5000
SplitButton mount 3313 3270 5000
Stack mount 533 564 5000
StackWithIntrinsicChildren mount 2091 2016 5000
StackWithTextChildren mount 5257 5238 5000
TagPicker mount 2849 2845 5000
Text mount 444 447 5000
TextField mount 1447 1467 5000
ThemeProvider mount 3113 3172 5000
ThemeProvider virtual-rerender 563 542 5000
Toggle mount 880 882 5000
ToggleNext mount 872 862 5000
button mount 118 112 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.45 0.5 0.9:1 2000 903
🦄 Button.Fluent 0.11 0.2 0.55:1 5000 547
🔧 Checkbox.Fluent 0.65 0.35 1.86:1 1000 654
🎯 Dialog.Fluent 0.16 0.22 0.73:1 5000 782
🔧 Dropdown.Fluent 3.07 0.46 6.67:1 1000 3073
🔧 Icon.Fluent 0.15 0.05 3:1 5000 728
🦄 Image.Fluent 0.07 0.11 0.64:1 5000 370
🔧 Slider.Fluent 1.67 0.35 4.77:1 1000 1668
🔧 Text.Fluent 0.07 0.03 2.33:1 5000 352
🦄 Tooltip.Fluent 0.11 18.44 0.01:1 5000 550

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AttachmentMinimalPerf.default 159 142 1.12:1
PortalMinimalPerf.default 139 126 1.1:1
FlexMinimalPerf.default 292 269 1.09:1
AttachmentSlotsPerf.default 1236 1158 1.07:1
AnimationMinimalPerf.default 412 392 1.05:1
AvatarMinimalPerf.default 502 479 1.05:1
Text.Fluent 352 334 1.05:1
CardMinimalPerf.default 572 549 1.04:1
GridMinimalPerf.default 331 317 1.04:1
SegmentMinimalPerf.default 349 337 1.04:1
ButtonSlotsPerf.default 630 614 1.03:1
ChatMinimalPerf.default 639 619 1.03:1
HeaderSlotsPerf.default 812 791 1.03:1
ListMinimalPerf.default 473 461 1.03:1
PopupMinimalPerf.default 714 696 1.03:1
RefMinimalPerf.default 219 213 1.03:1
Checkbox.Fluent 654 634 1.03:1
BoxMinimalPerf.default 346 340 1.02:1
DropdownManyItemsPerf.default 775 763 1.02:1
HeaderMinimalPerf.default 362 354 1.02:1
LabelMinimalPerf.default 411 401 1.02:1
LoaderMinimalPerf.default 763 748 1.02:1
MenuButtonMinimalPerf.default 1590 1566 1.02:1
SplitButtonMinimalPerf.default 3884 3822 1.02:1
StatusMinimalPerf.default 703 690 1.02:1
Icon.Fluent 728 712 1.02:1
Tooltip.Fluent 550 538 1.02:1
DividerMinimalPerf.default 358 355 1.01:1
EmbedMinimalPerf.default 1965 1947 1.01:1
LayoutMinimalPerf.default 396 391 1.01:1
ListNestedPerf.default 897 887 1.01:1
ProviderMergeThemesPerf.default 2041 2023 1.01:1
ProviderMinimalPerf.default 938 929 1.01:1
ToolbarMinimalPerf.default 985 971 1.01:1
TooltipMinimalPerf.default 814 802 1.01:1
Image.Fluent 370 365 1.01:1
ChatDuplicateMessagesPerf.default 433 434 1:1
ChatWithPopoverPerf.default 492 492 1:1
CheckboxMinimalPerf.default 2955 2956 1:1
DialogMinimalPerf.default 782 782 1:1
DropdownMinimalPerf.default 3138 3125 1:1
MenuMinimalPerf.default 867 870 1:1
RadioGroupMinimalPerf.default 417 415 1:1
TextAreaMinimalPerf.default 461 460 1:1
TreeMinimalPerf.default 897 895 1:1
CarouselMinimalPerf.default 464 468 0.99:1
ImageMinimalPerf.default 366 369 0.99:1
ItemLayoutMinimalPerf.default 1255 1270 0.99:1
ListCommonPerf.default 977 987 0.99:1
ListWith60ListItems.default 1107 1115 0.99:1
ReactionMinimalPerf.default 389 392 0.99:1
TableManyItemsPerf.default 2279 2293 0.99:1
TableMinimalPerf.default 413 419 0.99:1
CustomToolbarPrototype.default 3898 3919 0.99:1
Avatar.Fluent 903 908 0.99:1
Button.Fluent 547 555 0.99:1
Dialog.Fluent 782 789 0.99:1
Dropdown.Fluent 3073 3090 0.99:1
AlertMinimalPerf.default 296 303 0.98:1
HierarchicalTreeMinimalPerf.default 427 437 0.98:1
InputMinimalPerf.default 1334 1359 0.98:1
SliderMinimalPerf.default 1661 1688 0.98:1
Slider.Fluent 1668 1694 0.98:1
FormMinimalPerf.default 381 391 0.97:1
IconMinimalPerf.default 654 671 0.97:1
TextMinimalPerf.default 340 352 0.97:1
ButtonMinimalPerf.default 171 178 0.96:1
TreeWith60ListItems.default 208 216 0.96:1
AccordionMinimalPerf.default 141 148 0.95:1
VideoMinimalPerf.default 623 656 0.95:1

@msft-github-bot msft-github-bot merged commit ff937a4 into microsoft:master Aug 5, 2020
@size-auditor
Copy link

size-auditor bot commented Aug 5, 2020

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-next-Dropdown 222.869 kB 222.897 kB ExceedsBaseline     28 bytes
office-ui-fabric-react office-ui-fabric-react-Dropdown 222.869 kB 222.897 kB ExceedsBaseline     28 bytes

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

Baseline commit: c8bb1d97af900f52e46aba43cad1c6b51f5b6377 (build)

@khmakoto khmakoto deleted the multiSelectDropdownRequired branch August 6, 2020 00:25
@msft-github-bot
Copy link
Contributor

🎉[email protected] has been released which incorporates this pull request.:tada:

Handy links:

msft-github-bot pushed a commit that referenced this pull request Aug 6, 2020
)

#### Pull request checklist

- [ ] Addresses an existing issue: Fixes #0000
- [x] Include a change request file using `$ yarn change`

#### Description of changes

This PR is a port of fixes #14367 and #14369 done in `office-ui-fabric-react` to `@fluentui/react-next`.

#### Focus areas to test

(optional)
tmaster628 pushed a commit to tmaster628/fluentui that referenced this pull request Aug 12, 2020
…s not allowed for role='button' (microsoft#14369)

#### Pull request checklist

- [x] Addresses an existing issue: Fixes microsoft#14355
- [x] Include a change request file using `$ yarn change`

#### Description of changes

We were placing `aria-required` on `Dropdowns`, but this clashed with the `role='button'` we were placing on multiselect `Dropdowns` and creating accessibility warnings. We're fixing this issue by setting `aria-required` only for single select `Dropdowns`.

#### Focus areas to test

(optional)
tmaster628 pushed a commit to tmaster628/fluentui that referenced this pull request Aug 12, 2020
…rosoft#14377)

#### Pull request checklist

- [ ] Addresses an existing issue: Fixes #0000
- [x] Include a change request file using `$ yarn change`

#### Description of changes

This PR is a port of fixes microsoft#14367 and microsoft#14369 done in `office-ui-fabric-react` to `@fluentui/react-next`.

#### Focus areas to test

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