-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2544 +/- ##
==========================================
- Coverage 98.62% 98.61% -0.01%
==========================================
Files 101 98 -3
Lines 4210 4187 -23
Branches 532 532
==========================================
- Hits 4152 4129 -23
Misses 58 58
Continue to review full report at Codecov.
|
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
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.
Its nice you didnt have to change mdc-select/foundation.js at all
packages/mdc-select/_mixins.scss
Outdated
@@ -76,18 +82,26 @@ | |||
} | |||
|
|||
@mixin mdc-select-bottom-line-color_($color) { | |||
.mdc-select__bottom-line { | |||
@include mdc-select-bottom-line-fill-color($color); | |||
.mdc-line-ripple { |
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.
I dont think you need to set the mdc-line-ripple here
packages/mdc-select/_mixins.scss
Outdated
} | ||
|
||
.mdc-select__bottom-line::after { | ||
@include mdc-select-bottom-line-fill-color($color); | ||
.mdc-line-ripple::after { |
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.
::after?? whats this for?
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.
Not sure what it was even doing before. Removing the rule.
packages/mdc-select/constants.js
Outdated
@@ -26,7 +26,7 @@ export const numbers = { | |||
|
|||
export const strings = { | |||
CHANGE_EVENT: 'MDCSelect:change', | |||
BOTTOM_LINE_SELECTOR: '.mdc-select__bottom-line', | |||
BOTTOM_LINE_SELECTOR: '.mdc-line-ripple', |
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.
rename this to LINE_RIPPLE_SELECTOR?
@@ -21,7 +21,7 @@ import {assert} from 'chai'; | |||
import {MDCFloatingLabel} from '../../../packages/mdc-floating-label/index'; | |||
|
|||
const getFixture = () => bel` | |||
<div class="mdc-floating-label"></div> | |||
<label class="mdc-floating-label"></label> |
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.
I think this change snuck in here
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.
This is done on purpose. You're right in that it is more related to the floating label PR, but it wasn't caught until doing changes with line-ripple.
fixes #2129