fix(Popup) popup appearing in wrong position when open prop is set #2775
Conversation
cdaringe
changed the title
Codecov Report
@@ Coverage Diff @@
## master #2775 +/- ##
==========================================
+ Coverage 99.74% 99.74% +<.01%
==========================================
Files 160 160
Lines 2762 2771 +9
==========================================
+ Hits 2755 2764 +9
Misses 7 7
Continue to review full report at Codecov.
|
…to popup-open-prop-fix
cdaringe
force-pushed
the
popup-open-prop-fix
branch
from
866bb3c to
c822956
Compare
| this.setPopupStyle(this.props.position) | ||
| } | ||
|
|
||
| this.animationRequestId = requestAnimationFrame(this.setPosition) |
There was a problem hiding this comment.
haven't you introduced an infinite loop now? When does this ever get short circuited?
There was a problem hiding this comment.
good point. fixed in 7c2104f
There was a problem hiding this comment.
Unfortunately the infinite loop is still there - all the time the popup is displayed setPosition() is called in a loop, it then calls setPopupStyle which re-sets component's state which causes re-render - takes ~20% CPU when the popup is displayed.
|
hi all, kind and cordial bump! |
|
Such a gentleman 😉 reviewing! |
|
Just merged master, we can ship this once tests pass again. |
|
codecov seems to be all hung up! |
|
Congrats on merging your first pull request! 🎉🎉🎉 |
|
Released in |
|
nice! ps, if you havent seen already, the latest version of semantic-release auto posts back into PRs what versions PRs get released in. so great! |
|
With this commit, scrolling the window causes very strange behavior.
|
problem
solution
<Popup />correctly attached to the trigger if the trigger movesdemo
here i use
<Popup />for form validation.with fix, the
<Popup />immediately renders when the field is dirty and empty:without fix. the
<Popup />is out of the viewport. I have to fidget with thetrigger={<Input />}to get the Popup to eventually appear where it has been commanded to go: