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

HTML block: CodeMirror accessibility (and JS TypeErrors) #6680

Closed
afercia opened this issue May 10, 2018 · 3 comments
Closed

HTML block: CodeMirror accessibility (and JS TypeErrors) #6680

afercia opened this issue May 10, 2018 · 3 comments
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented May 10, 2018

The CodeMirror implementation for the HTML block should match as much as possible what was done in core for other CodeMirror instances, with regards to accessibility.

ARIA roles, properties and labelling:

The CodeMirror contenteditable area needs to be exposed as a role textbox with aria-multiline="true", which is equivalent to a textarea element.

The contenteditable area needs to be properly labelled.

Since the Tab key is used to insert a tab character, users need to be informed that to exit the CodeMirror area they have to press the Escape key first.

For example, this is what core does for the theme editor, using jQuery. See https://core.trac.wordpress.org/changeset/41586

// Improve the editor accessibility.
$( editor.codemirror.display.lineDiv )
	.attr({
		role: 'textbox',
		'aria-multiline': 'true',
		'aria-labelledby': 'theme-plugin-editor-label',
		'aria-describedby': 'editor-keyboard-trap-help-1 editor-keyboard-trap-help-2 editor-keyboard-trap-help-3 editor-keyboard-trap-help-4'
	});

The same attributes should be used in the Gutenberg implementation. These attributes need to be set on the contenteditable area returned by editor.codemirror.display.lineDiv. Please note:

  • aria-labelledby can be replaced with aria-label (with a meaningful text) if there's no visible text to reference
  • aria-describedby is used to point to the text in the Help tab; in Gutenberg, it should point to another element providing the same description. The element can be visually hidden. Something along these lines:
When using a keyboard to navigate:
In the editing area, the Tab key enters a tab character.
To move away from this area, press the Esc key followed by the Tab key.
Screen reader users: when in forms mode, you may need to press the Esc key twice.

After pressing Escape and tabbing away from the HTML block with Tab or Shift+Tab, I get these errors:

  • Uncaught TypeError: settings.onTabPrevious is not a function
  • Uncaught TypeError: settings.onTabNext is not a function

See src/wp-admin/js/code-editor.js

@afercia afercia added [Type] Bug An existing feature does not function as intended [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). labels May 10, 2018
@designsimply designsimply added the Needs Testing Needs further testing to be confirmed. label Jul 26, 2018
@afercia
Copy link
Contributor Author

afercia commented Sep 17, 2018

@designsimply not sure why there's the need for the Needs Testing label. These issues were already discussed during implementation of CodeMirror in core and they should be addressed in an equivalent way.

I'd say this should be milestoned for 5.0, as releasing it in its current state won't even have feature parity with the implementation of CodeMirror in core. @mtias any objections?

@designsimply
Copy link
Member

I will sometimes add the Needs Testing label if some time has passed after an issue has been opened and there hasn't been any additional activity. Hopefully it will help cut back on stale issues overall, especially as we grow our testing team. 🙂

@youknowriad
Copy link
Contributor

Core Mirror is removed from Gutenberg for now (see #10423) I'd close this issue for now and make sure we check the accessibility guidelines when we bring it back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

3 participants