Problem/Motivation

The color contrast for the skip to main content display is too low for the hover as well as the active state and breaks SC 1.4.3

focus state (color contrast of 9.73:1)
https://contrast-ratio.com/#%23fff-on-%23444
skip to main content widget when in focus

hover state (color contrast of 1.0029790028808814:1)
https://contrast-ratio.com/#%230036b1-on-%23444
skip to main content widget on hover

active state (color contrast of 1.11:1)
https://contrast-ratio.com/#%2300339a-on-%23444
skip to main content widget when in active state

Issue fork drupal-3273056

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:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rkoller created an issue. See original summary.

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

kmonahan’s picture

Status: Active » Needs review
FileSize
10.47 KB
10.59 KB
11.03 KB

Opened https://git.drupalcode.org/project/drupal/-/merge_requests/2105

After changes applied:
Focus
Skip to main content widget when in focus with MR changes

Hover
Skip to main content widget on hover with MR changes

Active
Skip to main content widget when active with MR changes

ckrina’s picture

Status: Needs review » Needs work

Thanks @kmonahan! I've left a small feedback regarding how you defined the colors that applies to your code, but if you could update that too on the default .skip-link state colors too (line 13 and 15 in skip-link.pcss.css) it would great.
I'd just choose the closer color from the gray scale for grays.

Johnny Santos’s picture

Assigned: Unassigned » Johnny Santos

Maybe I can give it a try?
I hope its ok

Johnny Santos’s picture

Assigned: Johnny Santos » Unassigned
Status: Needs work » Needs review

There it goes as requested on comment #5 and #6

mherchel’s picture

Status: Needs review » Needs work

@Johnny Santos Thanks for the work.

The changes have a couple problems:

1) It removes the non-hover background styling
2) Per @ckrina comment, you should be using the CSS variables to color the background, not a one-off hex value.
3) The compiled output doesn't match what you uploaded. This means that you forgot to re-compile the CSS one last time. This is why it says "Custom Commands Failed"

kmonahan’s picture

Status: Needs work » Needs review
FileSize
18.94 KB
19.05 KB
19.16 KB

Added to @Johnny Santos's update to use Claro variables for all colors and build the CSS.

Updated screenshots:
Focus
skip to main content widget when in focus

Hover
skip to main content widget when hovered over

Active
skip to main content widget when active

mherchel’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
7.51 KB

Latest changes look perfect. Tested focus, hover, and active states.

mherchel’s picture

Attached is the Drupal 10 version of the same fix.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: 3273056-11-10.0.x.patch, failed testing. View results

mherchel’s picture

Status: Needs work » Reviewed & tested by the community

Unrelated failure.

  • lauriii committed b710d3f on 9.4.x
    Issue #3273056 by kmonahan, mherchel, Johnny Santos, rkoller, ckrina:...

  • lauriii committed ac13dba on 9.3.x
    Issue #3273056 by kmonahan, mherchel, Johnny Santos, rkoller, ckrina:...
lauriii’s picture

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

Committed c3a73e9 and pushed to 10.0.x. Committed MR to 9.4.x and cherry-picked to 9.3.x. Thanks!

  • lauriii committed c3a73e9 on 10.0.x
    Issue #3273056 by kmonahan, mherchel, Johnny Santos, rkoller, ckrina:...

Status: Fixed » Closed (fixed)

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