feat(autocomplete): emit event when an option is selected #4187

crisbeto · 2017-04-20T21:10:09Z

Emits the select event when an option in the autocomplete is selected.

Note: I went with passing the selected option from the trigger to the panel, instead of listening to the onSelectionChange inside the panel, because it involves keeping track of less subscriptions and not having to re-construct them when the list of options changes.

Fixes #4094.
Fixes #3645.

devversion · 2017-04-20T21:17:47Z

src/lib/autocomplete/autocomplete.ts

@@ -50,6 +58,9 @@ export class MdAutocomplete implements AfterContentInit {
  /** Function that maps an option's control value to its display value in the trigger. */
  @Input() displayWith: (value: any) => string;

+  /** Event that is emitted whenever an option from the list is selected. */
+  @Output() select: EventEmitter<MdAutocompleteSelect> = new EventEmitter<MdAutocompleteSelect>();


Shouldn't we just expose the Observable instead of the EventEmitter?

Also as a side-question: What about a change event?

I wanted to avoid change, because typing in the select also changes the value. Regarding the event, using the EventEmitter is the way to handle @Output.

@Output can be basically any Observable. EventEmitter is just like an RxJS Subscription. We originally changed all @Outputs to just expose the Observable.

Actually it looks like a lot of components don't do that. @jelbourn any reason why we didn't continue just exposing the observable? I can go through them and fix the remaining ones.

This is pretty much the only use for the EventEmitter: https://angular.io/docs/ts/latest/api/core/index/EventEmitter-class.html

@crisbeto @devversion is this resolved? I don't want this PR to be forgotten!

+1 to have fix this problem :). I would to know when this PR will be accepted. Thank you

willshowell · 2017-08-01T18:25:51Z

@crisbeto think you could rebase this in anticipation of @kara's review? Very much looking forward to a solution better than,

    this.trigger.autocomplete.options.changes
      .startWith(null)
      .switchMap(() => this.trigger.optionSelections)
      .map(s => s.source)
      .filter(s => s.selected)

kara

Naming quibbles, but otherwise looks good. Can you rebase as well?

kara · 2017-08-15T21:32:33Z

src/lib/autocomplete/autocomplete.spec.ts

@@ -1146,6 +1147,29 @@ describe('MdAutocomplete', () => {
        expect(panel.classList).toContain(visibleClass, `Expected panel to be visible.`);
      });
    }));
+
+  it('should call emit an event when an option is selected', fakeAsync(() => {


Typo: 'should call emit an event' -> 'should emit an event'

kara · 2017-08-15T21:34:42Z

src/lib/autocomplete/autocomplete.ts

 /**
 * Autocomplete IDs need to be unique across components, so this counter exists outside of
 * the component definition.
 */
 let _uniqueAutocompleteIdCounter = 0;

+/** Event object that is emitted when an autocomplete option is selected */
+export class MdAutocompleteSelect {


How about MdAutocompleteSelectEvent ? Without "Event", it almost sounds like a hybrid md-autocomplete / md-select directive.

kara · 2017-08-15T21:36:00Z

src/lib/autocomplete/autocomplete.ts

@@ -50,6 +58,9 @@ export class MdAutocomplete implements AfterContentInit {
  /** Function that maps an option's control value to its display value in the trigger. */
  @Input() displayWith: (value: any) => string;

+  /** Event that is emitted whenever an option from the list is selected. */
+  @Output() select: EventEmitter<MdAutocompleteSelect> = new EventEmitter<MdAutocompleteSelect>();


@jelbourn and I discussed and we like (optionSelected). It's a bit more explicit, and otherwise it sounds a bit too much like md-select.

kara · 2017-08-15T21:41:18Z

src/lib/autocomplete/autocomplete.spec.ts

+    let event = spy.calls.mostRecent().args[0] as MdAutocompleteSelect;
+
+    expect(event.source).toBe(fixture.componentInstance.autocomplete);
+    expect(event.option.value).toBe('Washington');


Can you add a test for when the option list changes? e.g. emitting an event from a new option.

crisbeto · 2017-08-20T11:34:46Z

Thanks for the feedback @kara. It's addressed now, can you take another look?

kara

LGTM. @crisbeto Apply merge label when ready.

kara · 2017-08-22T22:00:52Z

src/lib/autocomplete/autocomplete.ts

+  constructor(public source: MdAutocomplete, public option: MdOption) { }
+}
+
+export type AutocompletePositionY = 'above' | 'below';


Where is this being used? Seems unrelated to this PR.

Emits the `select` event when an option in the autocomplete is selected. **Note:** I went with passing the selected option from the trigger to the panel, instead of listening to the `onSelectionChange` inside the panel, because it involves keeping track of less subscriptions and not having to re-construct them when the list of options changes. Fixes angular#4094. Fixes angular#3645.

angular-automatic-lock-bot · 2019-09-06T23:01:27Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

crisbeto added the pr: needs review label Apr 20, 2017

crisbeto assigned kara Apr 20, 2017

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Apr 20, 2017

devversion reviewed Apr 20, 2017

View reviewed changes

willshowell mentioned this pull request May 2, 2017

Deleting selection and re-choosing causes wrong selection #4356

Closed

jelbourn requested a review from kara July 28, 2017 17:10

kara reviewed Aug 15, 2017

View reviewed changes

kara added pr: needs cleanup / tests labels Aug 15, 2017

kara assigned crisbeto and unassigned kara Aug 15, 2017

crisbeto force-pushed the 4094/autocomplete-select-event branch from 21e4ae2 to 0adbc33 Compare August 20, 2017 11:34

crisbeto removed pr: needs cleanup / tests labels Aug 20, 2017

crisbeto assigned kara and unassigned crisbeto Aug 20, 2017

kara approved these changes Aug 22, 2017

View reviewed changes

kara assigned crisbeto and unassigned kara Aug 22, 2017

kara added pr: lgtm and removed pr: needs review labels Aug 22, 2017

crisbeto added 2 commits August 23, 2017 08:31

refactor: address feedback

e60439f

crisbeto force-pushed the 4094/autocomplete-select-event branch from 0adbc33 to e60439f Compare August 23, 2017 06:36

crisbeto added the action: merge The PR is ready for merge by the caretaker label Aug 23, 2017

kara merged commit 2dd5c7c into angular:master Aug 23, 2017

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(autocomplete): emit event when an option is selected #4187

feat(autocomplete): emit event when an option is selected #4187

crisbeto commented Apr 20, 2017

devversion Apr 20, 2017

crisbeto Apr 20, 2017

devversion Apr 20, 2017 •

edited

crisbeto Apr 20, 2017

willshowell Jul 12, 2017

kevincaradant Jul 28, 2017

willshowell commented Aug 1, 2017

kara left a comment

kara Aug 15, 2017

kara Aug 15, 2017

kara Aug 15, 2017

kara Aug 15, 2017

crisbeto commented Aug 20, 2017

kara left a comment •

edited

kara Aug 22, 2017

angular-automatic-lock-bot bot commented Sep 6, 2019

feat(autocomplete): emit event when an option is selected #4187

feat(autocomplete): emit event when an option is selected #4187

Conversation

crisbeto commented Apr 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devversion Apr 20, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

willshowell commented Aug 1, 2017

kara left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crisbeto commented Aug 20, 2017

kara left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented Sep 6, 2019

devversion Apr 20, 2017 •

edited

kara left a comment •

edited