-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Using composition to add more specificity to avoid style override #7171
Using composition to add more specificity to avoid style override #7171
Conversation
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.
lgtm! Thank you for making this change.
🎉 Handy Links: |
Hi @arkogupta we're rethinking this change. It looked harmless at first to me which is why I approved it but after speaking with my teammates, I realize that this change might cause problems in the future. OUIFR (office-ui-fabric-react) has a lot of different consumers and increasing the specificity of a selector to solve for one use case may cause issues for others. There must be an underlying root cause that's causing this specificity issue in the first place. In traditional css, the specificity rule is simple - whichever rule's first defined in the document has higher specificity. However, mergeStyles mixes the classes to create one classname. So it's more likely that this mixing isn't being done properly and multiple classnames are being appended to one element causing "specificity wars". Adding selectors seems hacky and we'd like to avoid that. Can you please provide us a better solution? If you could first find the underlying mixing problem and fix that, that would be much appreciated. 😄 |
Makes sense @aneeshack4 . I have raised another PR addressing this( #7211 ). Please take a look at it. |
Pull request checklist
$ npm run change
Description of changes
The style applied to the
borderWhileDragging
class was getting overridden inconsistently by the style applied to theroot
class. So I used composition by making use ofselectors
to add more specificity to prevent it from getting overridden.This demo link can be used to check that the border is indeed getting added and the selectors are getting applied:
https://msft.spoppe.com/teams/SPGroups/playground/cookieredirect/redirect.aspx?mobile=0&setcookie=https%3A%2F%2Fresourceseng.blob.core.windows.net%2Ffiles%2Fdev-arkgupta-arko-p510e
--git-odsp-next-app-min-master%2F&redirect=https%3A%2F%2Fmsft.spoppe.com%2Fteams%2FSPGroups%2FVNextDocLib%2FForms%2FAllItems.aspx.
Here's what was happening before :
After the changes :
Focus areas to test
(optional)
Microsoft Reviewers: Open in CodeFlow