UnderlineNav :focus styles

[langermank]

In testing global focus styles #1744 we've discovered that UnderlineNav overrides outline styles for focus, and also is structured in such a way that the default focus outline will appear cutoff. This PR makes some style changes to improve how each nav item behaves, which is really only noticeable on hover or focus.

The goal with this is to not introduce any breaking changes (no markup changes) and keep the sizing as close to current prod as possible.

• Adds a height declaration to .UnderlineNav instead of relying on .UnderlineNav-item to provide height with padding
• Introduces a new mixin that extends the touch target which visually allows .UnderlineNav-item to be smaller. This creates more room for a focus outline
• Minor spacing change for icons/counters inline with nav labels

I also added the focus styles directly to UnderlineNav which can be removed when global focus styles are introduced. I can remove that if we prefer to just use this PR as prep for global focus.

⚠️ Followup PR in dotcom to remove UnderlineNav hacks https://github.com/github/primer/issues/513

/cc @primer/css-reviewers

[changeset-bot]

🦋 Changeset detected

Latest commit: 759a925

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/css	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[langermank]

@simurai I'd like to deploy this to a lab one more time before merging, but for some reason the version release number is missing from the checks 🤔 I hoped maybe it would remain the same and a re-deploy to the lab would work, but no such luck. Any ideas?

[simurai]

for some reason the version release number is missing from the checks

Hmm.. weird.

I hoped maybe it would remain the same and a re-deploy to the lab would work, but no such luck.

Yeah, I think every push generates a new npm version. So that would need to be updated on dotcom again.

[simurai]

Ok, I merged another PR and then updated this branch. Which seemed to kick off another "Release / Canary (push)". New Canary version is:

0.0.0-20211130510

No idea what caused this hiccup? Maybe related to #1805. 🤷

[langermank]

Thanks @simurai! I'm planning to deploy late on Monday so we can have some overlap testing time 😄 will let you know!

[mperrotti]

I like your hover state treatment because it matches what ActionList does when we use it like vertical tabs for navigation. Great attention to detail 👏

[mperrotti]

I also added the focus styles directly to UnderlineNav which can be removed when global focus styles are introduced. I can remove that if we prefer to just use this PR as prep for global focus.

I think it's safer to leave the focus styles in until we do the global focus styles.

[langermank]

@simurai & co, here's the latest lab deploy (will expire in 4 hours) https://primer-underlinenav-test-2.review-lab.github.com/

[simurai]

here's the latest lab deploy (will expire in 4 hours) https://primer-underlinenav-test-2.review-lab.github.com/

Gave it a quick try. Haven't noticed anything unusual, except that the org nav item's height is different.

But I think that will be fixed by removing the hx_UnderlineNav class. I usually add a ⚠️ note in the PR description so that we don't forget it when updating dotcom with the PCSS version.