Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
From #2784569-9: Settings Tray Accessibility: Improve tabbing - comment #9, point 5
@Wim Leers review of tabbing
Tabbing to the "Advanced block options" link has a barely discernible
:focusstyle, which means that as a sighted keyboard user, it's very hard to not lose a sense of location.
Proposed resolution
Add CSS improve :focus.
Is this problem for all links that would appear in the off-canvas dialog?
Remaining tasks
- Create Patch
- Review
If this patch gets committed before https://www.drupal.org/node/2784443 then the changes should be updated there.
If the other gets committed first the component for this issue should be change to dialong system(does that exist?)
User interface changes
Links in off-canvas dialog should be more obvious when they have focus.
Before
No outlines. when items have focus.
After
API changes
None
Data model changes
None
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | interdiff-2915759-4-11.txt | 832 bytes | samuel.mortenson |
| #11 | offcanvas-focus-2915759-11.patch | 1.82 KB | samuel.mortenson |
| #5 | offcanvas-focus-details.png | 72.07 KB | samuel.mortenson |
| #5 | offcanvas-focus-links.png | 45.75 KB | samuel.mortenson |
Comments
Comment #2
tedbow#2784443: Move off-canvas functionality from Settings tray module into drupal.dialog.ajax library so that other modules can use it was committed 🎉! So removing from settings_tray.module component.
Comment #3
xjmThis is a bug, so retitling accordingly. Thanks!
Comment #4
samuel.mortensonThis patch removes the
outline: none;rules from the offcanvas CSS, for links and detail summaries (which were also almost impossible to see focused). This should address the :focus issue.Comment #5
samuel.mortensonHere are screenshots showing what the focus for links and detail summaries look like now, previously there was no blue outline.
Comment #6
xjm(Embedding screenshots.)
Comment #7
xjmFWIW this looks great to me.
Comment #8
xjmActually, I am confident enough in this to mark it RTBC. :) Thanks @samuel.mortenson!
Comment #9
tedbowI think it does look better now but now we are inheriting the focus styles from Bartik. I think since the off-canvas dialog is suppose to have "css reset" we should explicitly declare what the focus styles should. Maybe it should be the same as Seven or Bartik but we want it to look the same no matter what the front end theme is, as much as possible.
Comment #10
yoroy CreditAttribution: yoroy at Roy Scholten for Dropsolid commentedThis darker shade of blue on focus and hover is problematic because too low contrast. We could instead not change the color but add a text-decoration: underline; a much more standard way to identify links :) The same light blue would then be used for the focus outline.
Comment #11
samuel.mortensonI looked into #9, and we're not inheriting outline styles from Bartik/Seven, but from the browser. It's generally not recommended to override the browser default outline color/size/style, but if we did add this to the CSS reset we'd need to add outline rules for every focus-able element within the sidebar, specifically to all form elements like checkboxes and text inputs. Is that in scope for this issue, or something we'd like to do?
I addressed #10 by removing the link hover/focus color and adding an underline.
Comment #12
tedbow@samuel.mortenson thanks for looking to #9. I double checked and I am seeing it coming the browser also.
#10 seems like a good idea and #11 fixes that
RTBC 🙌
Comment #13
xjmI posted #2934499: Adjust foccus styling for the Settings Tray [x] close button to match the dialog system, which is postponed on #2863354: Add border to dialog [x] close button for hover and focus states, so I think this is sufficient to resolve the most critical part of the issue.
I tested this on simplytest.me and it seems to work well. Normally we don't make changes to Stable's CSS or templates, but in this case I did commit the changes to Stable because it's a nontrivial accessibility bug.
Committed and pushed to 8.5.x. Thanks! I did not backport it because it's a small visual change including for Stable (as mentioned above).
Comment #14
xjmComment #16
joelpittetOut of curiosity, I thought we weren't touching stable, is this because it's experimental? And if it is, maybe it should even have files in the stable theme until it's released?