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

Refactor back to Popper #744

Closed
RobbieTheWagner opened this issue Dec 30, 2019 · 13 comments
Closed

Refactor back to Popper #744

RobbieTheWagner opened this issue Dec 30, 2019 · 13 comments

Comments

@RobbieTheWagner
Copy link
Member

There is a Popper beta in the works that is supposedly very small and performant. We should give it a try and see if it fixes some of the issues we are having with Tether.

@FezVrasta
Copy link

FYI yesterday we released Popper 2 (stable)

@chuckcarpenter
Copy link
Member

@FezVrasta having a tough time getting this completely working w/ v2. My main issue is the window scrolls back to top and then to the target. It seems that the position:absolute is applied first and then the calculations for the transform position.

image

And then

image

I imagine I need to leverage a modifier here, but not clear where to start. If you have any ideas I'd appreciate it!

@FezVrasta
Copy link

Have you tried to set the position: fixed; top: 0; left: 0; as default style for the elements? This should prevent the page from scrolling automatically up.

Alternatively you may disable any focusable element inside the popper before popper positioned it

@chuckcarpenter
Copy link
Member

@FezVrasta would I do that by changing applyStyles modifier? Seems like that's where the initialStyles values are that get written inline first. I tried creating a custom one beforeWrite that sets state.elements.popper.style, but doesn't seem to change the default value.

@chuckcarpenter
Copy link
Member

A little more context, applying default styles on the element doesn't work adding in a custom modifier to set values isn't working either. If I change the applyStyles effects function for the initial values, then it works. Is that my only option @FezVrasta?

function effect(_ref2) {
  var state = _ref2.state;
  var initialStyles = {
    position: 'fixed',
    left: '0',
    top: '0',
    margin: '0'
  };

...

@chuckcarpenter
Copy link
Member

This fixes it, but would be great to know if there's a less intrusive path. #801

@FezVrasta
Copy link

I see, probably we should do that in Popper, would you mind opening an issue on the repo?

@atomiks
Copy link

atomiks commented Feb 18, 2020

Does the scroll problem happen on Firefox or Chrome 79? It's come up twice recently (including Popper v1) which makes me think Chrome 80 did something?

@RobbieTheWagner
Copy link
Member Author

@atomiks Chrome 80 broke array reduce I think. Not sure if that applies here https://www.reddit.com/r/webdev/comments/f26mbn/chrome_80_introduces_a_new_bug/

@atomiks
Copy link

atomiks commented Feb 18, 2020

We use reduce everywhere in Popper 😱 but yeah I don't think that would be related. It seems like it was reported back in mid 2019 so I guess not...

@RobbieTheWagner
Copy link
Member Author

@atomiks the bug effects Chrome 80-82 I believe. We hit errors with reduce just the other day, definitely still an unresolved bug in Chrome.

@chuckcarpenter
Copy link
Member

@atomiks I can confirm this is happening in FF 71.

@RobbieTheWagner
Copy link
Member Author

RobbieTheWagner commented Feb 20, 2020

We've got popper v2 fully working now I believe, so closing this. Thanks everyone!

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

No branches or pull requests

4 participants