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

Updated ComboBox error to have role="alert" #20013

Merged
merged 4 commits into from
Sep 29, 2021
Merged

Updated ComboBox error to have role="alert" #20013

merged 4 commits into from
Sep 29, 2021

Conversation

GeoffCoxMSFT
Copy link
Member

Pull request checklist

Description of changes

Updated ComboBox error message to be consistent with DropDown (correct ARIA approach)

  • Updated to be role="alert"
  • Updated div to be added/removed with error message
  • Removed aria live and atomic attributes
  • Removed applying aria hidden when no error message

Focus areas to test

ComboBox
TimePicker (uses ComboBox)

@GeoffCoxMSFT GeoffCoxMSFT self-assigned this Sep 28, 2021
@GeoffCoxMSFT GeoffCoxMSFT requested a review from a team as a code owner September 28, 2021 20:52
@GeoffCoxMSFT
Copy link
Member Author

@ecraig12345 - spoke with @smhigley about the right attributes for the ComboBox error message. This issue (assigned to you), might be partially fixed with this change: #13245.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 28, 2021 •
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.

Latest deployment of this branch, based on commit 4ceea9b:

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

@fabricteam
Copy link
Collaborator

fabricteam commented Sep 28, 2021 •
edited
Loading

📊 Bundle size report

🤖 This report was generated against 9d423ceade746ba26013f6b42cebe1e519011b17

@size-auditor
Copy link

size-auditor bot commented Sep 28, 2021 •
edited
Loading

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-TimePicker 220.891 kB 220.811 kB BelowBaseline     -80 bytes
office-ui-fabric-react fluentui-react-ComboBox 232.619 kB 232.539 kB BelowBaseline     -80 bytes

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

Baseline commit: 9d423ceade746ba26013f6b42cebe1e519011b17 (build)

@GeoffCoxMSFT GeoffCoxMSFT changed the title Updated error to be role alert Updated ComboBox error to have role="alert" Sep 28, 2021
@fabricteam
Copy link
Collaborator

