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
Disable Popover GPU acceleration on Windows. #179
Disable Popover GPU acceleration on Windows. #179
Conversation
components/popover/popover-base.jsx
Outdated
styleOverrides: {}, | ||
}; | ||
|
||
state = { | ||
showPopper: this.props.isOpen || false, | ||
popperModifiers: this.props.modifiers, |
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 realize that copying props into state is almost never a good idea.
I don't expect consumers to change this modifiers
prop, are we comfortable with this tradeoff, and does it need to be explicitly called out in a comment or something?
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.
Why is it not expected that consumers will change the modifiers
prop? And if it's not expected to be used why is it included as a prop?
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 should clarify: I don't expect that a consumer would change the prop between renders, so we would be OK with ignoring updates to the prop and only use the initial value that is passed in.
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.
Oh ok that makes more sense. Thank you.
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.
We could avoid the problem by only putting a shouldDisableGPUAcceleration
flag in state, and merging the necessary modifiers with the modifiers specified by the consumer at render time.
...Is it worth any of this complication? Should we disable it unconditionally?
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 opposed to disabling GPU acceleration unconditionally.
Without objection, doing that.
See also: