Follow up to to partially revert #3023300: Text area style updates.

Problem/Motivation

On formatted text areas, Claro hides the label for the text format selection. No justification was given for this.

There are a few accessibility problems with this:

  • The purpose of the text format <select> isn't clear without a label. The text format names can be literally anything a site builder chooses, so the options here may not be understandable without a visible label for the drop-down.
  • The name of the <select> isn't clear without a visible label. This makes it awkward for speech control users to operate. The normal way to operate a <select> is to speak the name of the control directly (i.e. "Click Text format"). If the name of the control isn't visible, you don't know what to say. It can't be operated by saying the current value (e.g. "Click Restricted HTML"). Other methods are available in speech control, but they involve extra steps. For example "Show numbers. Choose 24." is two steps, and you never know what number to expect so that's an additional cognitive load. Some speech control software offers a mouse targeting grid, but this usually takes 6-10 steps so it's a last resort.

The relevant WCAG success criteria is "3.3.2 Labels or Instructions" at level A.

Proposed resolution

Make the label visible, like it is in other themes. Use the inline variation.

Figma specs to see the spacing between elements&font sizes: https://www.figma.com/file/OqWgzAluHtsOd5uwm1lubFeH/Drupal-Design-system...

Remaining tasks

Remove the override of #title_display in _claro_text_format_prerender().

User interface changes

Restores a label which is important for understanding the form.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andrewmacpherson created an issue. See original summary.

huzooka’s picture

Priority: Major » Normal
andrewmacpherson’s picture

Priority: Normal » Major

We use major for triage of WCAG level A success criteria

huzooka’s picture

Priority: Major » Normal

This is not Major, or not a Bug imho. See this doc.

andrewmacpherson’s picture

Issues relating to WCAG level A have been treated as major for a while now, in triage for other experimental module/theme/profiles in core. We should probably update the doc you linked to reflect that.

Comments 26 & 28 mention this in #3079738: Add Claro administration theme to core.

katannshaw’s picture

@andrewmacpherson: Would making the label visible above the field like my screenshot shows be acceptable? If so I can create a patch for this.

-Kat

ckrina’s picture

Priority: Normal » Major
Issue summary: View changes
FileSize
16.09 KB

If we were going to take this approach showing the label, I'd say we should display the label inline like in this specs:

If Andrew thinks this issue has major priority, accessibility-wise, I'd prefer to respect that since he's Accessibility maintainer. It doesn't mean it has to be a beta blocker, but it means it has a major impact on accessibility and knowing it will help us address it sooner.

fhaeberle’s picture

Is this considered ready for implementation? Because discussion is still ongoing.

andrewmacpherson’s picture

Issue summary: View changes

#6 and #7: Either of these arrangements will satisfy the WCAG "Labels and instructions".

Added a bit more explanation to the issue summary, about why it's a bug. Lack of a visible label requires speech control users to jump through extra hoops to operate it. Dragon users cannot operate a <select> by speaking the current value; it expects you to say the label.

Can we add an issue credit for @mfairchild365 please? I didn't have a copy of Dragon to hand, so he did some testing for me.

andrewmacpherson’s picture

Issue summary: View changes

ckrina’s picture

ckrina’s picture

Perfect then, thanks @andrewmacpherson! Updating the issue summary with the final designs to implement.

And thanks @katannshaw too for the suggestion! I just proposed the inline label to keep the vertical use of space as limited as possible since it's been a complain in other designs in the past :)

Note for the implementation: it is possible that there isn't an inline variation of the inline label+text field/select implemented yet and it might have to be developed.

katannshaw’s picture

Assigned: Unassigned » katannshaw

I'm going to work on the patch by following the screenshot provided by @ckrina. Thanks to you and @andrewmacpherson for the clarity.

-Kat

huzooka’s picture

katannshaw’s picture

@huzooka: I'm trying to test your patch however I can't find the /src/ClaroPreRender.php used in the patch within the Claro theme.

FYI: I was planning on making that change within claro.theme, adding .container-inline to the wrapping div element just before the label, and then adding needed CSS for the xs input's right margin.

-Kat

katannshaw’s picture

Assigned: katannshaw » Unassigned
andrewmacpherson’s picture

Status: Active » Needs work
Issue tags: +Needs reroll

#16 - yeah, I was confused by that too. There are two branches in the Claro contrib repository (8.x-1.x and 8.x-2.x) but there isn't a top-level src/ directory in either of them.

Claro has now been added to the Drupal core 8.9.x branch as an experimental alpha theme. The patch which added it DOES contain the ClaroPreRender utility class. It basically gets used as a way to organize code better, and keep the hook_element_info_alter() short and tidy. See #3079738-146: Add Claro administration theme to core.

So the Claro contrib module is now out of sync with what's in Drupal core.

@huzooka (or anyone else) - can we have an update on what the Claro development process is now? I expect this affeccts a lot of issues now. .

huzooka’s picture

Project: Claro » Drupal core
Version: 8.x-1.x-dev » 8.9.x-dev
Component: User interface » Claro theme
shashikant_chauhan’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
2.14 KB

Just rerolled the patch from #15

shimpy’s picture

I can't able to see the claro theme in drupal 8.9 dev
Can anyone tell me in which directory i can find so. I am attaching the screenshot for the themes available in drupal 8.9 dev right now.

huzooka’s picture

Are you sure that you have the most-recent patches?

It is there!

shimpy’s picture

hi @huzooka

I have installed Drupal8.9 again now the claro is coming as experimental theme.
Might be perviously i installed 8.9 version before claro been updated as experimental thats's y it was not working.
Thanks for reminding me to install the latest release.

rensingh99’s picture

Issue summary: View changes
Status: Needs review » Needs work
FileSize
3.96 KB

Hi,

I have reviewed the patch #20. It's added the inline label but there are no space between label and select.

You can not use var(--space-xs) in form.css, you should use var(--space-xs) variable in form.pcss.css and after compile it will add CSS in form.css .

I am attaching screenshot.

Thanks,
Ren

huzooka’s picture

Assigned: Unassigned » huzooka
huzooka’s picture

Assigned: huzooka » Unassigned
Status: Needs work » Needs review
FileSize
3.76 KB

No interdiff, since this just fixes the problematic rebase.

rensingh99’s picture

Assigned: Unassigned » rensingh99
rensingh99’s picture

Assigned: rensingh99 » Unassigned
Status: Needs review » Reviewed & tested by the community
FileSize
2.64 KB

Hi,

I have reviewed the patch # 26. It's looking great.

Below is screenshot output after applying patch.

Only local images are allowed.

Thanks,
Ren

alexpott’s picture

Version: 8.9.x-dev » 8.8.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed and pushed b12a3a59e1 to 9.0.x and c9a231cc83 to 8.9.x. Thanks!

Going to confirm with a release manager about backport to 8.8.x

  • alexpott committed b12a3a5 on 9.0.x
    Issue #3083570 by huzooka, shashikant_chauhan, rensingh99, ckrina,...

  • alexpott committed c9a231c on 8.9.x
    Issue #3083570 by huzooka, shashikant_chauhan, rensingh99, ckrina,...
alexpott’s picture

Status: Patch (to be ported) » Fixed

Discussed with @catch - as claro is experimental this can be backported.

  • alexpott committed 8ad1028 on 8.8.x
    Issue #3083570 by huzooka, shashikant_chauhan, rensingh99, ckrina,...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

shimpy’s picture