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
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | screen-capture (1).mp4 | 2.68 MB | asha nair |
| #14 | www_screencapture_com_2022-5-3_19_16.mp4 | 1.06 MB | asha nair |
| #11 | Screen Recording 2022-04-30 at 10.10.49.mov | 1.39 MB | hebl |
| #10 | 3277552-reviewed.gif | 55.9 KB | rootwork |
| #2 | 3277552.mov | 6.58 MB | charles belov |
Issue fork drupal-3277552
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:
- 9.3.x
changes, plain diff MR !2200
- 3277552-seven-is-missing
changes, plain diff MR !2199
Comments
Comment #2
charles belovComment #3
rootworkI'd say this is appropriate for tagging as Novice, as the linked issue provides a template for the fix.
Comment #8
hebl commentedHey all.
Fairly new to contributing but I've had a stab at this one. Happy to rework anything if needed.
Thanks
Comment #9
hebl commentedComment #10
rootwork@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:

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.
Comment #11
hebl commented@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.
Comment #12
hebl commentedComment #13
hebl commentedComment #14
asha nair commentedPatch #11 applied successfully. On focussing buttons, they have a blue outline shadow.
Comment #15
asha nair commentedComment #16
rootworkThanks 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.
Comment #17
rootworkUpdating issue summary.
Comment #18
alexpottCommitted 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.
Comment #21
alexpottI checked and this is already fixed in Claro.