Skip to content
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

MatAutocomplete selection option event order #18650

Closed
raygig opened this issue Feb 28, 2020 · 3 comments · Fixed by #18707
Closed

MatAutocomplete selection option event order #18650

raygig opened this issue Feb 28, 2020 · 3 comments · Fixed by #18707
Assignees
Labels
area: material/autocomplete feature This issue represents a new feature or feature request rather than a bug or bug fix P4 A relatively minor issue that is not relevant to core functions

Comments

@raygig
Copy link

raygig commented Feb 28, 2020

Feature Description

The current order of events when selecting an option from MatAutocomplete is as follows:

  1. Update the NgModel
  2. Bring focus to the autocomplete
  3. Emit select event which calls @output() optionSelected

This is the code which executes this logic.

    MatAutocompleteTrigger.prototype._setValueAndClose = 
    function (event) {
        if (event && event.source) {
            this._clearPreviousSelectedOption(event.source);
            this._setTriggerValue(event.source.value);
            this._onChange(event.source.value);
            this._element.nativeElement.focus();
            this.autocomplete._emitSelectEvent(event.source);
        }
        this.closePanel();
    };

I am proposing to call the focus() event after this.autocomplete._emitSelectEvent(event.source);

The issue is if you want to test the current selected model value in the focus() event it will be incorrect because the setting of the model happens after focus is called.

Use Case

In the focus event we are testing if the model value is blank and if so then open the panel to give suggested search items.

@raygig raygig added the feature This issue represents a new feature or feature request rather than a bug or bug fix label Feb 28, 2020
@crisbeto
Copy link
Member

crisbeto commented Feb 29, 2020

Rather than using the focus even, couldn't you use the optionSelected event instead? It's not a big deal to change the behavior, but I'm trying to understand what the use case is.

@raygig
Copy link
Author

raygig commented Feb 29, 2020

We are trying to build a suggestion feature which is activated when the autocomplete is focused and the ngmodel is null.

Currently this breaks with this scenario.

  1. Autocomplete has ngmodel===null
  2. The user focuses the autocomplete
  3. Suggestions are displayed
  4. User searches ands selects an option
  5. The suggestion options display again

#5 happens because the focus event triggers before ngmodel is set.

@crisbeto crisbeto self-assigned this Mar 4, 2020
@crisbeto crisbeto added has pr P4 A relatively minor issue that is not relevant to core functions labels Mar 4, 2020
crisbeto added a commit to crisbeto/material2 that referenced this issue Mar 4, 2020
Currently we restore focus to the input and then we emit the change event, but we have a report that it may be making some use cases more difficult. From what I can tell, this shouldn't have much of an impact on existing users so these changes swap the order so that the focus event is last.

Fixes angular#18650.
mmalerba pushed a commit that referenced this issue Apr 7, 2020
Currently we restore focus to the input and then we emit the change event, but we have a report that it may be making some use cases more difficult. From what I can tell, this shouldn't have much of an impact on existing users so these changes swap the order so that the focus event is last.

Fixes #18650.
andrewseguin pushed a commit that referenced this issue Jan 18, 2022
…#18707)

Currently we restore focus to the input and then we emit the change event, but we have a report that it may be making some use cases more difficult. From what I can tell, this shouldn't have much of an impact on existing users so these changes swap the order so that the focus event is last.

Fixes #18650.
andrewseguin pushed a commit that referenced this issue Jan 18, 2022
…#18707)

Currently we restore focus to the input and then we emit the change event, but we have a report that it may be making some use cases more difficult. From what I can tell, this shouldn't have much of an impact on existing users so these changes swap the order so that the focus event is last.

Fixes #18650.

(cherry picked from commit 7409bd6)
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 18, 2022
forsti0506 pushed a commit to forsti0506/components that referenced this issue Apr 3, 2022
…angular#18707)

Currently we restore focus to the input and then we emit the change event, but we have a report that it may be making some use cases more difficult. From what I can tell, this shouldn't have much of an impact on existing users so these changes swap the order so that the focus event is last.

Fixes angular#18650.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: material/autocomplete feature This issue represents a new feature or feature request rather than a bug or bug fix P4 A relatively minor issue that is not relevant to core functions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants