<UnderlineNav> menu now uses the Anchored Position on smaller screen sizes to not clip, or go out of bounds.

[Rebstorm]

Closes #4059

Changelog

New

This PR fixes the issue with the <UnderlineNavMenu> on smaller screens using the menu. Where it would normally overflow to the left, this makes sure we align the absolute item using the AnchoredPosition API that you have provided.

Old Behavior:

New Behavior (321px >):

On very small screens (not supported, but hey - why not):

Changed

• Updates to menuStyles in the <UnderlineNav>
  It has become more complex, as the style has become complex as well. I am forced to check now for how many pixels we have available in the listRef. I am also using the getAnchoredPosition API now to calculate the position of the element, but only for smaller screens, as to not mess with the min-width that was previously set.

Removed

• Nothing has been removed.

Rollout strategy

• Patch release
• Minor release
• Major release; if selected, include a written rollout or migration plan
• None; if selected, include a brief description as to why

Testing & Reviewing

Merge checklist

• Added/updated tests
• Added/updated documentation
• Added/updated previews (Storybook)
• Changes are SSR compatible
• Tested in Chrome
• Tested in Firefox
• Tested in Safari
• Tested in Edge
• (GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

[changeset-bot]

🦋 Changeset detected

Latest commit: bc5a69f

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

This PR includes changesets to release 1 package

Name	Type
@primer/react	Patch

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

[Rebstorm]

Sorry 🙏 . I seem to have done a wonky-merge. Recreated the PR, feedback and conversation can be found here.

[Rebstorm]

Copy+Pasting my last message:

Woop woop! 🎉

Thank you a billion for the feedback, @broccolinisoup. It really helps with the implementation and understanding what the aim is 🙏.

Here's how I suggest we fix this:

I kind of like the margin: 0 solution which forces the menu to the right. It is simple and works. However, when the container does not offer enough room (as the more button is left-aligned), I suggest we use getAnchoredPosition like you suggested.

I've updated the code to do the following:

If we do not have enough space:

We use getAnchoredPosition to position our elements. The only real combination I found that would work would be to combine {align: 'center', side: 'outside-left'}. But if I understand the [documentation correctly](https://github.com/primer/behaviors/blob/main/docs/anchored-position.md#positionsettings-interface), that should be the right setting for popovers/menus.

If we have enough space:

Keep using right: 0 to make it right-aligned.

Again, thanks for the feedback! If I missed something, or, misunderstood something - please let me know and ill get to it 🙏

[broccolinisoup]

Hello @Rebstorm 👋🏻 Thanks so much for following up with the review comments 🙏🏻

I love your thinking on the solution! This is great. I have two main comments, one is about the position of the menu and the other is the styling performance. Please let me know if there is anything I can elaborate or if I misunderstand anything from your suggested solution. Thanks so much for your time and your contribution 🙏🏻

[Rebstorm]

To summarize the current fix, based on @broccolinisoup's feedback:

1. I moved the clientWidth calculation outside of the menuStyles to not force the getAnchoredPosition to run on each render, instead being handled as a ternary when setting the sx prop.
2. Created a constant for the min-width called baseMenuMinWidth. The baseMenuMinWidth is now referenced as the value we check, use, or test the 192px min-width. That way, we only use a single value.
3. Created a simple constant for the base menuStyles called baseMenuStyles, which contains right: '0'.
4. Simplified the test case as the menuStyles is now simpler.
5. The getAnchoredPosition now uses {align: 'start', side: 'outside-bottom'} for placement.

Again, thank you for a clear response and great feedback. This has been fun working on (and I hope it did not cause too much overhead for you), and if there's something missing - let me know!

[broccolinisoup]

Thanks so much for following up with the comments and pushing this PR to the end @Rebstorm! It looks great 🎉

Just a note: I'll hold off merging this (this is why I am commenting on the PR instead of approving) for a few days to test some action issues on the fork PRs.

Thanks again for raising this pull request and contributing to Primer 🥳 🎈

[broccolinisoup]

@joshblack I was testing #4235, it cancels the deploy preview (fork) action in this PR. Do you happen to know why that is failing to run? Also what is the best way you recommend to debug cancelled actions? I usually have a hard time debugging them, well, because there is not much proper logs 🫠

[broccolinisoup]

Hi @Rebstorm 👋🏻 Apologise for the delay here. I am going ahead merging the PR 🎉

Thanks so much again for your contribution 🙏🏻