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

Toolbar example for simple text editor #887

Merged
merged 89 commits into from
Dec 3, 2018
Merged

Toolbar example for simple text editor #887

merged 89 commits into from
Dec 3, 2018

Conversation

jongund
Copy link
Contributor

@jongund jongund commented Sep 13, 2018

This updated toolbar is intended to resolve issues #541 and #847.

Preview the new toolbar in Jon's repo

@jongund
Copy link
Contributor Author

jongund commented Nov 26, 2018

I updated pull request to add the design pattern references to the documentation and to remove left and right keyboard behaviors from the pull down menu items. Still working on the test cases.

@carmacleod
Copy link
Contributor

Finally had a chance to look at this quickly and write down a few suggestions:

  • Please don't use <i> for icons. I know everybody does it, but the semantics of <i> are "my content is in italics", not "my content is an icon". We should be using a span instead.

  • The "Text Alignment" group should be a "radiogroup" with "radio" children, not a role="menu".

  • Consider giving the div's with class="group" a role="group" and an aria-label. (Not sure if this would be too verbose - might check with @mcking65).

  • A more common ordering of this type of toolbar is font name first, then font size, then font emphasis like bold, italic, underline, then the paragraph alignment.

  • Cut, Copy, and Paste have a title (i.e. tooltip; note misspelled "from" in Cut/Copy), so for consistency (and for sighted users) Bold, Italics, Underline, Align left/center/right, Font type, and size increment/decrement should have a title also. (Or even better, all of the tools should have a custom tooltip that comes up on both hover and focus, or only on hover but user can type Shift+F1/Cmd+? to open tooltip when focused).

  • Would be nice if the tools supported the common shortcut keys, i.e. Ctrl/Cmd + B for Bold, Ctrl/Cmd + I for Italics, Ctrl/Cmd + U for Underline, Ctrl/Cmd + L/E/R for Left/Center/Right alignment; and if so, then all tooltips should include the appropriate shortcut key.

  • Would be nice if all of the tools supported a visual change on hover.

  • The visual difference between pressed/not pressed toggle buttons and checked/unchecked radios is too subtle. It would not pass the new WCAG 2.1 SC 1.4.11 Non-text Contrast, i.e.

    • Unpressed toggle white background:#FFFFFF in toolbar background:#EDEDEB contrast ratio is: 1.2:1
    • Pressed toggle 1-pixel outline:#F5F5F5 beside button background:#A9A9A9 contrast ratio is: 2.2:1
    • Downward-pointing triangle in menu button:#878787 on white background:#FFFFFF contrast ratio is: 3.6:1
    • Toolbar background:#EDEDEB to pressed toggle background:#F5F5F5 contrast ratio is: 1.1:1
    • Pressed toggle background:#F5F5F5 to unpressed toggle white background:#FFFFFF contrast ratio is: 1.1:1

@jongund
Copy link
Contributor Author

jongund commented Dec 2, 2018 via email

@jongund
Copy link
Contributor Author

jongund commented Dec 2, 2018

Updated pull request to include changes from Carolyn.

@carmacleod
Copy link
Contributor

Wow, that was fast - thanks, Jon!

Regarding:

  1. The standard radio button pattern does not work since selection (e.g. checked item) in the radio group pattern moves when focus moves

Ah, I see. So we need to fix the pattern to allow for radiogroups in toolbars. I've opened #949 to address this.

In the meantime, you might want to add aria-orientation="horizontal" to the "menu" to indicate that left/right arrow keys are expected (although I see that it works with up/down arrows as well, so not critical).

I like the new hover and focus styles - they are beautifully clear! It's still hard to tell the difference between checked and not-checked toggles and radios, though.

Are you still planning to add a title/tooltip to the Bold, Italics, Underline, Align left/center/right, Font type, and size increment/decrement tools (for consistency with Cut/Copy/Paste, and in case any sighted folks need help understanding the icons)?

Thanks again for the super-quick updates and explanations!

@mcking65 mcking65 merged commit 599d2de into w3c:issue541-toolbar-redesign Dec 3, 2018
@mcking65
Copy link
Contributor

mcking65 commented Dec 3, 2018

@jongund, during tomorrow's meeting, we'll discuss how to divide remaining work on this.

@annabbott
Copy link

I can now visually tell the difference between checked and not-checked toggles and radios with ease. Looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants