Problem/Motivation

The CKEditor toolbar buttons in the "available buttons" and "active toolbar" areas are missing focus. This is violation of W3C 2.4.7 Focus Visible (Level AA)

Steps to reproduce

1. Install Drupal 9.3.12
2. Login as admin
3. Go to /admin/config/content/formats/manage/full_html?destination=/admin/config/content/formats
4. Select CKEditor as the text editor
5. Use the tab key to try to focus one of the buttons in the toolbar configuration under "available buttons" and "active toolbar". 3277552.mov shows the result of tabbing from Roles/Administrator through the toolbar configuration to the CKEditor plugin settings.

Expected result: As buttons receive focus, they get a visible highlight

Actual result: As buttons receive focus, nothing visible happens

Proposed resolution

Similar to #3202493: Claro is missing focus in "Available buttons" within CKEditor toolbar configuration, apply appropriate styling to Seven

Remaining tasks

None.

User interface changes

Add visible focus to focused buttons. See MR.

API changes

None.

Data model changes

None.

Release notes snippet

N/A

Issue fork drupal-3277552

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

Charles Belov created an issue. See original summary.

charles belov’s picture

Issue summary: View changes
StatusFileSize
new6.58 MB
rootwork’s picture

Issue tags: +Novice, +Portland2022

I'd say this is appropriate for tagging as Novice, as the linked issue provides a template for the fix.

HEBL made their first commit to this issue’s fork.

hebl’s picture

Hey all.

Fairly new to contributing but I've had a stab at this one. Happy to rework anything if needed.

Thanks

hebl’s picture

Status: Active » Needs review
rootwork’s picture

Issue summary: View changes
Status: Needs review » Needs work
StatusFileSize
new55.9 KB

@HEBL welcome and thank you!

I reviewed this in tugboat: https://mr2200-krofo1kqbcenruivewydofgnv9a6n765.tugboat.qa/admin/config/...

Screenshot gif of me tabbing through the items:
animated gif of changed UI

I can confirm that the MR adds the focus styles for the "Available buttons" items.

However, it looks like it isn't showing up for the "Active toolbar" items -- although that wasn't specified in the issue summary, it is in the issue title. This is based on the fix for #3202493: Claro is missing focus in "Available buttons" within CKEditor toolbar configuration, minus some Claro specifics (like postcss), so I think this should also address those buttons. See for instance this screenshot of the other fix.

If you look closely in the gif, you'll see that the active toolbar item groups are getting a focus style (dotted outline) but not the individual items themselves.

I tested in Firefox on Linux, so @HEBL if you did see focus styles on those active toolbar items it may be a browser thing. But setting this back to needs work so you can take a look.

Also updating the issue summary to make the "active toolbar" part clear.

hebl’s picture

@rootwork, thanks for taking a look and providing feedback. Apologies for not spotting that issue.

I've made some changes to the MR which now include the focus state for the Active toolbar buttons too. More CSS has been added to cover the active toolbar buttons.

Please see attached screen recording of it now working on the active toolbar buttons.

hebl’s picture

Assigned: Unassigned » hebl
Status: Needs work » Needs review
hebl’s picture

Assigned: hebl » Unassigned
asha nair’s picture

StatusFileSize
new1.06 MB

Patch #11 applied successfully. On focussing buttons, they have a blue outline shadow.

asha nair’s picture

StatusFileSize
new2.68 MB
rootwork’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for the review and videos Asha Nair. I can also confirm that all the buttons now have the proper focus state.

This may require submaintainer approval, but for now moving to RTBC.

rootwork’s picture

Issue summary: View changes

Updating issue summary.

alexpott’s picture

Version: 9.3.x-dev » 9.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 5c2b568cbe to 10.0.x and 7eda63abbb to 9.5.x and a09f4347f7 to 9.4.x. Thanks!

I'm not sure about adding a library in a patch release so I chose to only to commit this to 9.4.x and up so it'll be fixed in 9.4.0.

  • alexpott committed 5c2b568 on 10.0.x
    Issue #3277552 by Hebl, Asha Nair, rootwork, Charles Belov: Seven is...

  • alexpott committed 7eda63a on 9.5.x
    Issue #3277552 by Hebl, Asha Nair, rootwork, Charles Belov: Seven is...
alexpott’s picture

I checked and this is already fixed in Claro.

  • alexpott committed a09f434 on 9.4.x
    Issue #3277552 by Hebl, Asha Nair, rootwork, Charles Belov: Seven is...

Status: Fixed » Closed (fixed)

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