-
Notifications
You must be signed in to change notification settings - Fork 359
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
Toolbar Example: Bugs with keyboard support, aria-disabled, and aria-label #847
Comments
@tatermelon is planning to submit a PR for this issue. |
I am planning on submitting a pull request on an updated toolbar example later this week. See the following link: Mostly need to clean up some CSS best practices and focus styling issues with my student. |
@jongund, the WIP you have looks like a more realistic toolbar. I didn't know you were working on that. With a few changes, it could resolve issue #541. That is something we should discuss. In fact, that would be a good way to frame the PR. For project management reasons, I don't want this issue closed until we have the resolution committed. So, I am re-opening it. The contract with Bocoup referenced the current toolbar; this one is significantly more complex. The tests for the current toolbar are merged into the Bocoup branch, The tests that fail due to the bugs in the current example are commented out. This example would drive rework on that set of tests, which @spectranaut may not be able to contain. Timing will also be a factor. I see quite a few issues that need resolution and possibly discussion in this new example. So, I am not completely confident it can be done before the Bocoup contract ends. The current target completion date for test development is sep 24. If you are confident you will have the resources to complete this toolbar along with necessary test revisions (if needed) before December as well as the other stuff we already have in the release 3 plan, I'm OK with adopting issue #541 as the resolution plan for this issue. I'll take it off @tatermelon's plate for now, and we can discuss the plan in the september 10 meeting. Some challenges I see in the current implementation of the new toolbar are ... It has toggle buttons that do not follow the toggle button pattern; once pressed, they cannot be unpressed. This is because they are toggles pretending to be radios. I am not sure how to best resolve that. This is a fairly common type of design, and one that is not easily resolved without tweaking existing ARIA patterns. I do not think we want toggles that cannot be toggled. Yes, you could add The only real barrier to using the existing radio group pattern in a toolbar is that the checked state cannot follow focus like in normal radio groups because the arrow keys are needed to navigate to other elements in the toolbar. That means requiring the user to press space to check the radio. Maybe that is OK. I'd recommend implementing that approach and bringing it to the group for review. I noticed that the toolbar label is "toolbar" ... It should probably be "Text format". Remember, never use the role name in the label. The buttons and button groups are not labeled. The font menu button says it is a font menu before a font is chosen, but does not say that it is the font menu after a font is chosen; it does tell the name of the current font, but the label should be something like "Font FONT_NAME". Even on page load, the label should probably reflect the current font. I'm putting this feedback here for now in response to your most recent comment. For follow-on discussion of work on an enhanced toolbar, however, we should do it in a new PR or a different issue. Issue #541 would be more appropriate since that is the issue that calls for enhancements to make the existing toolbar more illustrative and realistic. |
…#pull 950) completes work to: * Resolve keyboard, aria-disabled, and aria-label bugs listed in issue #847. * Add additional widget types requested in issue #541, including toggle, checkbox, spinner, menu button, radio group, and link. * The above are all issues discussed in toolbar review issue #539. Note: missing regression tests are tracked by issue #965.
As noted in issue #539, the toolbar example has the following bugs:
aria-label="Example Toolbar"
ondiv
withrole="
toolbar"`aria-disabled="true"
and appropriate greyed out styling on one of the button elements (add to second button element in toolbar). As documented in accessibility features section, the disabled button should remain focusable.Home
key does not move focus to first element.End
key does not move focus to last element.Right Arrow
key does not wrap focus from last to first element.Left Arrow
key does not wrap focus from first to last element.The text was updated successfully, but these errors were encountered: