USWDS - Nav: Resolve padding overflow bug

[mahoneycm]

Summary

Fixed a bug that caused long nav button spans to overflow into its padding. Now, nav buttons properly break into two lines and maintain center alignment.

Breaking change

This is not a breaking change.

Related issue

Closes #5656

Related pull requests

#5267
Changelog →

Preview link

All headers demo →

Header storybook preview →

Problem statement

When navigation button spans cause a line break, their text flows into their padding instead of breaking onto a new line

Solution

• Set the span element to display: inline-block
• Update usa-nav__primary child item flex alignment to center

Testing and review

1. Visit the testing repo or Header storybook preview story
2. In desktop views:
   1. Confirm larger spans do not overflow into padding area
   2. Confirm icons maintain center alignment
   3. Confirm nav buttons maintain equal size
   4. Confirm appropriate default and hover states
3. In mobile views:
   1. Confirm there is no visual regression
   2. Confirm icons are vertically aligned

Testing checklist

• Long nav button spans do not break center alignment
• Buttons maintain equal size when one span breaks into two lines
• Mobile styles are unaffected
• Hover state is unaffected
• No visual regressions

Further considerations

Note

There seems to be a CSS behavior that causes a width calculation quirk and leaves empty space between the span and the icon. This behavior is present on develop.
I briefly tried resolving but didn't find any quick solutions. Do we think this is out of scope or should we dig deeper?

[mejiaj]

Looks good, thank you. Tested mobile and desktop for regressions on chromium.

[amyleadem]

@mahoneycm I noticed that changing the display to inline-block caused the nav items to change height, which results in the submenu to jump up and down if one nav item is taller than another:

I quickly played with a possible fix and found that adding the following restores the height. The following code renders the screenshot below.

.usa-nav__primary {align-items:stretch}
.usa-nav__primary button {height:100%}

It might also be worth exploring alternatives to inline-block as well.

Once this issue is addressed, I'll test some more to make sure it doesn't have a risk for inadvertent changes.

[mahoneycm]

@amyleadem Thank you so much for spotting that! Definitely an improvement and will prevent any weird click area issues.

After some testing, I noticed that the last nav item (the link) could become uncentered if another button's text breaks into three lines. This seems unlikely at the current font size but possible if a user were to override the nav link sizes.

I toyed with it for a while but was unable to get the link to maintain its vertical alignment without updating markup. I was hoping to find a solution to center the items while maintaining the full height but was unable to, unfortunately.

Do you think this is enough of an edge case to not worry about? It seems unlikely to me but I was hoping to cover all our bases.

[mahoneycm]

Im curious if it would make more sense if we moved these lines to usa-nav__primary.usa-accordion__button.span on line 300 instead of [aria-expanded]. Seems like it might be a bit more descriptive but it is further away from the other aria-expanded=true/false styles which are relevant to this work.

uswds/packages/usa-nav/src/styles/_usa-nav.scss

Lines 307 to 315 in 05e4ee2

	.usa-accordion__button {
	span {
	@include at-media($theme-header-min-width) {
	margin-right: 0;
	padding-right: units(2);
	}
	}
	}
	}

[mahoneycm]

Redundant media query

Unrelated to this work but it looks like we have a duplicate media query on line 291.

uswds/packages/usa-nav/src/styles/_usa-nav.scss

Lines 286 to 300 in 05e4ee2

	@include at-media($theme-header-min-width) {
	background-image: none;
	background-color: color("primary-darker");
	color: color("white");
	@include at-media($theme-header-min-width) {
	span {
	@include place-icon($expand-less-icon, "after");
	&::after {
	right: units(1.5);
	background-color: white;
	@media (forced-colors: active) {
	background-color: ButtonText;
	}
	}

Dismissing this review because the PR has changed.

[mejiaj]

Caution

Selectors like this are heavy handed and could cause breaking changes. Acceptable because this component heavily relies on this pattern. We should use this with a lot of caution.

[amyleadem]

Tested the following in Chrome, Safari, and Firefox at desktop widths.

• Navbar items align to vertical center when one item spans multiple lines and other items span one line. Also confirm that all navbar elements are the same height regardless of text height.
  • Test when current item is two lines, other items one line
  • Test one non-current item is two lines, others are one line
  • Test when simple link is two lines, others are one line
• Navbar items align to vertical center when all items are one line
  • No changes from develop except a small shift in icon location
• Navbar items align to vertical center when all items span multiple lines
• The code affects only the mobile presentation.
  • All changes are inside the $theme-header-min-width breakpoint
• Everything still works after pulling in USWDS - Header: Vertically center icon in nav #5654