-
Notifications
You must be signed in to change notification settings - Fork 6.8k
feat(form-field): add support for separate label and placeholder #8223
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
Conversation
|
@mmalerba I'm open to using the native |
|
Could this be done without any breaking changes? |
|
The only real breaking change is renaming the |
|
Let's go with that- I want to avoid anything we have to call a breaking API change |
4bfc4b3 to
e180748
Compare
|
Refactored to avoid breaking changes. |
e180748 to
0e44d32
Compare
|
Still need to take a closer look at the code, but had a couple thoughts after looking at the demo:
Thanks for doing this! Something I've been wanting to do for a while but haven't gotten the time |
|
|
we might not be able to do the sequenced animation, but could we at least slap a transition on the opacity? I think it will make it look a little nicer |
|
We could do that, but it would be a progressive enhancement since not all browsers allow you to transition the native placeholder (e.g. Chrome does, but Firefox doesn't). |
0e44d32 to
bb61885
Compare
|
Rebased and made it so the placeholder doesn't overlap the animating label @mmalerba, can you take another look? |
src/lib/form-field/form-field.html
Outdated
| <ng-content></ng-content> | ||
|
|
||
| <span class="mat-input-placeholder-wrapper mat-form-field-placeholder-wrapper"> | ||
| <span class="mat-form-field-label-wrapper"> |
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'm sure people are targeting those classes with custom styles, lets leave them for now with a TODO to remove them at the next major.
src/lib/form-field/form-field.html
Outdated
| <!-- We add aria-owns as a workaround for an issue in JAWS & NVDA where the label isn't | ||
| read if it comes before the control in the DOM. --> | ||
| <label class="mat-input-placeholder mat-form-field-placeholder" | ||
| <label class="mat-form-field-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.
Same, leave the old classes too for now
| } | ||
|
|
||
| _shouldLabelFloat() { | ||
| return this._canLabelFloat && (this._control.shouldLabelFloat || |
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 logic seems different than before, it didn't consider _canPlaceholderFloat before
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.
If I remember correctly, some of the logic was done by combining CSS classes. I simplified it to make it easier to follow.
| } | ||
|
|
||
| _hideControlPlaceholder() { | ||
| return !this._hasLabel() || !this._shouldLabelFloat(); |
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.
Maybe the _canLabelFloat from above should be factored into this logic instead?
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.
The _shouldLabelFloat call here already factors in _canLabelFloat.
a348880 to
dc4f44b
Compare
|
I restored the classes and addressed the comments @mmalerba, can you take another look? |
dc4f44b to
9db6693
Compare
|
@crisbeto ready to presubmit if you can rebase |
9db6693 to
0a75795
Compare
|
Rebased @jelbourn. |
|
@crisbeto please rebase. Thanks! |
* Adds the ability for the user to specify both a label and placeholder text for a form field. * Renames all of the "floating placeholder" related terminology to refer to a "floating label" instead. * Aligns the input placeholder color with the Material spec. Fixes angular#6194.
0a75795 to
26be8c4
Compare
|
Rebased @tinayuangao. |
…ular#8223) * feat(form-field): add support for separate label and placeholder * Adds the ability for the user to specify both a label and placeholder text for a form field. * Renames all of the "floating placeholder" related terminology to refer to a "floating label" instead. * Aligns the input placeholder color with the Material spec. Fixes angular#6194. * fix: add transition to placeholder text
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixes #6194.