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

Fix: FloatingUI arrow() padding #2315

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

lilyMrt
Copy link

@lilyMrt lilyMrt commented Apr 4, 2023

Related to #2302

Arrow function passed through a step's middleware will have a null element, in which case the padding will not be applied. Sum up all such occurrences and pass the final padding to the arrow function with the newly added arrow element.

It allows arrow() passed from step middleware to actually be applied. Perhaps it could be cut down and done through a function passed to merge(), but I'm not familiar with that library.

Can write some tests for it, though I think this more of a bug fix for behavior that should already be working than new functionality. If not, I think the docs should be updated to specify which floatingUI options/middleware functions are accepted/work.

Related to shepherd-pro#2302
Arrow function passed through a step's middleware will have a null element, in which case the padding will not be applied. Sum up all such occurrences and pass the final padding to the arrow function with the newly added arrow element.
@RobbieTheWagner
Copy link
Member

@lilyMrt I appreciate you attempting to tackle this issue! I noticed something when debugging this. It seems if I resize my window, which triggers FloatingUI's autoUpdate, the arrow fixes itself and moves to the right place. I think it's a timing issue, where we need to make sure the tooltip is rendered before we call autoUpdate.

@lilyMrt
Copy link
Author

lilyMrt commented Apr 11, 2023

@RobbieTheWagner Would it be possible to include some example steps using floatingUIOptions in the welcome.js demo site? It's particularly hard to get started with these new options and identify bugs when the source itself does not include any working examples using any of the floating UI functions.
But I think you are correct, autoUpdate is being called too soon, and a few settings are not applied to the step until the page is resized.

@mrceperka
Copy link

@RobbieTheWagner Same issue is happening to me right now. Any ideas how to force autoUpdate on demand?

The worst thing is, that sometimes arrow is placed correctly. But most of the times it is wrong.
Packages: [email protected], [email protected]

Video with the arrow issue.
shepherd-arrow-bug.webm

@chuckcarpenter
Copy link
Contributor

@mrceperka I've noticed that myself recently as well. React package will get whatever bugs are in this main one, for sure. I'll take a look some early this week to see if I can incorporate this fix.

@mrceperka
Copy link

Thanks for the reply @chuckcarpenter. In the end I had to use completely different package so this is no longer bugging me 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants