Skip to content
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

Add !important to [hidden] #557

Merged
merged 1 commit into from
Aug 9, 2018
Merged

Add !important to [hidden] #557

merged 1 commit into from
Aug 9, 2018

Conversation

muan
Copy link
Contributor

@muan muan commented Aug 8, 2018

Reference #467.

While the issue with .d-* styles need to be sorted out, having this !important now would help us with the many cases that we have already encountered with .btn[hidden], where display: inline-block takes precedence over [hidden].


Related but I don't want this to be a blocker – what do y'all think about just duplicating [hidden] {display: none !important;} at the end of visibility-display.scss? Considering these packages can be used separately and this is just one line of code 🤷🏻 IMO it's not horrible. 😬

https://github.com/primer/primer/blob/efddb570a8623458b2ca10da6c7bbfd965712c2b/modules/primer-utilities/lib/visibility-display.scss#L30

@muan muan requested a review from a team August 8, 2018 23:05
@shawnbot
Copy link
Contributor

shawnbot commented Aug 8, 2018

I'm a huge fan of this. It makes showing and hiding things (in JS just setting element.hidden) so much simpler. This might need to be part of v11, though, since it's technically a breaking change.

@shawnbot shawnbot added the ⭐️rep an industry leading reputation label Aug 8, 2018
@jonrohan jonrohan changed the base branch from master to release-10.9.0 August 9, 2018 17:40
Copy link
Member

@jonrohan jonrohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @muan, I'm scheduling this for 10.9.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⭐️rep an industry leading reputation Tag: Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants