-
Notifications
You must be signed in to change notification settings - Fork 538
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
UnderlineNav: Use anchoredPosition to position overflow menu #4414
Conversation
🦋 Changeset detectedLatest commit: 6555591 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
size-limit report 📦
|
Closes #4059, I think, if that would be helpful to add to the PR description 👀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @siddharthkp! I apologise not responding to the issue yesterday before you pushed this PR 😢
We already have an open PR #4234 to fix this and I gave feedback as well. I had issues with the CI jobs and didn't have a chance to get back to it so haven't merged that yet.
My main feedback was around when to use anchor positioning. If I am not underestimating the use cases, we don't actually need anchor positioning until we are short on space and that way, we don't have to calculate the menu position in every render when there is a resize.
Again sorry for that it is my bad that we ended up with two PRs for the same issue 😭 Would you be able to have a look at #4234 and share if you have any feedback?
Closing as duplicate of #4234 🎉 |
Instead of keeping the position fixed, adjust the position based on the space available (by using anchoredPosition)
Prefers aligning left towards the other link, but switches to right when there is not enough space:
underlinenav-anchored-overflow.mov
Rollout strategy
Testing & Reviewing
Merge checklist