WordPress.org

Make WordPress Core

Opened 7 days ago

Last modified 5 days ago

#41872 assigned defect (bug)

Code Editor: Minor accessibility improvements to the CodeMirror editing areas

Reported by: afercia Owned by: melchoyce
Milestone: 4.9 Priority: normal
Severity: normal Version: trunk
Component: Customize Keywords: has-patch has-screenshots
Focuses: ui, accessibility, administration Cc:

Description

The CodeMirror-powered code editor areas recently introduced in [41376] can be improved a bit with some minor accessibility enhancements.

First of all, the CodeMirror editing area needs a proper ARIA role and aria-multiline attribute to be exposed to assistive technologies as equivalent to a native HTML textarea. This was mentioned in the original ticket, see https://core.trac.wordpress.org/ticket/12423#comment:49

Further improvements:

  • all the editing areas need to be labeled, whether using a <label> element, an aria-label, or an aria-labelledby attribute.
  • all the underlying textarea elements need to be labeled, this is particularly important when CodeMirror is disabled in the user profile and the textarea are actually used
  • worth noting the Custom HTML widget used to have a "Content:" label that was removed, it should be restored also for consistency with all the other widgets
  • the Additional CSS textarea in the Customizer needs a label
  • the Plugin and Theme editors miss a label since forever (I guess), this is a good opportunity to add it
  • the help text with instructions to escape from the keyboard trap related to the Tab key needs to be consistently used as target for aria-describedby on both the textarea elements and the CodeMirror editing areas
  • this is a nice opportunity to improve the help text and make it consistent, users and translators will be happy
  • the Additional CSS help toggle and panel aria-expanded attribute needs to be fixed, see https://github.com/WordPress/better-code-editing/issues/89

Attachments (2)

41872.diff (19.7 KB) - added by afercia 7 days ago.
41872.2.diff (20.2 KB) - added by westonruter 7 days ago.
Use editor.codemirror.display.lineDiv for obtaining .CodeMirror-code

Download all attachments as: .zip

Change History (12)

#1 @afercia
7 days ago

  • Keywords has-patch added

41872.diff is a first pass to try to address the points above.

Also:

  • in the Customizer, all the "help-toggle" buttons are now buttons with a type="button" attribute, so there's no need to prevent the default action, use a keydown event, and filter the keys. They just need a click event
  • prevents a focus loss when closing the Additional CSS help
  • in the Plugin editor page: changes a <big> element to <h2> for consistency with the Theme editor page
  • removes some translation domain leftovers ('better-code-editing')
  • changes the Help text list to a paragraph with an ID so it can be targeted by aria-describedby
  • standardises the Help text wherever it is used, see new text below: suggestions for better wording welcome
  • fixes a typo "plan text mode" to "plain text mode"

About the new Help text, I'd say it's important that is always the same everywhere to avoid confusion for users and make translators happy. New text:

When using a keyboard: In the editing area the Tab key enters a tab character. To move away from this area by pressing Tab, press the Esc key followed by the Tab key. Screen reader users: you may need to press the Esc key twice to exit forms mode and then to allow the Tab key to move away from the editing area.

@westonruter I'd greatly appreciate your feedback and review.

@afercia
7 days ago

@westonruter
7 days ago

Use editor.codemirror.display.lineDiv for obtaining .CodeMirror-code

#2 @afercia
7 days ago

  • Keywords has-screenshots added

Visual changes introduced in 41872.diff:

https://cldup.com/pB1BYCoY2d.png

https://cldup.com/GUoRBtLqSo.png

https://cldup.com/b45nB46t8V.png

#3 @westonruter
7 days ago

  • Owner set to melchoyce
  • Status changed from new to assigned

@afercia the code changes look good, other than the jQuery for obtaining .CodeMirror-code. This can be simplified by just accessing editor.codemirror.display.lineDiv. Done in 41872.2.diff.

Otherwise, for the text changes and label additions, I'll defer to @melchoyce, @michelleweber, and @mizejewski.

Last edited 7 days ago by westonruter (previous) (diff)

#4 @afercia
7 days ago

accessing control.editor.codemirror.display.lineDiv

Ah, good to know, thanks!

#5 @afercia
7 days ago

and label additions

Worth noting the label for the Custom HTML widget is not an addition :) actually, in 4.8 there's a label and it was removed in the CodeMirror patch, which is an a11y regression.

#6 @westonruter
7 days ago

For adding inline help to Custom HTML widget, see #41876.

#7 @westonruter
7 days ago

An additional change needed for 41872.2.diff. When clicking on the label it should focus on the CodeMirror editor.

In other words:

$( label ).on( 'click', function() {
    editor.codemirror.focus();
} );

#8 @afercia
7 days ago

Yeah, I was hesitant to address also the focus issue, because, ideally, that should work out of the box :( I've also quickly tried that and it triggers some weird behaviors:

  • in Firefox, the page scrolls and the editor area first line goes under the WP toolbar
  • in both Chrome and Firefox, if one line in the editor area was previously focused, clicking the label puts focus on that line because I guess CodeMirror remembers the previous insertion point; this behaviour is confusing when the focused line is outside of the visible editor area portion (which doesn't scroll)

About the Help text, see also the discussion on Slack.

#9 @afercia
6 days ago

Trying to keep the Help text as an unordered list and at the same time using this text as target for aria-describedby is not so easy because the text is composed by separate strings (and elements):

https://cldup.com/8rHGjEmEfK.png

There are basically two options:

  • wrap the whole Help text within a container with an ID and target the wrapper using aria-describedby="wrapper-ID": this doesn't work consistently across various browsers/screen readers combinations, see test results below
  • aria-describedby accepts multiple IDs so can target multiple elements; this would require to use a unique ID on each element of the Help text and target them this way: aria-describedby="element-1 element-2 element-3 element-4"; this works with all combos I've tested with, but it's not so clean in terms of code, especially for the JavaScript part

However, there's one more important concern: translations. Ideally, messages that are meaningful as a whole, shouldn't be split in separate strings. Splitting a string, increases the chances the whole message will be translated inconsistently and should be avoided. Feedback from the polyglots team would be very appreciated.

Test results: for the ones who want to have some fun testing with screen readers, I've prepared 3 codepens:

Test aria-describedby targeting parent element with multiple children
https://codepen.io/afercia/full/XeJxwB

Test aria-describedby targeting parent element (hidden with display none) with multiple children
https://codepen.io/afercia/full/YrPdmj/

Test aria-describedby targeting multiple elements
https://codepen.io/afercia/full/BwyGNp

Safari + VoiceOver have a very weird behavior, where also the visibility of the help text plays a role. When targeting the wrapper, the whole text gets announced only if it's hidden with display: none. Instead, when it's visible, only the first sentence gets read out. For clarity, I've made a short video:

https://cloudup.com/cCcpmOxRjqi

Conclusion: I'd recommend to keep it simple and use just a single element (a paragraph). Worth also noting the Help text should be the same in 4 different places, and when used in the admin pages Help tabs, a list might not look so nice: depending on the screen width and translations, there's the chance the text will wrap producing a few "widows", for example:

https://cldup.com/ncnV-TE9hp.png

#10 @westonruter
5 days ago

  • Summary changed from Minor accessibility improvements to the CodeMirror editing areas to Code Editor: Minor accessibility improvements to the CodeMirror editing areas
Note: See TracTickets for help on using tickets.