-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat(popover): use floating-ui for autoAlign #14654
feat(popover): use floating-ui for autoAlign #14654
Conversation
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
…add autoUpdate, add arrow, add offset
… logical property
…493-floating-ui-poc
✅ Deploy Preview for v11-carbon-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
…493-floating-ui-poc
…493-floating-ui-poc
…493-floating-ui-poc
@tay1orjones All good with the changes in the |
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.
This looks awesome! I noticed a small issue with the Slug
storybook alignments using the old values, but I will create a separate issue for that and tackle that with the upcoming Slug
changes.
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.
🔥🚀 Awesome, LGTM!! (other than a couple merge conflicts)
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.
Looks Awesome @tay1orjones ! 🌟 Just two comments:
Tab tip experimental auto align story
The example of the popover with no caret should be sitting directly underneath the icon box with no space between them.
Playground story
Align prop: When the caret is set to to true, there is no caret present in the popover for right-top
alignment. All other alignments with caret and without caret look great!
@laurenmrice Thank you for catching both of those! They should be fixed now. |
Toggletip Experimental Auto Align story
|
@laurenmrice Thanks! I just pushed up a commit that fixes that issue. I also removed the autoalign story from VRT for now because
I added the popover playground story to vrt instead. |
afce28d
) * WIP: add floating-ui to popover * WIP: use scss for align positioning, update CSSOM styling, add flip, add autoUpdate, add arrow, add offset * wip(popover): explore logical properties support, popover offset from logical property * WIP * WIP: add fixed strategy, remove default popover styling when autoAlign * WIP: properly set refs on reference element and floating element * WIP * wip(popover): correct caret placement * wip(popover): update styles to support new alignment names * feat(popover): cleanup autoalign implementation/styles * fix(popover): improve autoAlign * fix(popover): add forwardRef to ToggletipButton * fix(popover): improve autoAlign compatibility with slug * fix(popover): position absolute is not necessary on the container * fix(slug): scope popover selectors for autoalign functionality * chore: yarn dedupe * chore(popover): remove console logs * fix(popover): improve tests, cloneElement logic * fix(popover): update align props across the entire system * chore(popover): fix typescript issues * fix(popover): ensure component api table in docs works * chore(popover): typescript fixes * fix(popover): ensure autoAlign properly pads out the slug caret * fix(popover): typescript error * fix(popover): remove offset when isTabTip * fix(popover): correct caret style typo * docs(popover): clean up stories * test(popover): update story name * chore: fix toggletip story width, vrt popover playground --------- Co-authored-by: Andrea N. Cardona <cardona.n.andrea@gmail.com>
Closes #14493
This PR enhances Popover's experimental
autoAlign
functionality to automatically position the floating element to be within view, even on scroll.This uses
floating-ui
and relies solely on thefixed
strategy, usingposition: fixed
on the floating element. In the majority of cases, Popover floating elements will no longer be clipped by parents withoverflow: hidden
.This implementation requires two important changes I'd like to highlight:
.#{$prefix}--popover-container
must not haveposition: relative
Changelog
Changed
offset
, caret size, etc) when possible.autoAlign={true}
autoAlign
Removed
Testing / Reviewing