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

fix(react-examples): Add narrator cue for cleaning action in Date Picker examples #16463

Merged
merged 2 commits into from
Jan 26, 2021
Merged

Conversation

andrefcdias
Copy link
Contributor

@andrefcdias andrefcdias commented Jan 13, 2021

Pull request checklist

Description of changes

Added an Announced component for when the user presses the Clear button on the Date Picker Input and Date Picker Required examples

…ker examples

(microsoftdesign/fluentui#10254)
@fabricteam
Copy link
Collaborator

fabricteam commented Jan 13, 2021

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 888 846 5000
BaseButtonCompat mount 979 953 5000
Breadcrumb mount 42319 42638 5000
Checkbox mount 1654 1581 5000
CheckboxBase mount 1315 1403 5000
ChoiceGroup mount 5034 4971 5000
ComboBox mount 988 979 1000
CommandBar mount 10258 10249 1000
ContextualMenu mount 6154 6247 1000
DefaultButtonCompat mount 1174 1194 5000
DetailsRow mount 3802 3800 5000
DetailsRowFast mount 3818 3808 5000
DetailsRowNoStyles mount 3571 3513 5000
Dialog mount 1512 1531 1000
DocumentCardTitle mount 1817 1804 1000
Dropdown mount 3491 3440 5000
FocusTrapZone mount 1708 1800 5000
FocusZone mount 1842 1868 5000
IconButtonCompat mount 1827 1866 5000
Label mount 331 328 5000
Layer mount 1826 1886 5000
Link mount 503 473 5000
MakeStyles mount 1972 2024 50000
MenuButtonCompat mount 1579 1527 5000
MessageBar mount 2024 2043 5000
Nav mount 3406 3454 1000
OverflowSet mount 1058 1074 5000
Panel mount 1453 1463 1000
Persona mount 882 876 1000
Pivot mount 1484 1486 1000
PrimaryButtonCompat mount 1350 1325 5000
Rating mount 8154 8041 5000
SearchBox mount 1378 1385 5000
Shimmer mount 2708 2653 5000
Slider mount 1963 2001 5000
SpinButton mount 5158 5188 5000
Spinner mount 424 416 5000
SplitButtonCompat mount 3310 3283 5000
Stack mount 532 530 5000
StackWithIntrinsicChildren mount 1589 1590 5000
StackWithTextChildren mount 4926 4827 5000
SwatchColorPicker mount 10709 10592 5000
Tabs mount 1407 1419 1000
TagPicker mount 2931 2949 5000
TeachingBubble mount 11933 11860 5000
Text mount 426 436 5000
TextField mount 1406 1475 5000
ThemeProvider mount 2183 2165 5000
ThemeProvider virtual-rerender 659 657 5000
Toggle mount 822 844 5000
button mount 713 730 5000
buttonNative mount 115 113 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🦄 Avatar.Fluent 0.19 0.53 0.36:1 2000 389
🦄 Button.Fluent 0.13 0.21 0.62:1 5000 630
🔧 Checkbox.Fluent 0.67 0.37 1.81:1 1000 668
🎯 Dialog.Fluent 0.17 0.23 0.74:1 5000 859
🔧 Dropdown.Fluent 3.05 0.45 6.78:1 1000 3047
🔧 Icon.Fluent 0.15 0.07 2.14:1 5000 748
🦄 Image.Fluent 0.09 0.13 0.69:1 5000 442
🔧 Slider.Fluent 1.61 0.45 3.58:1 1000 1609
🔧 Text.Fluent 0.09 0.03 3:1 5000 438
🦄 Tooltip.Fluent 0.12 0.91 0.13:1 5000 592

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AvatarMinimalPerf.default 242 218 1.11:1
Text.Fluent 438 395 1.11:1
FormMinimalPerf.default 527 482 1.09:1
DividerMinimalPerf.default 430 397 1.08:1
ImageMinimalPerf.default 476 441 1.08:1
TreeMinimalPerf.default 918 851 1.08:1
Avatar.Fluent 389 361 1.08:1
AlertMinimalPerf.default 339 317 1.07:1
FlexMinimalPerf.default 354 331 1.07:1
HeaderSlotsPerf.default 922 862 1.07:1
LabelMinimalPerf.default 504 477 1.06:1
TextMinimalPerf.default 414 391 1.06:1
TextAreaMinimalPerf.default 587 555 1.06:1
ButtonMinimalPerf.default 208 198 1.05:1
DropdownManyItemsPerf.default 807 770 1.05:1
SplitButtonMinimalPerf.default 4116 3963 1.04:1
StatusMinimalPerf.default 821 787 1.04:1
TreeWith60ListItems.default 194 186 1.04:1
AttachmentSlotsPerf.default 1332 1291 1.03:1
ButtonSlotsPerf.default 637 619 1.03:1
CardMinimalPerf.default 652 635 1.03:1
CheckboxMinimalPerf.default 2981 2896 1.03:1
ItemLayoutMinimalPerf.default 1365 1324 1.03:1
TableMinimalPerf.default 470 455 1.03:1
AnimationMinimalPerf.default 447 438 1.02:1
ButtonOverridesMissPerf.default 1818 1777 1.02:1
ButtonUseCssNestingPerf.default 1185 1159 1.02:1
HeaderMinimalPerf.default 439 430 1.02:1
ListWith60ListItems.default 695 684 1.02:1
PortalMinimalPerf.default 171 167 1.02:1
RadioGroupMinimalPerf.default 534 523 1.02:1
ReactionMinimalPerf.default 465 454 1.02:1
CustomToolbarPrototype.default 3910 3831 1.02:1
TooltipMinimalPerf.default 859 844 1.02:1
Dialog.Fluent 859 839 1.02:1
Image.Fluent 442 433 1.02:1
Tooltip.Fluent 592 581 1.02:1
ChatDuplicateMessagesPerf.default 401 398 1.01:1
GridMinimalPerf.default 400 397 1.01:1
ListCommonPerf.default 734 724 1.01:1
ListMinimalPerf.default 566 561 1.01:1
MenuButtonMinimalPerf.default 1732 1717 1.01:1
Checkbox.Fluent 668 661 1.01:1
Icon.Fluent 748 743 1.01:1
AccordionMinimalPerf.default 179 179 1:1
ButtonUseCssPerf.default 875 875 1:1
ChatWithPopoverPerf.default 485 484 1:1
DropdownMinimalPerf.default 3068 3066 1:1
InputMinimalPerf.default 1380 1374 1:1
LayoutMinimalPerf.default 454 454 1:1
MenuMinimalPerf.default 956 957 1:1
ProviderMergeThemesPerf.default 1580 1575 1:1
SegmentMinimalPerf.default 391 390 1:1
TableManyItemsPerf.default 2243 2241 1:1
Button.Fluent 630 633 1:1
AttachmentMinimalPerf.default 186 188 0.99:1
BoxMinimalPerf.default 417 423 0.99:1
ChatMinimalPerf.default 683 689 0.99:1
DatepickerMinimalPerf.default 49629 49937 0.99:1
DialogMinimalPerf.default 851 861 0.99:1
EmbedMinimalPerf.default 4361 4393 0.99:1
LoaderMinimalPerf.default 780 785 0.99:1
PopupMinimalPerf.default 740 744 0.99:1
RefMinimalPerf.default 238 241 0.99:1
IconMinimalPerf.default 755 761 0.99:1
ToolbarMinimalPerf.default 1050 1056 0.99:1
VideoMinimalPerf.default 701 705 0.99:1
Dropdown.Fluent 3047 3077 0.99:1
Slider.Fluent 1609 1619 0.99:1
CarouselMinimalPerf.default 538 548 0.98:1
RosterPerf.default 1334 1367 0.98:1
SkeletonMinimalPerf.default 410 428 0.96:1
SliderMinimalPerf.default 1611 1672 0.96:1
ProviderMinimalPerf.default 969 1019 0.95:1
ListNestedPerf.default 623 663 0.94:1

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jan 13, 2021

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 0eeb71e:

Sandbox Source
Fluent UI Button Configuration
codesandbox-react-template Configuration
codesandbox-react-northstar-template Configuration

@size-auditor
Copy link

size-auditor bot commented Jan 13, 2021

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 21e7c948365236acc497fb981d6605337db11bef (build)

@ecraig12345
Copy link
Member

This is yet another of these example accessibility issues where I'm not convinced that the original complaint is valid. I can see the need for confirming operation completion for more significant and possibly slower tasks, such as deleting items from a list, but this is an extremely simple action. I wonder if either updating the button text to be more descriptive, or adding an aria-label would be adequate? @JustSlone what do you think?

@JustSlone
Copy link
Collaborator

@ecraig12345 I could go either way. However, in this case I think the suggested fix is reasonable and demonstrates a more general pattern for how to announce changes being made by the clear button.

If we do go with a more descriptive button I would recommend using aria-label rather than the button text as it's unlikely to have a verbose clear button in an actual application.

If you want to pursue the simpler path I'd recommend following up with @smhigley or our trusted tester internally to confirm the aria-label approach is sufficient here. If we do choose to go this path, let's follow up quickly and not let such a simple PR drag on and consume a lot of time.

@@ -14,9 +15,11 @@ const onFormatDate = (date?: Date): string => {

export const DatePickerFormatExample: React.FunctionComponent = () => {
const [value, setValue] = React.useState<Date | undefined>();
const [clearedAnnounced, setClearedAnnounced] = React.useState<JSX.Element | undefined>();
Copy link
Member

Choose a reason for hiding this comment

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

For this to work properly if the clear button is used multiple times, you'll also need to remove the announced message when a new date is set. Basically instead of passing setDate directly to DatePicker's onSelectDate, add a wrapper which clears the message too.

@ecraig12345
Copy link
Member

@JustSlone Thanks for the input! Let's go with the announced solution, though adding an aria-label (like "Clear start date") also wouldn't hurt.

@smhigley
Copy link
Contributor

For what it's worth, I think a clearer label + no announcement is a better pattern for this use case, and for clear buttons in general. The exception would be if there's a delay between the user pressing the button and the action completing, or maybe if the button and the thing it acts on are separated.

Otherwise, heavy use of live regions in cases like these end up making the UI somewhat chatty, which isn't awful but also isn't ideal.

@ecraig12345
Copy link
Member

For what it's worth, I think a clearer label + no announcement is a better pattern for this use case, and for clear buttons in general. The exception would be if there's a delay between the user pressing the button and the action completing, or maybe if the button and the thing it acts on are separated.

Otherwise, heavy use of live regions in cases like these end up making the UI somewhat chatty, which isn't awful but also isn't ideal.

Cool, just having a good label (or aria-label? since the field association is pretty obvious visually) sounds better to me as well.

@andrefcdias
Copy link
Contributor Author

andrefcdias commented Jan 22, 2021

I went with @smhigley's approach as it avoids the need for a more complicated implementation.
Opted for an aria-label as, like @ecraig12345 said, Clear is already pretty clear visually (pun not intended).

@andrefcdias andrefcdias merged commit f04d6a4 into microsoft:master Jan 26, 2021
msft-fluent-ui-bot pushed a commit that referenced this pull request Feb 2, 2021
#### Pull request checklist

- [x] Include a change request file using `$ yarn change`

#### Description of changes

Aggregate cherry-picking for all my contributions for the `react-examples` package

#### Focus areas to test

- [x] #16497
- [x] #16496
- [x] #16494
- [x] #16479
- [x] #16430
- [x] #16475
- [x] #16463
- [x] #16495
- [x] #16481
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.

7 participants