-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
docs(Toolbar): accessibility polish - testing preparation #12884
docs(Toolbar): accessibility polish - testing preparation #12884
Conversation
return ( | ||
<Toolbar | ||
aria-label="Toolbar can contain a radio group in a menu" | ||
items={[ | ||
toolbarItem('underline', <UnderlineIcon />), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: To be honest, I would just spread objects here. From client's perspective it's much easier to just go trough the items object, instead of seeing what the function that is invoked is doing...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be much more readable :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed :)
...ages/fluentui/docs/src/examples/components/Toolbar/Content/ToolbarExamplePopup.shorthand.tsx
Outdated
Show resolved
Hide resolved
description="Toolbar item can open a menu which can contain radio groups." | ||
examplePath="components/Toolbar/Content/ToolbarExampleMenuRadioGroup" | ||
/> | ||
<ComponentExample | ||
title="Toolbar can contain a submenu in a menu" | ||
toolbarAriaLabel="Example Toolbar can contain a submenu in a menu" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shuoldn't this be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch thank you :)
@@ -7,6 +7,7 @@ const Performance = () => ( | |||
<ExampleSection title="Performance"> | |||
<ComponentPerfExample | |||
title="Custom styled" | |||
toolbarAriaLabel="Example performance" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch thank you :)
…/ToolbarExamplePopup.shorthand.tsx commit based on Marija comment Co-Authored-By: Marija Najdova <mnajdova@gmail.com>
…kolaps33/office-ui-fabric-react into mituron/acc-testing-preparation
Description of changes
In order to give Toolbar component for testing, we should be consistent with ARIA.
https://www.w3.org/TR/wai-aria-practices-1.2/#toolbar
Apply following:
and from role "Toolbar" itself
https://www.w3.org/TR/wai-aria-1.2/#toolbar
Apply following:
In order to fulfill these requirements above:
Focus areas to test
(optional)
Microsoft Reviewers: Open in CodeFlow