[UnderlineNav] : Final implementation and UI fixes & docs improvement

[broccolinisoup]

TLDR: This PR addresses a couple of implementation issues, UI fixes as well as some documentation improvement.

---

Storybook link: https://primer-7f80a98c95-13348165.drafts.github.io/storybook/?path=/story/components-underlinenav--internal-responsive-nav
Docs link: https://primer-7f80a98c95-13348165.drafts.github.io/drafts/UnderlineNav2

---

Issues addressed in this PR:

• Render menu items as the given as prop to the UnderlineNav
• Wrap the nav with a container that overflow: hidden to prevent arrow buttons showing up when they are supposed to be hidden.
• Display loading counters for all nav items.
• Add string type to counter prop to support cases like 12K
• Remove event.preventDefault() from click and keypress handlers as UnderlineNav should support with href along with react routing configuration. When react routing is configured, it ignores the href
• Take the 4px left and right padding off from the link items and add 8px as a gap between the list items to align with design specs and take that gap value into account when calculating the possible number of items that can fit in the screen.
• UI fixes to align with design specs.

Improvements in this PR:

• Add responsive behaviour example to the documentation (Scroll behaviour for the course pointer devices)
• Add React router example to the documentation
• Some docs update in general

Merge checklist

• Added/updated tests
• Added/updated documentation
• Tested in Chrome
• Tested in Firefox
• Tested in Safari
• Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

[changeset-bot]

⚠️ No Changeset found

Latest commit: e59e212

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[github-actions]

size-limit report 📦

Path	Size
dist/browser.esm.js	77.93 KB (0%)
dist/browser.umd.js	78.58 KB (0%)

[broccolinisoup]

👋🏼 @danielguillan I have implemented the feedback you have given me here. I have a concern here for the touch target size though. I am not sure if it would be semantically correct to make the 44x44 wrapper div clickable to achieve the min touch target while keeping the button in a smaller width and height (because the focus ring should be around the button) if that makes sense. I think, the entire target touch area should be a button element. If that is the case, here how they will/should look. Unless I am missing a point here!

Rest should be fine. I would appreciate your feedback on the fixes 🙏🏼 Also just to provide a context on giving 8px gap between items and removing the 4px padding left and right, I had to slightly change the responsive calculation because the items got 8px shorter due to removing the padding and that space went into the gap. it should be working fine and should be aligned with the design specs (I hope at least.)

Looking forward to hearing your feedback 🙌🏼

[broccolinisoup]

Overflow behaviour drastically changed due to the accessibility concerns and it would have been too complex to deal with merge conflicts here. So I am closing this PR and addressed above issues separately in these PRs:

• [UnderlineNav2]: Deprecate coarse input detection and its scroll behaviour (A11y sign-off remediations) #2447
• [UnderlineNav2]: React router implementation fixes & docs improvement #2448
• [UnderlineNav2]: Add string type to the counter prop and display loading counter for all #2449