Popper-core: computeStyle dependant on current placement

[wclr]

Is it possible somehow to have computeStyle.x to be top when popper is going to be placed on top, and bottom, when actual placement is bottom?

[FezVrasta]

May you elaborate? Sorry I'm not understanding.

[wclr]

Well you can set computeStyle.x while creation. I wonder if it is possible to reset it depending on where popper is placed while update.

The case is quite obvious:
1) Say you have popper with dynamic height.

2) Say popper is placed at bottom and you just decrease popper element height (for example by some UI actions). In this case, you actually don't have to update placement, and everything is ok.

3) Say popper is placed at top, you decrease popper element height, and poper's top will stay at is the current position, but height is decreased - so you have to update placement, to have it in the right position.

So to handle this case without hassle it would be more correct to use computeStyle.x = bottom when actual placement is bottom, and computeStyle.x = top when placement is top

[marcohamersma]

I'm trying to solve the same problem (keeping a popper with dynamic height in place after resizing), and could definitely use this as well. I initially assumed Popper would do it this by default.

[FezVrasta]

Oh, well, actually I thought the lib supported this already 😅

So you are saying that if the popper is positioned on top, it should be positioned using bottom: x instead of top: x, so that if the popper changes its size it doesn't need to recompute the position.

And I guess the same would be valid for left and right.

PRs are welcome

[marcohamersma]

Yep, exactly what I mean :)

I might take a shot at a pull request later, but I'm currently running in to a bug in popper. I'll have to make another issue ^_^.

[wclr]

so I believe should be quite simple to determine which x/y should be used depending on current placement?

  var x = placement === 'top' ? 'top' : placement === 'bottom' ? 'bottom' : options.x;
  var y = placement === 'left' ? 'right' : placement === 'right' ? 'left': options.y;

[FezVrasta]

Yes I think that should be it.

[wclr]

Though to work correctly I also needed to add

  styles[x] = '';
  styles[y] = '';

[Clairly]

@whitecolor et al, did you end up solving this? I'm having the same problem with a dynamic height popper.

[wclr]

As I posted, I use my own modified version of pooper to solve several of my cases which are not supported.

[Clairly]

Do you mind sharing where you changed the x/y computation? I poked around and couldn't find where to change it.

[wclr]

@Clairly this is a whole file with changes
https://gist.github.com/whitecolor/f59639616e031421d2a2ea9ed8b33487

[marcohamersma]

Hey all, it would be great to see this issue fixed. I managed to fix it by creating a local copy of popper.js and using the fixes @whitecolor mentioned.

I wanted to make a pull request, but realised we should probably figure out why styles[x] and styles[y] need to be set to an empty string for this to work as expected. Anyone have an idea?

If anyone's interested, here's the diff that I applied to my local file (1.14.7) to make it work.

[diondiondion]

@FezVrasta Any chance this might be added to popper.js before v2 if a PR was made for it? It seems like a relatively simple fix that has the potential to remove the need to run scheduleUpdate in many cases where it's currently necessary.

[FezVrasta]

Of course, go for it 🚀

[FezVrasta]

Hey, it looks like this was actually already implemented in v1 😂

https://popper.js.org/docs/v1/#computestylex
https://popper.js.org/docs/v1/#computestyley

We lost support for it on v2 though.

[diondiondion]

Hey, it looks like this was actually already implemented in v1 😂

Whoops, how did we miss this? Great work on the new v2 documentation by the way, it's SOO much better than the old one! Can't wait to try the new version.

We lost support for it on v2 though.

Maybe it's just me, but I don't think I'd ever need this to be a configurable property, just something that's automatically set depending on the current placement.

[FezVrasta]

Maybe it's just me, but I don't think I'd ever need this to be a configurable property, just something that's automatically set depending on the current placement.

Yeah the plan for v2 is to support it in an automatic manner.