Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

fix(autocomplete): prevent flashing of invalid state #12064

Conversation

shishkinilya
Copy link
Contributor

@shishkinilya shishkinilya commented Dec 16, 2020

PR Checklist

Please check your PR fulfills the following requirements:

  • The commit message follows our guidelines
  • Tests for the changes have been added or this is not a bug fix / enhancement
  • Docs have been added, updated, or were not required

PR Type

What kind of change does this PR introduce?

[X] Bugfix
[ ] Enhancement
[ ] Documentation content changes
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

When user click on suggestion item, input element lost focus, and then programmatically get focused again, in between these moments invalid state flashes

Also described in #10975

Fixes #10975

What is the new behavior?

Invalid state doesn't flash

Does this PR introduce a breaking change?

[ ] Yes
[X] No

Other information

Perhaps you can help me, my branch fails the test in autocomplete.spec.js "shows the md-not-found template even if we have lost focus", and to be honest, I don't understand the meaning of this test. Why should the md-not-found element must be visible in case of focus loss? Plus, this does not match the actual behavior of the component, if the input element loses focus, the md-not-found element is also hidden.

- when user clicks on suggestion, input element lost focus and then programmatically get focused
- again, add stop immediate propogation to prevent blur event handling by other handlers which
- update ng-model validity state

Fixes angular#10975
@google-cla google-cla bot added the cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ label Dec 16, 2020
Copy link
Member

@Splaktar Splaktar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for investigating this issue!

src/components/autocomplete/js/autocompleteController.js Outdated Show resolved Hide resolved
src/components/autocomplete/js/autocompleteController.js Outdated Show resolved Hide resolved
@Splaktar Splaktar self-assigned this Dec 16, 2020
@Splaktar Splaktar added this to the 1.2.2 milestone Dec 16, 2020
@Splaktar Splaktar added type: bug P5: nice to have These issues will not be fixed without community contributions. ux: polish in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs labels Dec 16, 2020
- function blur most often called without an argument

Fixes angular#10975
@Splaktar Splaktar removed the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Dec 17, 2020
@Splaktar Splaktar marked this pull request as ready for review December 17, 2020 12:38
Copy link
Member

@Splaktar Splaktar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I verified this manually using the autocomplete floating label demo.

@Splaktar Splaktar added the pr: merge ready This PR is ready for a caretaker to review label Dec 17, 2020
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Splaktar Splaktar added the pr: lgtm This PR has been approved by the reviewer label Dec 17, 2020
@Splaktar Splaktar merged commit a4732a9 into angular:master Dec 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ P5: nice to have These issues will not be fixed without community contributions. pr: lgtm This PR has been approved by the reviewer pr: merge ready This PR is ready for a caretaker to review type: bug ux: polish
Projects
None yet
Development

Successfully merging this pull request may close these issues.

autocomplete: md-require-match flashes ng-message error on selection
3 participants