-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(text-field): Make outline visibility directly linked to floating labels #2073
Conversation
@@ -63,3 +63,15 @@ | |||
mdc-text-field-transition(opacity); | |||
fill: transparent; | |||
} | |||
|
|||
// Never show the idle outline when the label is floating, otherwise 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.
Is it correct to be putting this within the outline subcomponent CSS when it references classes outside that subcomponent? Otherwise this idea 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.
eh youre probably right. I moved it to mdc-text-field.scss
@@ -255,3 +255,15 @@ | |||
.mdc-text-field--textarea.mdc-text-field--disabled { | |||
@include mdc-text-field-textarea-disabled_; | |||
} | |||
|
|||
// Never show the idle outline when the label is floating, otherwise 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.
One more thought, though I'm being annoying now because this contradicts my earlier one because I didn't think of this...
Do you think it would make more sense to put these styles within an @at-root
within the mdc-text-field-outlined_
mixin instead? It's kind of off in no-man's land here which makes it harder to spot when working in the context of outlined styles, otherwise.
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.
These styles are no longer specific to mdc-text-field--outlined
. They are general styles for any text field that uses both a floating label and an outline.
Right now the only text field that uses a floating label and an outline happens to be mdc-text-field--outlined
. But thats an internal detail.
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 had to re-read this and it took me a minute to realize what you were getting at here but I think I see what you're getting at :P
Codecov Report
@@ Coverage Diff @@
## master #2073 +/- ##
=======================================
Coverage 99.33% 99.33%
=======================================
Files 84 84
Lines 3769 3769
Branches 490 490
=======================================
Hits 3744 3744
Misses 25 25 Continue to review full report at Codecov.
|
No description provided.