refactor(notched-outline): Refactor notched outline to use 3 divs #4035
Conversation
There was a problem hiding this comment.
Reminder to hit the main text field README for notched-outline updates as well... and we probably need notes in the select and text field READMEs about placing the floating-label inside the notched outline.
There are also still files with the floating label outside the notched outline:
- demos/text-field.html
- demos/theme/index.html
- test/screenshot/spec/mdc-textfield/classes/focused-helper-text-persistent.html (there's one case where there's 2 floating labels, one outside and one inside)
- test/screenshot/spec/mdc-textfield/classes/focused.html (ditto)
|
Will this issue fix this?
There is no outline visible on the right side of the text field. https://codepen.io/kfranqueiro/pen/QJVObP Running on windows 10, chrome 70.0.3538.110 Or is this another issue? |
|
It should resolve that issue.
…On Sat, Dec 1, 2018, 6:55 AM Thomas Dekiere ***@***.*** wrote:
Will this issue fix this?
[image: image]
<https://user-images.githubusercontent.com/4201102/49329562-2713de80-f581-11e8-9d13-9a696105a6d5.png>
There is no outline visible on the right side of the text field.
After you click the textfield the outline on the right side appears and
will stay even when clicking outside the textfield.
https://codepen.io/kfranqueiro/pen/QJVObP
Running on windows 10, chrome 70.0.3538.110
Or is this another issue?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#4035 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/Ag65XHIK1ZrVAbU-Gsyx0wsKRJ2Nv02Nks5u0ph6gaJpZM4YIxr7>
.
|
| font-size: ($scale * 1rem); | ||
| } | ||
|
|
||
| .mdc-notched-outline--upgraded { |
There was a problem hiding this comment.
I removed the "calling mixins with parent selectors" part of the readme since it was obsolete as written... however, this particular mixin still operates under the assumption that the context is a parent, otherwise this selector won't match.
On the other hand, if we were to revise this to &.mdc-notched-outline--upgraded, you would need to select on the notched outline element (not a parent) in order for it to work, which text field and select currently don't.
We could potentially include both selectors (.mdc-notched-outline--upgraded, &.mdc-notched-outline--upgraded), but that's arguably emitting extra styles, and I'm not sure if there'd be any possible side effects.
Typically we set the expectation that applying a mixin to the root class of its component will work. We do make this sort of assumption in a few other places (I searched for &. in code, though half are in private mixins)... what do you think?
There was a problem hiding this comment.
I'll make a quick commit adding both selectors and see if it changes anything.
| @@ -67,59 +73,24 @@ CSS Class | Description | |||
| --- | --- | |||
| `mdc-notched-outline` | Mandatory. Container for the SVG of the notched outline path. | |||
There was a problem hiding this comment.
@williamernest @kfranqueiro This line in the documentation still mentions SVG despite the notched outline not using SVG anymore.
There was a problem hiding this comment.
Man, I knew I was still going to miss something in the README. Thanks for pointing it out.
Placeholder PR so I can run screenshots.
Notes for diffs:
BREAKING CHANGE: The notched outline has been changed from using an SVG for the outline to using 3 div elements. This approach resolves initial rendering issues as well as inconsistencies between the different types of outlines. Please refer to the Readme or the screenshot test pages for details and examples.