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

Arrow position not working when using custom FloatingUI options #3034

Open
lalpou opened this issue Oct 21, 2024 · 6 comments
Open

Arrow position not working when using custom FloatingUI options #3034

lalpou opened this issue Oct 21, 2024 · 6 comments
Labels

Comments

@lalpou
Copy link

lalpou commented Oct 21, 2024

I'm using the following custom FloatingUI options in the "defaultStepOptions":

const shepherdTour = new Shepherd.Tour({
    steps: tourOptions.steps,
    keyboardNavigation: true,
    defaultStepOptions: {
	    scrollTo: { behavior: 'smooth', block: 'center' },
	    floatingUIOptions: {
		    middleware: [shift({ padding: 32 }), offset(16)]
	    }
    }
});

When the position of the tooltip is calculated, the middleware of the arrow and the shift options is applied in the following order:

[flip, shift, arrow, shift, offset] // The first 3 are from internal Shepherd.js settings

This causes an issue with how the arrow is placed, since the arrow middleware should be last in the order.

I've attempted to fix this using the beforeShowPromise and the whenShow functions, but run into issues with applying the arrow middleware in those places. In the beforeShowPromise the step element hasn't been generated yet, meaning the arrow can't be targeted and in the whenShow function the computePosition function for FloatingUI has already been run.

How can I apply shift with padding while still achieving correct arrow placement?

@lalpou
Copy link
Author

lalpou commented Oct 22, 2024

Reverting back to before the changes made in #3005 fixes the problem. I guess some of the default middleware should still override the user set ones (such as the arrow)

@RobbieTheWagner
Copy link
Member

Thanks for letting us know @lalpou. @chuckcarpenter I wonder if we need to not just merge the options but like merge some and push some onto the end etc?

@chuckcarpenter
Copy link
Member

@RobbieTheWagner I think it's worth testing an append of arrow last, unless there's a user supplied one

@earlwilson
Copy link

I am also running into this issue where doing a shift moved the arrow instead of keeping the arrow centered on the attachTo element. Reverting to npm version 12.0.6 fixed it and it now correctly keeps the arrow centered even if the tooltip position is shifted via FloatingUI Options.

@RobbieTheWagner Any idea if there are plans to fix this in the near future?

@chuckcarpenter
Copy link
Member

@earlwilson thanks for verifying. It's our intent to add a test and apply a fix, but we've both not had the time to dive in quite yet. We will do that as soon as we can, but would also welcome a PR if you think you could tackle it?

@chuckcarpenter
Copy link
Member

@lalpou apologies it's been awhile, but I'm actually having a hard time replicating. Do you have somewhere we can see an example? Even with arrow in the middle it seems to take the correct placement. The only thing I have different here is a reset for dialog b/c that does some weird stuff with margin.

dialog.shepherd-element {
    padding: 0;
    border: none;
    margin: 0;
    background: none;
    position: static;
    width: auto;
    height: auto;
    color: inherit;
    font: inherit;
  }
image

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

No branches or pull requests

4 participants