Popper-core: `position` still applied when `applyStyle` modifier is disabled

Created on 15 Jun 2017  ·  10Comments  ·  Source: popperjs/popper-core

Hi @FezVrasta

When we disable applyStyle modifier it still append inline style (position: absolute)

CodePen demo

https://codepen.io/Johann-S/pen/jwVLGG

Steps to reproduce the problem

  1. Inspect the pop element and you'll see inline style

What is the expected behavior?

Pop element shouldn't have inline style

# BUG 🐞 low medium

Most helpful comment

The first adds more DOM manipulations, which may lead to worse performance, also, it doesn't give full control over the DOM manipulation to the consumer... I'm more for the second option, mostly because, so far, Popper.js always preferred verbose but powerful configurations over "easy" but limiting ones.

I'd prefer the 3rd party wrappers to make the API simple to use, I hope it makes sense.

All 10 comments

So... We need to apply the position to the popper element even if the applyStyle is disabled because without the proper position there's no way to properly compute the right offsets.

A possible solution could be to add an option to disable this behavior, defined outside from the applyStyle modifier options.

Thinking about your specific use case, I think you could directly avoid to initialize the popper if you will never need it on that specific element, what do you think?

An alternative solution to implement on Poppe.rjs side could be to:

  1. Apply the position
  2. Compute the offsets
  3. Remove the position if applyStyle is disabled.

/cc @souporserious, I'd like your feedback on this matter because it interests 3rd party integrations

Thinking about your specific use case, I think you could directly avoid to initialize the popper if you will never need it on that specific element, what do you think?

We cannot be sure if we never need Popper.js in this particular use case because we cannot predict what folks will do. That's why we have an update method in our Dropdown plugin.

But we can let this in this state (in Bootstrap) and update later 👍

I like the idea of applying, computing, then removing. It'd be awesome to not have to add to the API surface :)

@souporserious wouldn't make sense to let the user code manage even this DOM manipulation to get rid of any DOM manipulation performed by Popper.js?

Ah true. Shouldn't it be doable with the new stuff you just did? Since I'm not setting position and transforms anymore, could you take the value calculate it and then give it back so I can perform the DOM manipulation on my end? Maybe I'm misunderstanding 🤔

All this happens within a single update cycle, so you couldn't use the update hook to link React. Not sure if I'm clear...

Hmm I'm not sure then ¯_(ツ)_/¯. I think what you said above sounds good. Maybe I need to see an example to understand better.

So, right now, the situation is as follows:

  1. position: absolute is set on init
  2. Update lifecycle starts
  3. Modifiers compute the position of the popper
  4. applyStyle modifier edits the popper node to apply the computed styles
  5. go to 2 (on new update)

With the 3rd approach I proposed we'd have:

  1. Update lifecycle starts
  2. position: absolute is set
  3. Modifiers compute the position of the popper
  4. applyStyle modifier edits the popper node to apply the computed styles
  5. position: absolute is removed if applyStyle is enabled
  6. go to 1 (on new update)

This approach makes Popper perform 2 additional DOM write accesses on each update, and prevents 3rd party wrappers to completely control the DOM manipulation. The good thing is that, it works without much troubles.

The alternative process I propose is:

  1. position: absolute is set if applyStyle is enabled
  2. Update lifecycle starts
  3. Modifiers compute the position of the popper
  4. applyStyle modifier edits the popper node to apply the computed styles
  5. go to 1 (on new update)

This doesn't set any position to the popper if applyStyle isn't enabled, because it expects the consumer to properly set the position using the wrapper library he's writing.

In the React use case, you'd set position: absolute on any popper element by default.

Does it make sense?

Makes way more sense, thanks for clarifying! :)

I'm not too concerned with either approach. They both sound great in my opinion. If the first doesn't seem to add any downside, but could be a better dev experience I vote for that :)

The first adds more DOM manipulations, which may lead to worse performance, also, it doesn't give full control over the DOM manipulation to the consumer... I'm more for the second option, mostly because, so far, Popper.js always preferred verbose but powerful configurations over "easy" but limiting ones.

I'd prefer the 3rd party wrappers to make the API simple to use, I hope it makes sense.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

kerf007 picture kerf007  ·  3Comments

nainardev picture nainardev  ·  3Comments

Madhu94 picture Madhu94  ·  3Comments

tyrw picture tyrw  ·  4Comments

FezVrasta picture FezVrasta  ·  3Comments