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

[a11y] Draftail toolbar buttons can not be selected/focused using keyboard #149

Closed
Xaviju opened this issue May 14, 2018 · 11 comments
Closed
Labels
bug Something isn't working help wanted Extra attention is needed question Further information is requested
Milestone

Comments

@Xaviju
Copy link

Xaviju commented May 14, 2018

Do you want to request a feature or report a bug?
Report a bug

What is the current behavior?

I can't focus on the draftail buttons using keyboard. All of them have a tabindex="-1" on the file ToolbarButton.js:55
This is a matter or accesibility Is there a reason for this attribute on the toolbar?

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. GIFs and screenshots are very helpful too.

Only using keyboard: Focus (using tab key) on an toolbar button (bold, for example) on draftail and try to select (pressing enter key) it and to set the text on bold.

What is the expected behavior?

To be able to use the keyboard to select the button tag and afterwards write in bold text in the textarea.

Which versions of Draftail + Draft.js, and which browser / OS are affected by this issue? Did this work in previous versions of Draftail or Draft.js?

v0.17.1
Using all major browsers, latest versions

I'll be happy to PR a fix if bug is confirmed. Thanks for your work!

@thibaudcolas
Copy link
Collaborator

thibaudcolas commented May 14, 2018

Hey @Xaviju, thank you for taking the time to write this.

This is done on purpose (see f627456) for two reasons:

  1. Most buttons only affect the portion of text that's selected, or where the cursor is. If you moved focus to a button to activate it with the keyboard, the cursor's position in the field wouldn't be displayed anymore and there would be no visual indication of what content will be changed.
  2. When the page contains multiple Draftail fields, it's better for users if focus jumps between the text inputs than having to go through all of the toolbar elements for each Draftail instance.

TinyMCE, CKEditor, and a lot of other editors also do the same thing. Let me know if you think there's a better way – I did spend some time to make sure Draftail at least followed accessibility basics (and there are automated tests using aXe), but I'm no expert.

@loicteixeira
Copy link

Keyboard navigation is often used to refer to non-mouse navigation, i.e. either actual keyboard navigation or reader but I'll assume that we refer to the former here.

Regarding @thibaudcolas' points:

  1. Although it doesn't solve the issue, remember the editor has shortcuts for most of its actions, thus mitigating the need to tab through the controls.
  2. I believe there are some navigation patterns for child elements. Basically, you can tab through the main elements, but then you can enter a particular element and see its children (e.g. with a dropdown, you don't want all the options to be within the default tab order, but you still want to be able to then select an option from there).

That's a tough one but definitely something important.

@Xaviju
Copy link
Author

Xaviju commented May 16, 2018

Thanks for the responses. I have not enough a11y knowledge to ensure that this is the expected behavior. I was just testing my app using a non-mouse navigation. I wasn't able to click on those buttons so I thought it was a bug.
Before closing the issue, I'll ask some experts how do they use this editors.

@Xaviju
Copy link
Author

Xaviju commented May 16, 2018

Closing this issue for now.

Although the method of using a tabindex="-1" is strongly discouraged for accesibility and some screen-reader users that read this issue told me that buttons should not have a negative tabindex (responses in spanish) I can see that most of the editors, including those that claim that are accessible, are using the same approach.

So it must be accessible somehow and its me that doesn't know how. 🤔

Thanks again for your time.

@Xaviju Xaviju closed this as completed May 16, 2018
@thibaudcolas
Copy link
Collaborator

👌I'd love to revisit this, and any other way to make the editor more accessible, if someone who has more screen-reader knowledge is willing to get involved: telling us what to do based on what existing "screen-reader friendly" editors do, and how to test it properly.

For example, one area of improvement would be in how we convey what formatting is activated in the editor to a SR user so they know what keyboard shortcuts to use.

@thibaudcolas
Copy link
Collaborator

thibaudcolas commented May 16, 2018

Another thought: should we add a note in the project's README or elsewhere to state where the editor stands on accessibility matters? That is – "we try to follow best practices, we use aXe to test the basics", and potentially "please get in touch if you can help us make the editor more accessible"?

@thibaudcolas thibaudcolas added bug Something isn't working help wanted Extra attention is needed question Further information is requested labels May 16, 2018
@Xaviju
Copy link
Author

Xaviju commented May 16, 2018

That sounds really good and I would love to see that more often!
It would encourage more diverse users to participate in the development as well.

@cfarm
Copy link

cfarm commented Jul 26, 2019

Thinking about this response more:

Most buttons only affect the portion of text that's selected, or where the cursor is. If you moved focus to a button to activate it with the keyboard, the cursor's position in the field wouldn't be displayed anymore and there would be no visual indication of what content will be changed.

Can we add a visual indication like a background color that stays visible on the selected text when you move keyboard focus to the buttons in the rich text toolbar?

When the page contains multiple Draftail fields, it's better for users if focus jumps between the text inputs than having to go through all of the toolbar elements for each Draftail instance.

I agree with @loicteixeira that there are standard solutions to this:

I believe there are some navigation patterns for child elements. Basically, you can tab through the main elements, but then you can enter a particular element and see its children (e.g. with a dropdown, you don't want all the options to be within the default tab order, but you still want to be able to then select an option from there).

@thibaudcolas
Copy link
Collaborator

I’ll reopen this after discussion with @cfarm – while the current implementation works quite well for screen reader users, for other keyboard users there is a major flaw in not having any way at all to know what keyboard shortcuts are available (or otherwise use the buttons).

I’m not sure what the best solution would be but I need to take your suggestions from #149 (comment) into account, and also look into how other editors do this.

Any input is very much appreciated.

@thibaudcolas thibaudcolas reopened this Jul 28, 2019
@thibaudcolas thibaudcolas added this to the v1.3.0 milestone Jul 28, 2019
@thibaudcolas
Copy link
Collaborator

Oh and I’d like to make this editor compliant with ATAG (https://www.w3.org/WAI/standards-guidelines/atag/) as well as WCAG. That’s probably a separate issue, but I thought I should mention it here as well because I’d love some help with this as I have 0 experience with ATAG.

@thibaudcolas thibaudcolas modified the milestones: v1.3.0, v1.4.0 Aug 14, 2019
@thibaudcolas thibaudcolas modified the milestones: v1.4.0, v1.5.0 Apr 14, 2021
@thibaudcolas thibaudcolas modified the milestones: v1.5.0, v2.0.0 Jul 12, 2022
@thibaudcolas
Copy link
Collaborator

What this issue is originally about (selecting/focusing toolbar options with the keyboard) has been implemented with the new block toolbar and command palette features. I believe this neatly solves this problem so I’ll close this again (though further accessibility fixes & improvements are still very welcome as separate issues).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants