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

Basic accessibility support for the keyboard #5298

Closed
oleq opened this issue Jan 10, 2017 · 6 comments · Fixed by ckeditor/ckeditor5-ui#143
Closed

Basic accessibility support for the keyboard #5298

oleq opened this issue Jan 10, 2017 · 6 comments · Fixed by ckeditor/ckeditor5-ui#143
Assignees
Labels
domain:accessibility This issue reports an accessibility problem. package:ui status:discussion type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Milestone

Comments

@oleq
Copy link
Member

oleq commented Jan 10, 2017

Things to consider:

  1. Critical: Focusing and navigating the toolbar, as in v4:

    Press Alt+F10 to navigate to the toolbar. Move to the next and previous toolbar group with TAB and SHIFT+TAB. Move to the next and previous toolbar button with RIGHT ARROW or LEFT ARROW. Press SPACE or ENTER to activate the toolbar button.

    We don't have toolbar groups yet, so there's one thing less to worry about.

  2. Critical: Focus cycling in forms and balloon panel navigation (i.e. in the link panel), similar to dialog navigation in v4:

    Inside a dialog, press TAB to navigate to the next dialog element, press SHIFT+TAB to move to the previous dialog element, press ENTER to submit the dialog, press ESC to cancel the dialog. [...]

  3. Critical: Dropdown navigation (i.e. headings dropdown)

    Inside a list-box, move to next list item with TAB OR DOWN ARROW. Move to previous list item with SHIFT+TAB or UP ARROW. Press SPACE or ENTER to select the list option. Press ESC to close the list-box.

  4. Optional: Page Navigation Using the "Tab" Key, as in http://sdk.ckeditor.com/samples/tabindex.html.

  5. Optional: Accessibility instructions dialog similar to v4:

    image

Note: ARIA attributes are not in the scope of this issue.

@oleq oleq self-assigned this Jan 10, 2017
@Reinmar
Copy link
Member

Reinmar commented Jan 10, 2017

Quick comment – before 1.0.0 we need to think what keyboard a11y requires and work on things which are important for the API of our UI library. The idea is that this will prevent the need to doing big refactoring after 1.0.0 (which would result in breaking changes in the API).

In other words – we need to list all the things, but we have to code only these critical for the API.

@fredck
Copy link
Contributor

fredck commented Jan 10, 2017

Optional: Page Navigation Using the "Tab" Key

Wouldn't classify this one as optional.

@Reinmar
Copy link
Member

Reinmar commented Jan 10, 2017

Wouldn't classify this one as optional.

I'm curious when we have to do anything about that. So far editor on https://ckeditor5.github.io/ can be focused and left easily. The only issue is that toolbar buttons get focus before editor, but this, I think, should be fixed by removing them from tab cycle (by tabindex=-1).

But this raises a question – should buttons have tabindex=-1 by default, or should it be added only to buttons which land in the toolbar? If so, then how?

@fredck
Copy link
Contributor

fredck commented Jan 11, 2017

I'm curious when we have to do anything about that. So far editor on https://ckeditor5.github.io/ can be focused and left easily

For inline editables, this should not be a problem. So, by case, things work out of the box with the classic creator.

There may be other creators which would instead have troubles. For example the iframe creator. So maybe this is a "per creator" issue to be handled.

The only issue is that toolbar buttons get focus before editor, but this, I think, should be fixed by removing them from tab cycle (by tabindex=-1).

Yes, this is an issue. The toolbar should be transparent to the browser tabbing order.

But this raises a question – should buttons have tabindex=-1 by default, or should it be added only to buttons which land in the toolbar? If so, then how?

This depends a lot on our strategy for keyboard navigation. Using tabindex makes sense only if we want to leave to the browser to handle it. I assume we'll have to introduce a custom keyboard handler, just like we do in v4. At that point, I would go tabindex=-1 by default.

@oleq
Copy link
Member Author

oleq commented Jan 17, 2017

@oleq
Copy link
Member Author

oleq commented Jan 19, 2017

Continuing the discussion from ckeditor/ckeditor5-ui#143 (review), I think it's not a big deal for FocusCycler to work with tree structures like toolbar w/ groups.

When groups will be implemented in ToolbarView, two FCs should be spawned. The first one will handle tab/shift+tab navigation across the groups; it will iterate over a collection of groups.

A second one will change it's #focusables collection property, once a group is focused by the first FC, to enable focus cycling across items of that particular group.

It's probably a matter of bringing an event or observable to FC, so that second instance will automatically to re–attach itself to another toolbar group (collection of items in that group). That's all, there's no need to implement any recursive strategy or anything. Two FCs should handle it IMO.

@mlewand mlewand transferred this issue from ckeditor/ckeditor5-ui Oct 9, 2019
@mlewand mlewand added this to the iteration 6 milestone Oct 9, 2019
@mlewand mlewand added status:discussion type:feature This issue reports a feature request (an idea for a new functionality or a missing option). package:ui labels Oct 9, 2019
@Reinmar Reinmar added the domain:accessibility This issue reports an accessibility problem. label Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:accessibility This issue reports an accessibility problem. package:ui status:discussion type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants