refactor(tabs): removed ::after for css-only indicators #1983
Conversation
BREAKING CHANGE: all css-only mdc-tabs must have a .mdc-tab-bar__indicator child element
Codecov Report
@@ Coverage Diff @@
## master #1983 +/- ##
=======================================
Coverage 99.43% 99.43%
=======================================
Files 84 84
Lines 3717 3717
Branches 485 485
=======================================
Hits 3696 3696
Misses 21 21Continue to review full report at Codecov.
|
| @@ -18,6 +18,16 @@ | |||
| @import "@material/theme/mixins"; | |||
| @import "@material/rtl/mixins"; | |||
|
|
|||
| .mdc-tab-bar__indicator, | |||
| .mdc-tab__indicator { | |||
There was a problem hiding this comment.
not sure if this should be in mdc-tab.scss, or if its ok to be in here.
There was a problem hiding this comment.
It should be in mdc-tab.scss.
Since you want to share CSS properties across the two classes, you should create a private mixin and re-use it across the two files.
| @@ -18,6 +18,16 @@ | |||
| @import "@material/theme/mixins"; | |||
| @import "@material/rtl/mixins"; | |||
|
|
|||
| .mdc-tab-bar__indicator, | |||
| .mdc-tab__indicator { | |||
There was a problem hiding this comment.
It should be in mdc-tab.scss.
Since you want to share CSS properties across the two classes, you should create a private mixin and re-use it across the two files.
packages/mdc-tabs/tab/mdc-tab.scss
Outdated
|
|
||
| .mdc-toolbar & { | ||
| @include mdc-theme-prop(background-color, text-primary-on-primary); | ||
| &.mdc-tab--active, |
There was a problem hiding this comment.
You're breaking BEM here, and being unnecessarily specific, you end up with
.mdc-tab-bar:not(.mdc-tab-bar-upgraded) .mdc-tab.mdc-tab--active .mdc-tab__indicator {
...
It should be
.mdc-tab-bar:not(.mdc-tab-bar-upgraded) .mdc-tab--active .mdc-tab__indicator {
...
packages/mdc-tabs/tab/mdc-tab.scss
Outdated
| opacity: .87; | ||
| } | ||
| } | ||
| // stylelint-enable plugin/selector-bem-pattern | ||
|
|
||
| // stylelint-disable plugin/selector-bem-pattern | ||
| .mdc-tab-bar--icons-with-text:not(.mdc-tab-bar-upgraded) .mdc-tab::after { | ||
| top: $mdc-tab-with-icon-and-text-height - 2px; |
There was a problem hiding this comment.
Did you keep this logic somewhere?
There was a problem hiding this comment.
This is no longer needed since its addressed in the .mdc-tab__indicator { bottom: 2px; } rule
packages/mdc-tabs/tab/mdc-tab.scss
Outdated
| &--active, | ||
| &:not(.mdc-tab--active):active .mdc-tab__indicator { | ||
| opacity: .87; | ||
| &--active, |
There was a problem hiding this comment.
remove this & syntax, it is confusing
BREAKING CHANGE: all css-only mdc-tabs must have a .mdc-tab-bar__indicator
child element