-
-
Notifications
You must be signed in to change notification settings - Fork 78.5k
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
Navbar optimizations #29864
Navbar optimizations #29864
Conversation
@MartijnCuppens did you test this with IE 11 too? |
Yes |
Generally speaking, I always prefer to be explicit instead of using shorthand. Shorthand often hides an explicit choice and can add extra property-values. Long form is also a little more readable IMO as it avoids special syntax around |
It's a thin line for sure :/ That being said when we just need to set all properties, a shorthand is fine IMHO. No need not to leverage shorthands' power in this case. |
7b2083f
to
a39a634
Compare
- Remove redundant `display: inline-block` from flex children - Remove `line-height: inherit;` which is the default value of `line-height` - Use flex shorthand - Improve background shorthand - Fix removed brand margin caused by requiring containers in navbars
a39a634
to
16f2c08
Compare
It's a thin line indeed, but in this case:
Let's live by this rule. I'm gonna check where we need to adjust things on other places (I'll open a separate PR to fix these issues). |
- Remove redundant `display: inline-block` from flex children - Remove `line-height: inherit;` which is the default value of `line-height` - Use flex shorthand - Improve background shorthand - Fix removed brand margin caused by requiring containers in navbars
display: inline-block
from flex childrenline-height: inherit;
which is the default value ofline-height