feat(top-app-bar): Add dense #2475
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2475 +/- ##
=======================================
Coverage 98.87% 98.87%
=======================================
Files 102 102
Lines 4072 4072
Branches 510 510
=======================================
Hits 4026 4026
Misses 46 46Continue to review full report at Codecov.
|
| } | ||
|
|
||
| .mdc-top-app-bar__section { | ||
| padding: 0 $mdc-top-app-bar-dense-section-horizontal-padding; |
There was a problem hiding this comment.
I don't think you need this css rule. Isn't it covered by line 154?
williamernest
force-pushed
the
feat/top-app-bar/add-dense
branch
from
c55b35c
to
d034263
Compare
| denseCheckbox.addEventListener('change', function() { | ||
| appBarEl.classList[this.checked ? 'add' : 'remove']('mdc-top-app-bar--dense'); | ||
|
|
||
| shortCheckbox.disabled = this.checked || prominentCheckbox.checked; |
There was a problem hiding this comment.
Dense disables short, but short does not disable dense
| .mdc-top-app-bar__title { | ||
| @include mdc-rtl-reflexive-box(padding, left, $mdc-top-app-bar-title-left-padding); | ||
|
|
||
| padding-bottom: $mdc-top-app-bar-prominent-dense-title-bottom-padding; |
There was a problem hiding this comment.
This is also overriding mobile-specific title bottom padding for prominent (not dense). Is that intentional?
(TBH I find it hard to follow the plethora of different title paddings but I get the impression it's to counteract outer padding...)
There was a problem hiding this comment.
It's intentional. The --dense style is desktop only, so any styles it applies should override any other styles from the mobile media queries.
closes: #2426
Adds the
--densestyle to the top app bar.