-
Notifications
You must be signed in to change notification settings - Fork 353
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
fix(Popper): fixed flicker when opening popper content #9339
Conversation
Preview: https://patternfly-react-pr-9339.surge.sh A11y report: https://patternfly-react-pr-9339-a11y.surge.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing I'm noticing is that for Popover, there's now a noticeable delay when the Popover closes and the trigger receives focus again:
Popover.trigger.focus.delay.mov
Compared to the current behavior where the trigger receives focus fairly instantaneously. That's the only thing I think that's potentially blocking, just because we're swapping one visual bug with another (this might depend whether it makes sense for there to be this delay in the first place)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 🎉 I'm not noticing any flickering when checking Popover, Tooltip, or a random Page demo that has a username dropdown. Previous comment about the trigger focus delay is also resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
* fix(Popper): fixed flicker when opening popper content * fixed re-focus delay when closing popover * updated DatePicker test to have consistent value
What: Closes #9119
Additional issues:
Essentially this fixes the flicker by having the opacity change that makes the Popper content visible to users happen after Popper has initially rendered and figured out where it should be placed.
In accomplishing that goal I moved the largely identical opacity change logic in Tooltip/Popover directly into Popper.