feat(select): emit change event #2458
Conversation
googlebot
added
the
cla: yes
crisbeto
force-pushed
the
2248/select-change-event
branch
from
39b2365
to
ec89251
Compare
Adds an event to `md-select` that is emitted when the selected option has changed. Fixes angular#2248.
crisbeto
force-pushed
the
2248/select-change-event
branch
from
ec89251
to
5105a89
Compare
| @@ -3,7 +3,7 @@ | |||
| <div class="demo-select"> | |||
| <div *ngIf="showSelect"> | |||
| <md-card> | |||
| <md-select placeholder="Food i would like to eat" [formControl]="foodControl"> | |||
| <md-select placeholder="Food i would like to eat" [formControl]="foodControl" (change)="changeListener($event)"> | |||
There was a problem hiding this comment.
Can we make a separate demo for the change event that does not use a form directive? The change event is only intended for people not using Angular forms modules, since these already have their own built-in change events. Just don't want to confuse the intended usage with this example.
| @@ -237,6 +237,27 @@ describe('MdSelect', () => { | |||
| expect(fixture.componentInstance.select.selected).not.toBeDefined(); | |||
| }); | |||
|
|
|||
|
|
|||
| it('should emit an event when the selected option has changed', () => { | |||
There was a problem hiding this comment.
This should have its own test component that doesn't use a form module directive
|
Addressed the feedback @kara. |
| }); | ||
|
|
||
| @Component({ | ||
| selector: 'basic-select', | ||
| template: ` | ||
| <div [style.height.px]="heightAbove"></div> | ||
| <md-select placeholder="Food" [formControl]="control" [required]="isRequired"> | ||
| <md-select placeholder="Food" [formControl]="control" [required]="isRequired" | ||
| (change)="changeListener($event)"> |
There was a problem hiding this comment.
Can you remove the change event and listener from the basic select component? It should only be in the new test component.
|
Fixed @kara, not sure how I forgot to remove the last one. |
| @@ -1169,6 +1201,7 @@ class BasicSelect { | |||
| isRequired: boolean; | |||
| heightAbove = 0; | |||
| heightBelow = 0; | |||
| changeListener = jasmine.createSpy('MdSelect change listener'); | |||
|
@crisbeto Is this ready to merge? Just noticed it doesn't have the label. |
|
It is, I'm not sure who's supposed to add the label. |
|
@crisbeto Since you are the author and a member of the repo, you are the one that adds the "merge ready" label :) "LGTM" just means that it's approved; "merge ready" means that the author has made all the changes they wanted to. |
|
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. |
Adds an event to
md-selectthat is emitted when the selected option has changed.Fixes #2248.