Code Editor: Minor accessibility improvements to the CodeMirror editing areas

[afercia]

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

[afercia]

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.

[westonruter]

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

[afercia]

Visual changes introduced in 41872.diff​:

[westonruter]

@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.

[afercia]

accessing control.editor.codemirror.display.lineDiv

Ah, good to know, thanks!

[afercia]

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.

[westonruter]

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

[westonruter]

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();
} );

[afercia]

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.

[afercia]

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):

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:

[westonruter]

Bumping priority to high for visibility and alignment with 4.9 goals, and given proximity to beta 1 deadline.

[melchoyce]

Conclusion: I'd recommend to keep it simple and use just a single element (a paragraph).

I still think it'll be much easier for sighted users to understand if it's broken up into paragraphs and list items. If we decide to do one paragraph, then I think we need to revisit the decision to open these instructions by default. I don't want to overwhelm people with a giant paragraph the first time they open the CSS panel.

Is there a way to add a note for translators to look at all the strings in context before translating?

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.

That's fine — nothing can be expected to be perfect when you're dealing with variable widths. :)

[afercia]

I've worked a bit in the last days to a patch to keep the list in all the 4 places where the help text is used. Just need to refresh it after some recent changes to core.

Worth noting that, to keep the list and also target the proper elements with aria-describedby, there's the need to do things like this for the HTML part:

aria-describedby="editor-keyboard-trap-help-1 editor-keyboard-trap-help-2 editor-keyboard-trap-help-3 editor-keyboard-trap-help-4"

and this for the JS part:

'aria-describedby' => 'editor-keyboard-trap-help-1 editor-keyboard-trap-help-2 editor-keyboard-trap-help-3 editor-keyboard-trap-help-4'

which is ugly 😞

@westonruter thoughts?

I'll upload the patch as soon as I have time to refresh it.

[westonruter]

@afercia Thank you for working on that tedious task. The beauty of the code is always secondary to the benefit the code will yield to the users (in both visual and aural modes), so if this compound aria-describedby is what it takes then I fully support it. Besides, it's not so ugly so much as it is verbose. 👍

[melchoyce]

Replying to westonruter:

@afercia Thank you for working on that tedious task. The beauty of the code is always secondary to the benefit the code will yield to the users (in both visual and aural modes), so if this compound aria-describedby is what it takes then I fully support it. Besides, it's not so ugly so much as it is verbose. 👍

+1!

[afercia]

When clicking on the label it should focus on the CodeMirror editor.

Trying this on the Additional CSS, focus always goes to the last line (and if the content is very long, it's out of view). Seems to me because of:

			// Refresh when receiving focus.
			control.editor.codemirror.on( 'focus', function( codemirror ) {
				codemirror.refresh();
			});

Is this really necessary? And why it is used only for the Additional CSS and not for the other CodeMirror instances? If there are no particular reasons, I'd consider to remove it.

[afercia]

41872.3.diff​ tries to refactor the patch to keep the help text with the unordered list. Things to check:

• please check how input_attrs is passed to the JS template
• added a third item to the "keyboard trap" help text list and reviewed the pre-existing text to make it a bit shorter and more clear; feedback for better wording/English welcome
• the "keyboard trap" help text is now the same in all the 4 places where it is used; maybe it could use a function to avoid repetition. Not sure where the function should go to be available both in the admin screens and in the Customizer
• Custom HTML widget: some help text should be displayed only when CodeMirror is enabled, made some changes accordingly
• Additional CSS: focus issue when clicking on the label, see comment above
• removed the URL fragment identifier used for the link to the profile page profile.php#syntax_highlighting because in the profile page the related control stays hidden behind the admin toolbar (see screenshot below)
• does get_edit_profile_url() needs to be escaped? see esc_url( get_edit_profile_url() )

About translations: the "keyboard trap" help text is now composed by 4 strings. As mentioned before, not sure this is so ideal for translation and I'd like to have some feedback. /cc @ocean90 @SergeyBiryukov

Screenshot of the profile page with the Syntax Highlighting control hidden by the admin bar:

Screenshot of the keyboard trap help text in the Customizer > Additional CSS:

(the same text is used in the plugin/theme editors and targeted by aria-describedby for the Custom HTML widget)

[westonruter]

Replying to afercia:

When clicking on the label it should focus on the CodeMirror editor.

Trying this on the Additional CSS, focus always goes to the last line (and if the content is very long, it's out of view). Seems to me because of:

			// Refresh when receiving focus.
			control.editor.codemirror.on( 'focus', function( codemirror ) {
				codemirror.refresh();
			});

Is this really necessary? And why it is used only for the Additional CSS and not for the other CodeMirror instances? If there are no particular reasons, I'd consider to remove it.

Oh thank you for identifying this!! I had opened #41900 for this issue. I'll follow up there with working to remove this.

[afercia]

Great, thanks! 🍷

[westonruter]

@afercia looks good! I made just a couple tweaks for alignment with coding standards. Go ahead and commit.

[westonruter]

@afercia 41872.5.diff​ ensures that all of the input_attrs are output by looping over each one. This is what will be required by #30738.

[afercia]

41872.6.diff​ refreshed after recent changes. Also: consistently escapes $scrollto and removes a few default textdomains.

[afercia]

In 41586:

Accessibility: CodeMirror editing areas minor improvements.

• properly labels all the code editor areas (Theme/Plugin, Custom HTML widget, Additional CSS), whether CodeMirror is enabled or disabled
• adds role="textbox" and aria-multiline="true" to the CodeMirror editing area to allow assistive technologies properly identify it as a textarea
• standardizes the "keyboard trap" help text across the admin and keeps it as a list for better readability
• use the Help text elements as target for aria-describedby, to make screen readers read out the help text when focusing the editors
• fixes the aria-expanded attribute usage in the Customizer "Additional CSS" help toggle
• moves focus to the CodeMirror editing area when clicking on the associated label
• in the Plugin editor screen: changes a <big> element to <h2> for better semantics and consistency with the Theme editor screen
• also, removes a few textdomain leftovers, see better-code-editing and default

Props westonruter, melchoyce, afercia.
Fixes #41872.

[westonruter]

In 41957:

Code Editor: Improve ability to create Customizer CodeEditorControl instances in JS, lessening PHP dependencies.

Allow CodeEditorControl to be instantiated with a editor_settings param which is merged with wp.codeEditor.defaultSettings.

Also:

• Turn redundant "CSS Code" control label into screen reader text for Additional CSS.
• Remove code-editor as script dependency for custom-html-widgets since enqueueing is determined by wp_enqueue_code_editor().
• Remove useless exporting of code_type param to JS in WP_Customize_Code_Editor_Control.
• Add disabled class to Custom HTML widget's Save button when linting errors are present.
• Remove redundant span inside CodeEditorControl's label.

See #41897, #12423, #41872.