fabricteam commented Sep 28, 2021 •
edited
Loading

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1085 1082 5000
BaseButton mount 1055 1048 5000
Breadcrumb mount 2869 2852 1000
ButtonNext mount 522 532 5000
Checkbox mount 1776 1756 5000
CheckboxBase mount 1508 1535 5000
ChoiceGroup mount 5478 5506 5000
ComboBox mount 1084 1085 1000
CommandBar mount 11141 11091 1000
ContextualMenu mount 7211 7187 1000
DefaultButton mount 1311 1319 5000
DetailsRow mount 4192 4200 5000
DetailsRowFast mount 4243 4334 5000
DetailsRowNoStyles mount 3997 4006 5000
Dialog mount 2655 2675 1000
DocumentCardTitle mount 177 167 1000
Dropdown mount 3669 3647 5000
FluentProviderNext mount 7521 7533 5000
FluentProviderWithTheme mount 347 379 10
FluentProviderWithTheme virtual-rerender 99 103 10
FluentProviderWithTheme virtual-rerender-with-unmount 493 489 10
FocusTrapZone mount 2053 2030 5000
FocusZone mount 2025 2055 5000
IconButton mount 2030 2023 5000
Label mount 388 381 5000
Layer mount 3338 3358 5000
Link mount 510 554 5000
MakeStyles mount 1968 2005 50000
MenuButton mount 1694 1730 5000
MessageBar mount 2190 2207 5000
Nav mount 3831 3702 1000
OverflowSet mount 1220 1239 5000
Panel mount 2534 2537 1000
Persona mount 946 957 1000
Pivot mount 1632 1606 1000
PrimaryButton mount 1468 1478 5000
Rating mount 8843 9021 5000
SearchBox mount 1568 1539 5000
Shimmer mount 2964 2958 5000
Slider mount 2291 2247 5000
SpinButton mount 5591 5610 5000
Spinner mount 464 456 5000
SplitButton mount 3525 3529 5000
Stack mount 584 571 5000
StackWithIntrinsicChildren mount 2033 2052 5000
StackWithTextChildren mount 5473 5550 5000
SwatchColorPicker mount 11768 11763 5000
Tabs mount 1591 1623 1000
TagPicker mount 3067 3076 5000
TeachingBubble mount 13981 13887 5000
Text mount 479 475 5000
TextField mount 1612 1596 5000
ThemeProvider mount 1289 1304 5000
ThemeProvider virtual-rerender 626 640 5000
ThemeProvider virtual-rerender-with-unmount 2057 2137 5000
Toggle mount 914 949 5000
buttonNative mount 134 133 5000

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
TreeWith60ListItems.default 233 199 1.17:1
AttachmentMinimalPerf.default 224 195 1.15:1
ChatDuplicateMessagesPerf.default 372 334 1.11:1
TextMinimalPerf.default 417 377 1.11:1
ImageMinimalPerf.default 484 445 1.09:1
PortalMinimalPerf.default 207 190 1.09:1
AttachmentSlotsPerf.default 1274 1183 1.08:1
ReactionMinimalPerf.default 488 453 1.08:1
FormMinimalPerf.default 521 488 1.07:1
RefMinimalPerf.default 269 251 1.07:1
ListCommonPerf.default 760 718 1.06:1
ListNestedPerf.default 686 647 1.06:1
TableMinimalPerf.default 471 445 1.06:1
ButtonMinimalPerf.default 209 199 1.05:1
TreeMinimalPerf.default 937 896 1.05:1
VideoMinimalPerf.default 762 724 1.05:1
ChatWithPopoverPerf.default 468 448 1.04:1
DividerMinimalPerf.default 424 408 1.04:1
DropdownManyItemsPerf.default 822 794 1.04:1
GridMinimalPerf.default 398 381 1.04:1
HeaderSlotsPerf.default 891 854 1.04:1
LabelMinimalPerf.default 467 448 1.04:1
StatusMinimalPerf.default 798 771 1.04:1
IconMinimalPerf.default 753 727 1.04:1
TextAreaMinimalPerf.default 618 593 1.04:1
AnimationMinimalPerf.default 458 446 1.03:1
CarouselMinimalPerf.default 546 532 1.03:1
HeaderMinimalPerf.default 431 417 1.03:1
ButtonSlotsPerf.default 655 644 1.02:1
CardMinimalPerf.default 669 659 1.02:1
ChatMinimalPerf.default 759 742 1.02:1
DialogMinimalPerf.default 854 837 1.02:1
DropdownMinimalPerf.default 3491 3437 1.02:1
ItemLayoutMinimalPerf.default 1417 1390 1.02:1
ListMinimalPerf.default 608 597 1.02:1
LoaderMinimalPerf.default 789 775 1.02:1
RadioGroupMinimalPerf.default 524 515 1.02:1
AccordionMinimalPerf.default 181 179 1.01:1
AvatarMinimalPerf.default 232 230 1.01:1
ButtonOverridesMissPerf.default 2007 1985 1.01:1
CheckboxMinimalPerf.default 3093 3065 1.01:1
ListWith60ListItems.default 753 749 1.01:1
MenuMinimalPerf.default 969 959 1.01:1
ProviderMergeThemesPerf.default 1873 1860 1.01:1
SkeletonMinimalPerf.default 433 427 1.01:1
SplitButtonMinimalPerf.default 4869 4813 1.01:1
EmbedMinimalPerf.default 4732 4728 1:1
LayoutMinimalPerf.default 429 428 1:1
MenuButtonMinimalPerf.default 1863 1862 1:1
TableManyItemsPerf.default 2230 2228 1:1
CustomToolbarPrototype.default 4523 4519 1:1
ToolbarMinimalPerf.default 1091 1086 1:1
TooltipMinimalPerf.default 1163 1163 1:1
AlertMinimalPerf.default 318 321 0.99:1
BoxMinimalPerf.default 394 396 0.99:1
DatepickerMinimalPerf.default 6158 6200 0.99:1
FlexMinimalPerf.default 323 325 0.99:1
InputMinimalPerf.default 1449 1466 0.99:1
PopupMinimalPerf.default 646 652 0.99:1
ProviderMinimalPerf.default 1289 1305 0.99:1
SliderMinimalPerf.default 1887 1912 0.99:1
SegmentMinimalPerf.default 411 421 0.98:1
RosterPerf.default 1390 1442 0.96:1

@smhigley
Copy link
Contributor

@GeoffCoxMSFT you probably already noticed, but it looks like @fluentui/react-experiments also needs update-snapshots for the build to pass

@GeoffCoxMSFT GeoffCoxMSFT merged commit e726d8b into master Sep 29, 2021
@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot msft-fluent-ui-bot mentioned this pull request Sep 29, 2021
Closed
@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

GeoffCoxMSFT pushed a commit that referenced this pull request Sep 29, 2021
@GeoffCox
Port 'Updated ComboBox error to have role="alert" (#20013)' to 7.0
00bc797
GeoffCoxMSFT added a commit that referenced this pull request Sep 29, 2021
@GeoffCoxMSFT @GeoffCox
Port 'Updated ComboBox error to have role="alert"' to 7.0 (#20032)
863c862 
* Port 'Updated ComboBox error to have role="alert" (#20013)' to 7.0

* Change files

* Update experiment snapshot

* Change files

Co-authored-by: GeoffCox <[email protected]>
@GeoffCoxMSFT GeoffCoxMSFT deleted the combobox-error-aria branch November 2, 2021 22:47
mlp73 pushed a commit to mlp73/fluentui that referenced this pull request Jan 17, 2022
@GeoffCoxMSFT
Updated ComboBox error to have role="alert" (microsoft#20013)
c5fcd7c 
* Updated error to be role alert

* Change files

* Updated snapshot in react-experiments

* Change files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smhigley smhigley smhigley approved these changes

@ecraig12345 ecraig12345 Awaiting requested review from ecraig12345

Assignees

@GeoffCoxMSFT GeoffCoxMSFT

Labels
Component: Combobox
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ComboBox error message has incorrect role for ARIA
4 participants
@GeoffCoxMSFT @fabricteam @smhigley @msft-fluent-ui-bot