Skip to content

Fix focus link colour for mobile menu#314

Merged
finnlewis merged 3 commits into2.xfrom
feature/306-menu-focus-link
Jun 3, 2025
Merged

Fix focus link colour for mobile menu#314
finnlewis merged 3 commits into2.xfrom
feature/306-menu-focus-link

Conversation

@markconroy
Copy link
Member

Closes #306

Sets focus colour for mobile menu items to dark (on yellow) instead of white (on yellow).

Copy link
Member

@msayoung msayoung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does fix the focus, but it's now black on black on hover.
I think we need to set a specfiic focus colour alongside the focus background colour rather than reusing the hover variable?

@markconroy
Copy link
Member Author

Whaaaaat? How did I miss that. I'll get it sorted.

@markconroy
Copy link
Member Author

Ready again @msayoung

@msayoung msayoung self-requested a review April 7, 2025 11:56
Copy link
Member

@msayoung msayoung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better, but the text is now white on yellow when focus and hover are applied.

@markconroy
Copy link
Member Author

@msayoung I'm not sure this is fixable.

I'm not aware of a CSS property for "do this when hovered and focussed at the same time", I think we've only got :hover and :focus, unless we put !important perhaps on the :hover state?

@msayoung
Copy link
Member

@markconroy this should do it:

.off-canvas a:focus,
.off-canvas a:focus:hover {
  color: var(--off-canvas-link-focus-color);
}

.off-canvas a:hover {
  color: var(--off-canvas-link-hover-color);
}

@markconroy
Copy link
Member Author

I'm about to test this, and then go a a bottle of champagne if it works. I never even thought of the idea of :focus:hover, sooo clever.

@markconroy
Copy link
Member Author

@msayoung You're a genius. It works. Let's get this approved

@msayoung msayoung self-requested a review June 3, 2025 11:24
@finnlewis finnlewis merged commit ed2a0dc into 2.x Jun 3, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Focus appearance / text contrast in the mobile menu

3 participants