Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

fix(Popper): improve positioning in multiple cases#2187

Merged
layershifter merged 8 commits intomasterfrom
docs/update-examples
Dec 19, 2019
Merged

fix(Popper): improve positioning in multiple cases#2187
layershifter merged 8 commits intomasterfrom
docs/update-examples

Conversation

@layershifter
Copy link
Member

@layershifter layershifter commented Dec 16, 2019

Fixes #2064.
Fixes #2072.

Case 1: jump on focus

The same as mui/material-ui#16740, when opening a Popup with an autofocus input inside it, should not cause the page to jump (scroll).

Case 2: scrollbar sizing issues

The situation arises when an element on the page has as height the entire viewport (100vh). Imagine the following DOM:

What happens is that right before the computation of the position of the popper, the popper gets position: absolute. However, since the popper appears after the page in the DOM, it will be positioned after it as well. Since the height of the page then becomes 100vh + height(popper), the scrollbar appears.


See code comments for descriptions and linked issues.

@layershifter layershifter changed the title Docs/update examples fix(Popper): wip Dec 16, 2019
@DustyTheBot
Copy link
Collaborator

DustyTheBot commented Dec 16, 2019

Perf comparision

Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
Button.Fluent 1.22 0.13 9.38:1 5000 6076
Checkbox.Fluent 1.4 0.24 5.83:1 5000 6993
Icon.Fluent 0.25 0.03 8.33:1 5000 1239
Slider.Fluent 1.76 0.25 7.04:1 5000 8779

Generated by 🚫 dangerJS

@layershifter layershifter changed the title fix(Popper): wip fix(Popper): improve positioning during first render Dec 19, 2019
@layershifter layershifter added 🚀 ready for review 🧰 fix Introduces fix for broken behavior. labels Dec 19, 2019
})

const originalUpdate = instance.update
instance.update = () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the PR title says 'first render'? This fixes resizing after the first render.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated 👍

@layershifter layershifter changed the title fix(Popper): improve positioning during first render fix(Popper): improve positioning in multiple cases Dec 19, 2019
@layershifter layershifter merged commit f446eff into master Dec 19, 2019
@layershifter layershifter deleted the docs/update-examples branch December 19, 2019 14:02
@oliviertassinari
Copy link

😆

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

🧰 fix Introduces fix for broken behavior. 🚀 ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect popup placement after jump-resize Popup clipping in RTL

4 participants