Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Navigation: Update focus style, Get WordPress button mobile style #373

Merged
merged 3 commits into from
Mar 29, 2023

Conversation

ryelle
Copy link
Contributor

@ryelle ryelle commented Mar 28, 2023

Fixes #372, fixes #363. This updates the focus style across the header, and removes the special styling on the mobile "Get WordPress" button.

I'm using the "current item" color for the focus state, which is --wp--preset--color--blueberry-2 on the black background, --wp--preset--color--deep-blueberry on the white background, and --wp--preset--color--white on the blue background.

Default Blue bg White bg
Top level item focus branch-default-1-large-menu-focus branch-blue-1-large-menu-focus branch-white-1-large-menu-focus
Menu open, submenu item focus branch-default-2-large-menu-open branch-blue-2-large-menu-open branch-white-2-large-menu-open
Get WordPress focus branch-default-3-large-button-focus branch-blue-3-large-button-focus branch-white-3-large-button-focus
Small screen, logo focus branch-default-4-small-logo-focus branch-blue-4-small-logo-focus branch-white-4-small-logo-focus
Small screen, menu open, top level item focus branch-default-5-small-menu-top-level branch-blue-5-small-menu-top-level branch-white-5-small-menu-top-level
Small screen, menu open, submenu item focus branch-default-6-small-menu-2nd-level branch-blue-6-small-menu-2nd-level branch-white-6-small-menu-2nd-level

The spacing around the small screen menu items have also been tweaked, but I've kept the highlight at the edge of the screen

@StevenDufresne
Copy link
Contributor

I don't think i see another issue for this, but can we address the fact that the focus indicator is cut off and not aligned on the bottom (off by a couple pixels?):

Screenshot 2023-03-29 at 10 31 26 AM

Seems to be related to a border bottom on the li.

I would also prefer if the outline was closer to the text, but that could probably be done later.

@jasmussen
Copy link
Collaborator

Thanks for the PR, and excellent catch Steve! The box shadow needs to be inset, that also corrects the border radius. So instead of:

box-shadow: 0 0 0 1.5px var(--wp-global-header--link-color--active);

it should be

box-shadow: inset 0 0 0 1.5px var(--wp-global-header--link-color--active);

That should fix the crop.

The exception to this is the button, which would loose accessible contrast if the border was inset.
@ryelle
Copy link
Contributor Author

ryelle commented Mar 29, 2023

I've switched the box-shadow to inset, so it's not cut off anymore:

Screenshot 2023-03-29 at 4 54 41 PM

Will merge this now to keep things moving, but if we need to iterate more we can.

@ryelle ryelle merged commit b32eb9e into trunk Mar 29, 2023
@ryelle ryelle deleted the update/header-menu-focus branch March 29, 2023 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
3 participants