chore(select): Remove default variant #3476
Conversation
|
All 349 screenshot tests passed for commit 31bfd09 vs. |
|
All 349 screenshot tests passed for commit fc5c80c vs. |
|
All 349 screenshot tests passed for commit 45e4af7 vs. |
Codecov Report
@@ Coverage Diff @@
## master #3476 +/- ##
=========================================
Coverage ? 98.41%
=========================================
Files ? 120
Lines ? 5033
Branches ? 619
=========================================
Hits ? 4953
Misses ? 80
Partials ? 0
Continue to review full report at Codecov.
|
williamernest
requested review from
kfranqueiro
and removed request for
acdvorak
| @include mdc-rtl-reflexive-position(left, 16px); | ||
|
|
||
| bottom: 12px; | ||
| line-height: 1.75rem; |
There was a problem hiding this comment.
Is this related to any of our typography variables or mixins? Or just a component-specific value?
There was a problem hiding this comment.
This is required to get the notched outline to float to the correct position. This will be refactored in a follow up PR so that the notched outline requires no changes in the default version of text field and the select.
| @@ -145,7 +145,7 @@ class MDCSelect extends MDCComponent { | |||
| this.outline_ = outlineFactory(outlineElement); | |||
| } | |||
|
|
|||
| if (this.root_.classList.contains(cssClasses.BOX)) { | |||
| if (!this.root_.classList.contains(cssClasses.OUTLINED)) { | |||
There was a problem hiding this comment.
This seems like a change of behavior. Did outlined previously not have ripples?
There was a problem hiding this comment.
Never mind... I missed the !.
There was a problem hiding this comment.
The outlined variant does not have ripples. This code used to single out the box variant to add ripples, but since there is no longer a default variant this needs to exclude the outlined variant.
|
🤖 Beep boop! Screenshot test report 🚦14 screenshots changed from |
| @@ -83,21 +83,6 @@ See [Importing the JS component](../../docs/importing-js.md) for more informatio | |||
|
|
|||
| ## Variants | |||
|
|
|||
| ### Select Box | |||
There was a problem hiding this comment.
Should we also remove the default style deprecation notice?
There was a problem hiding this comment.
I think we should leave the deprecation notice in so people see it if they come looking for why their default variant changes.
packages/mdc-select/README.md
Outdated
| @@ -215,7 +199,7 @@ Mixin | Description | |||
| `mdc-select-focused-label-color($color)` | Customizes the label color of the select when focused. | |||
| `mdc-select-bottom-line-color($color)` | Customizes the color of the default bottom line of the select. | |||
| `mdc-select-focused-bottom-line-color($color)` | Customizes the color of the bottom line of the select when focused. | |||
| `mdc-select-corner-radius($radius)` | Customizes the corner radius of the box variant of the select. | |||
| `mdc-select-corner-radius($radius)` | Customizes the corner radius of the select. | |||
There was a problem hiding this comment.
Should we say "of the filled select variant" or something? Since there's a separate mixin for outlined...
packages/mdc-select/mdc-select.scss
Outdated
| will-change: opacity, transform, color; | ||
|
|
||
| // stylelint-disable-next-line plugin/selector-bem-pattern | ||
| .mdc-floating-label { |
There was a problem hiding this comment.
Should this be moved to the floating-label_ mixin you created? (Though that mixin is only seemingly invoked in one place)
williamernest
force-pushed
the
chore/select/remove-default-variant
branch
from
5020366 to
7e01f0e
Compare
|
🤖 Beep boop! Screenshot test report 🚦14 screenshots changed from |
|
🤖 Beep boop! Screenshot test report 🚦14 screenshots changed from |
|
🤖 Beep boop! Screenshot test report 🚦14 screenshots changed from |
|
🤖 Beep boop! Screenshot test report 🚦14 screenshots changed from |
fixes: #2899