[Popover] Remove style-propable mixin #3201
Conversation
|
|
||
| return ( | ||
| <Paper | ||
| style={this.mergeStyles(styles.base, style, openStyles.base)} |
There was a problem hiding this comment.
I take it that the reason why styles.base is separate from openStyles.base is so that developers can't override openStyles. I really wonder if that additional logic is worth it though for what we're getting for it. Right now position is overridable and that would completely break this component. In addition transformOrigin isn't being protected either.
Personally, I think it's easier on us and the developer not make any exceptions about what they can and can't override. I think the "protection mechanism" is too brittle for it to be worth it in this case. I'm open to changing it back though!
There was a problem hiding this comment.
No I agree with you. there are no important reasons to protect against overriding it. i'd say this is good 👍
There was a problem hiding this comment.
Sounds good, I'll remove [WIP] from the title because I wanted us to discuss that point first before merging. Thanks!
newoga
changed the title
|
Awesome 👍 🎉 |
|
@newoga Thanks! |
[Popover] Remove style-propable mixin
I noticed a regression/breaking change last minute with this that I can change back, but wanted to get your feedback first about this scenario in general. I'll leave a comment at the particular line of code I'm talking about.