Merged
Conversation
talldan
added
[Type] Enhancement
talldan
changed the title
talldan
force-pushed
the
update/tool-panel-accessibility
branch
from
8c32251 to
30388e8
Compare
talldan
commented
Apr 29, 2022
| <> | ||
| <Tooltip | ||
| text={ describedBy ? describedBy : label } | ||
| text={ children?.length && describedBy ? describedBy : label } |
Contributor
Author
There was a problem hiding this comment.
The Button component was always showing the label for the tooltip of icon buttons, except when a describedBy was defined and then it would show the description.
This doesn't seem right because it makes the label not visible.
talldan
commented
Apr 29, 2022
| : _x( 'View options', 'Button label to reveal tool panel options' ); | ||
| const dropDownMenuLabelText = sprintf( | ||
| // translators: %s: The name of the tool e.g. "Color" or "Typography". | ||
| _x( '%s options', 'Button label to reveal tool panel options' ), |
Contributor
Author
There was a problem hiding this comment.
This works ok, except for the dimensions panel:
Every other tool uses a singular noun, but I understand why 'Dimension' doesn't seem right.
Member
There was a problem hiding this comment.
Because it's a mouthful? I think it should be fine: it's the label for a collective group of dimensions, e.g., options of/for dimensions.
talldan
commented
Apr 29, 2022
packages/components/src/tools-panel/tools-panel-header/component.tsx
Outdated
Show resolved
Hide resolved
|
Size Change: +190 B (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
Contributor
|
Tested with Voiceover on mac and it works as described in the testing instructions. |
Member
|
I know there are some tests to fix up but echoing @carolinan's comments: works as described using Voiceover and the UX change to the dropdown visibility seem natural to me. I did stop to pause and wonder if the panel should close on "Reset all", but I think it's minor detail that could be discussed at a later point. |
talldan
commented
May 2, 2022
talldan
force-pushed
the
update/tool-panel-accessibility
branch
from
ee15c69 to
ac8af34
Compare
talldan
force-pushed
the
update/tool-panel-accessibility
branch
from
2db91a2 to
9049e85
Compare
Contributor
Author
|
All the tests are passing now and I've added some additional unit tests 👍 |
ramonjd
approved these changes
May 2, 2022
Member
ramonjd
left a comment
ramonjd
left a comment
There was a problem hiding this comment.
Tested again using voiceover with keyboard and mouse
LGTM
I committed the CHANGELOG.md conflict (tests were passing before I did this 😄 )
Contributor
|
@talldan Thanks for your work here! 👍 I was AFK and still trying to catch up but this works really well. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
A few fixes for #40460
Addresses some accessibility issues for the
ToolsPanelcomponent:menu-itemand doesn't havearia-selectedspecified.speakto announce the outcome of selecting a menu item in the options dropdownAlso:
Buttons, where the tooltip displayed the description instead of the label.Testing Instructions
(with a screenreader active)