shipshapecode / shepherd

Guide your users through a tour of your app
https://shepherdjs.dev
Other
13.05k stars 646 forks source link

Arrow position not working when using custom FloatingUI options #3034

Open lalpou opened 1 month ago

lalpou commented 1 month ago

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 commented 1 month ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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

earlwilson commented 2 weeks ago

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 commented 2 weeks ago

@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?