-
Notifications
You must be signed in to change notification settings - Fork 6.8k
feat(autocomplete): add value support #2516
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
902d72b to
c6112aa
Compare
c541077 to
30e83a9
Compare
30e83a9 to
24c94ec
Compare
b8cb2df to
f431e49
Compare
| </md-card> | ||
|
|
||
| <md-card> | ||
| <div>TD value (currentState): {{ currentState }}</div> |
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.
TD?
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.
Fixed to "template-driven" for clarity
| */ | ||
| private _subscribeToClosingActions(): void { | ||
| this.autocomplete.options.changes | ||
| .startWith(null) |
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.
Add comment explaining what this does? I don't know what startWith(null) is for or what switchMap does
| */ | ||
| private _setValueAndClose(event: MdOptionSelectEvent | null): void { | ||
| if (event) { | ||
| // TODO(kara): revisit animation once floating placeholder is toggle-able |
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.
TODO probably isn't necessary any more
| expect(fixture.componentInstance.stateCtrl.dirty) | ||
| .toBe(false, `Expected control to stay pristine if value is set programmatically.`); | ||
| }); | ||
|
|
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.
Should there be any tests for touched state?
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.
That should be tested by the forms module and/or mdInput. The autocomplete doesn't actually touch those events, so seems duplicative to test here.
78a5be9 to
e21a8e1
Compare
|
@jelbourn Comments addressed! |
jelbourn
left a comment
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.
LGTM
e21a8e1 to
e4c8a67
Compare
|
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. |
Autocomplete is WIP. This is a small piece to add value population.
Note: The placeholder animation will still happen when setting the value. This will be fixed in a later PR once floating placeholders are more configurable in input.