Skip to content
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

Provide ability to determine if collision has occurred on popover when passing in sideOffset / alignOffset #933

Closed
matteron opened this issue Nov 15, 2024 · 4 comments
Labels
enhancement An improvement to an existing feature/component

Comments

@matteron
Copy link

Describe the feature in detail (code, mocks, or screenshots encouraged)

It would be nice to have an ability to pass a callback as values for sideOffset / alignOffset to get the side that's going to be used before determine the offsets.

Something like

type SideOffset = number | (side: Side) => number;

I'm using the library somewhat unconventionally by creating a custom context menu using popovers as I want a single instance of the component at all times.

I have a popover with the content dynamically anchored to the element that is right clicked. Then manually calculate the offsets using the mouse event. This works fine until the collision detection flips the side and then the offset causes the element to be very far from the target element.

First screenshot shows the typical behavior.
Screenshot 2024-11-15 at 3 19 37 PM

Second screenshot shows the distance from the mouse when collision detection flips the side.
Screenshot 2024-11-15 at 3 19 54 PM

What type of pull request would this be?

Enhancement

Provide relevant links or additional information.

Using next branch

@huntabyte huntabyte added the enhancement An improvement to an existing feature/component label Nov 15, 2024
@huntabyte
Copy link
Owner

I think this is a reasonable request. I'll look into adding this.

@huntabyte
Copy link
Owner

huntabyte commented Nov 16, 2024

I'll need to dig further into floating UI's source code to determine how feasible this is.

My initial shot at it resulted in the floating state breaking entirely, which leads me to believe it needs to know the sideOffset/alignOffset before determining which side to move the floating content to (imagine if you had sideOffset={3000} and it wouldn't fit on the flipped side + that offset).

So it likely takes that into account when computing the side/alignment and if so, it makes this essentially impossible to execute before the flip happens and would need to be a side effect executed after the positioning likely creating a jarring UX where it quickly moves to one position and then immediately where you actually want it to go.

@matteron
Copy link
Author

So, I took a look through the context menu code and realized I was kinda going down an unnecessary complex path. I imitated what you do with having a virtualEl that's just of type Measurable and have it working much better with less code. Personally, I don't need this feature anymore, though I'm not sure whether or not to close the issue in case someone actually needs it.

@huntabyte
Copy link
Owner

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement to an existing feature/component
Projects
None yet
Development

No branches or pull requests

2 participants