shepherd icon indicating copy to clipboard operation
shepherd copied to clipboard

Can't use FloutingUI autoPlacement

Open sirmspencer opened this issue 1 year ago • 8 comments

Shepherd adds flip and shift to default options, then does a deep merge with any other options passed in. Flip and autoPlacement are not compatible so adding autoPlacement to floatingUIOptions.middleware just causes errors.

I tried not passing in a placement so shouldCenter is true, but there are side effects in other parts of the code that block floating UI all together.

You could check for something like :on "auto" to skip adding flip. autoPlacement doesn't need a placement so you can skip options.placement = attachToOptions.on; too.

https://github.com/shepherd-pro/shepherd/blob/master/src/js/utils/floating-ui.js#L169

sirmspencer avatar Jan 23 '24 01:01 sirmspencer

Same here, version 12.0.1, if I pass on: 'auto' the step is positioned weirdly.

Popper.js Floating UI
Screenshot 2024-05-14 at 12 04 46 Screenshot 2024-05-14 at 12 04 49

Another problem is with passing offset middleware, when clicking between steps, the step jumps to its offset. Not sure if tied to the same problem

https://github.com/shepherd-pro/shepherd/assets/72975/23be6587-3eb0-43e2-9b81-56e849b7b60b

patrikholcak avatar May 14 '24 10:05 patrikholcak

@patrikholcak 'auto' isn't an option for position, we just use the as noted by FUI

export type PopperPlacement =
  | 'top'
  | 'top-start'
  | 'top-end'
  | 'bottom'
  | 'bottom-start'
  | 'bottom-end'
  | 'right'
  | 'right-start'
  | 'right-end'
  | 'left'
  | 'left-start'
  | 'left-end';

Sounds like what is asked by @sirmspencer is an option to have 'auto' and then use autoPlacement middleware and omit flip.

chuckcarpenter avatar Jun 27 '24 23:06 chuckcarpenter

Right, so should I open another issue for my problem? I can’t upgrade because of this. 😞

patrikholcak avatar Jun 28 '24 07:06 patrikholcak

@patrikholcak 'auto' isn't an option for position, we just use the as noted by FUI

export type PopperPlacement =
  | 'top'
  | 'top-start'
  | 'top-end'
  | 'bottom'
  | 'bottom-start'
  | 'bottom-end'
  | 'right'
  | 'right-start'
  | 'right-end'
  | 'left'
  | 'left-start'
  | 'left-end';

Sounds like what is asked by @sirmspencer is an option to have 'auto' and then use autoPlacement middleware and omit flip.

I think so. Its been 6 months so id have to look a little deeper again.

sirmspencer avatar Jul 02 '24 04:07 sirmspencer

@patrikholcak @sirmspencer we'd be happy to also take a PR to prioritize this if you're up for it?

chuckcarpenter avatar Jul 04 '24 06:07 chuckcarpenter

It seems like you both had a good handle on what you wanted and where to insert it into the library, so a PR would be great if you have the time!

RobbieTheWagner avatar Oct 08 '24 20:10 RobbieTheWagner

@sirmspencer @patrikholcak we changed the ordering of the merging for the floatingUIOptions. Could you please see if things work better for you now?

RobbieTheWagner avatar Oct 11 '24 17:10 RobbieTheWagner

:wave: I just opened a PR for auto positions support #3009

patrikholcak avatar Oct 11 '24 17:10 patrikholcak

I'll take a look, thanks!

sirmspencer avatar Oct 31 '24 23:10 sirmspencer

Closed by https://github.com/shipshapecode/shepherd/pull/3009

RobbieTheWagner avatar Nov 01 '24 17:11 RobbieTheWagner