feature(md-input): refactor MdInput as an attribute #1858
Conversation
googlebot
added
the
cla: yes
hansl
force-pushed
the
md-input-refactor
branch
2 times, most recently
from
20a1787
to
3cc8ddd
Compare
|
@hansl seems to be some CI failures. |
|
Seems like it. Looking. |
hansl
force-pushed
the
md-input-refactor
branch
4 times, most recently
from
8de1caa
to
da9b2a8
Compare
hansl
force-pushed
the
md-input-refactor
branch
4 times, most recently
from
691f295
to
e9f2efb
Compare
| [class]="_cssClass" | ||
| [attr.style]="_safeCssStyle" | ||
| [class.md-end]="align == 'end'" | ||
| [class.md-input-wrapper]="true"> |
There was a problem hiding this comment.
I think you could simplify the classes to
<div (click)="focus()"
class="md-input-wrapper"
[class.md-end]="align == 'end'"
[ngClass]="_cssClass"
[attr.style]="_safeCssStyle"| @@ -57,7 +57,7 @@ md-input, md-textarea { | |||
| } | |||
|
|
|||
| // The Input element proper. | |||
| .md-input-element { | |||
| input[md-input], textarea[md-textarea] { | |||
There was a problem hiding this comment.
Could we keep the single class in order to keep specificity low?
| } | ||
| @Component({template: ` | ||
| <input md-input [placeholder]="p"> | ||
| <template #p>template placeholder</template> |
There was a problem hiding this comment.
Include an element inside the placeholder?
| @Input('class') _cssClass: string = ''; | ||
| @Input('style') _cssStyle: string = ''; | ||
| get _safeCssStyle(): SafeStyle { | ||
| return this._dom.bypassSecurityTrustStyle(this._cssStyle || ''); |
There was a problem hiding this comment.
I think we should only bypassSecurityTrustStyle if we know that the style was set by a static value and not by binding, though I'm not sure there's a way to know for sure (since you could have something like style="width: {{w}}px").
If we always call bypassSecurityTrustStyle, we're effectively creating a hole in the SCE checks; a binding on style should fail if the user doesn't explicitly trust the value themselves.
@mprobst any thoughts on this? TL;DR forwarding style from one element to another.
| } | ||
| get inputId(): string { return `${this.id}-input`; } | ||
| @HostBinding('attr.class') get _attrClass(): any { return null; } | ||
| @HostBinding('attr.style') get _attrStyle(): any { return null; } |
There was a problem hiding this comment.
I also just thought about the fact that ngClass, ngStyle, [class.whatever], and [style.whatever] will all potentially be a problem here. We should chat about dealing with these (potentially with Tobias)
| @@ -0,0 +1,12 @@ | |||
| <label class="md-placeholder" | |||
There was a problem hiding this comment.
It might be a problem if the placeholder is always the label for the input. I can see cases where you would want to provide a more specific label than what's in the placeholder.
There was a problem hiding this comment.
We tested it with the old interface, and there wasn't a problem.
| import {coerceBooleanProperty} from '../core/coersion/boolean-property'; | ||
|
|
||
|
|
||
| export const MD_PLACEHOLDER_HOST_TOKEN = new OpaqueToken('mdPlaceholderHost'); |
There was a problem hiding this comment.
These don't normally have _TOKEN at the end (e.g., NG_VALIDATORS).
Add description comment for the token.
The goal is to simplify the MdInput interface and allow it to evolve and collaborate with other directives on an <input>. For example, Chips could be a directive on the `<input>` as well. The next steps are going to be adding back MdPlaceholder as such a directive that collaborates with MdInput.
hansl
force-pushed
the
md-input-refactor
branch
3 times, most recently
from
2a1ef5a
to
2319aab
Compare
hansl
force-pushed
the
md-input-refactor
branch
from
2319aab
to
3fe01d9
Compare
|
@jelbourn PTAL. |
|
Closing in favor of #2052 |
|
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. |
using Projection.