-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat(floating-label): Add max-width mixin #2956
Conversation
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'd like to see how exactly this will be used in #2859, since this PR mostly exists to support that. The usages currently in that PR either set this to auto (wouldn't that be equivalent to never setting max-width?), or seem to assume that an icon exists when that isn't guaranteed.
@@ -79,6 +79,7 @@ Mixin | Description | |||
`mdc-floating-label-shake-keyframes($modifier, $positionY, $positionX, $scale)` | Generates a CSS `@keyframes` at-rule for an invalid label shake. Used in conjunction with the `mdc-floating-label-shake-animation` mixin. | |||
`mdc-floating-label-shake-animation($modifier)` | Applies shake keyframe animation to label. | |||
`mdc-floating-label-float-position($positionY, $positionX, $scale)` | Sets position of label when floating. | |||
`mdc-floating-label-max-width($max-width))` | Sets the width of the 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.
width -> max-width
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.
Done!
@@ -67,3 +67,9 @@ | |||
animation: mdc-floating-label-shake-float-above-#{$modifier} 250ms 1; | |||
} | |||
} | |||
|
|||
@mixin mdc-floating-label-max-width($max-width) { | |||
.mdc-floating-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.
Does it really make sense for this selector to be baked into this mixin? Would this not ordinarily be applied to a selector that already references the floating label root element? (See also the ink/fill color mixins)
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.
Updated
Codecov Report
@@ Coverage Diff @@
## master #2956 +/- ##
=========================================
- Coverage 98.3% 98.3% -0.01%
=========================================
Files 101 101
Lines 4375 4368 -7
Branches 564 564
=========================================
- Hits 4301 4294 -7
Misses 74 74
Continue to review full report at Codecov.
|
@@ -79,6 +79,7 @@ Mixin | Description | |||
`mdc-floating-label-shake-keyframes($modifier, $positionY, $positionX, $scale)` | Generates a CSS `@keyframes` at-rule for an invalid label shake. Used in conjunction with the `mdc-floating-label-shake-animation` mixin. | |||
`mdc-floating-label-shake-animation($modifier)` | Applies shake keyframe animation to label. | |||
`mdc-floating-label-float-position($positionY, $positionX, $scale)` | Sets position of label when floating. | |||
`mdc-floating-label-max-width($max-width))` | Sets the max width of the 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.
Extra )
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.
Got it!
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.
Sorry, I didn't realize this was still waiting on my approval ._.
Make sure this is labeled as feat(floating-label)
when you merge it.
No description provided.