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

fix(material/autocomplete): mark control as touched once panel is closed #18318

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Jan 28, 2020

Currently we mark the autocomplete control as touched on blur. The problem is that the blur event happens a split second before the panel is closed which can cause the error styling to show up and disappear quickly which looks glitchy. These changes change it so that the control is marked as touched once the panel is closed.

Also makes a couple of underscored properties private since they weren't used anywhere in the view.

Fixes #18313.

Caretaker note: Change is problematic, lots of tests depend on programmatic blur events to mark a control as touched.

@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: patch This PR is targeted for the next patch release labels Jan 28, 2020
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jan 28, 2020
@crisbeto
Copy link
Member Author

Caretaker note: this changes the order of events slightly which has the potential to break tests. We should evaluate how to proceed based on the presubmit results.

jelbourn
jelbourn previously approved these changes Jan 29, 2020
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Jan 29, 2020
@jelbourn
Copy link
Member

SIGH

error TS2341: Property '_onChange' is private and only accessible within class 'MatAutocompleteTrigger'.

96       this.autoTrigger._onChange(this.autoTrigger.activeOption.value);

There are 110 failing targets. Half of them are build errors like this, and the other half are test failures due to the behavior change. We've got to get people using the harnesses.

@jelbourn jelbourn added the presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged label Jan 29, 2020
@crisbeto crisbeto force-pushed the 18313/autocomplete-touched-blur branch from 808a3a5 to 54bdaa3 Compare January 29, 2020 18:05
@crisbeto
Copy link
Member Author

I've put back the two functions so at least the compilation issues are resolved.

@mmalerba mmalerba removed the lgtm label Jul 31, 2020
@crisbeto crisbeto force-pushed the 18313/autocomplete-touched-blur branch from 54bdaa3 to 71331d0 Compare August 18, 2020 19:50
@crisbeto crisbeto changed the title fix(autocomplete): mark control as touched once panel is closed fix(material/autocomplete): mark control as touched once panel is closed May 15, 2021
@crisbeto crisbeto force-pushed the 18313/autocomplete-touched-blur branch from 71331d0 to 97d0d86 Compare May 15, 2021 15:48
@crisbeto crisbeto force-pushed the 18313/autocomplete-touched-blur branch from 97d0d86 to 0884dcc Compare August 17, 2021 20:20
@andrewseguin andrewseguin added needs rebase and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels Dec 28, 2021
@crisbeto crisbeto force-pushed the 18313/autocomplete-touched-blur branch from 0884dcc to 2c7ce85 Compare February 25, 2022 09:57
@crisbeto crisbeto removed needs rebase presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged labels Feb 25, 2022
@crisbeto crisbeto force-pushed the 18313/autocomplete-touched-blur branch from 2c7ce85 to 1d23c86 Compare February 25, 2022 14:49
@crisbeto crisbeto requested a review from a team as a code owner February 25, 2022 14:49
@crisbeto crisbeto force-pushed the 18313/autocomplete-touched-blur branch from 1d23c86 to f63b699 Compare February 25, 2022 14:51
Currently we mark the autocomplete control as touched on `blur`. The problem is that the `blur` event happens a split second before the panel is closed which can cause the error styling to show up and disappear quickly which looks glitchy. These changes change it so that the control is marked as touched once the panel is closed.

Also makes a couple of underscored properties private since they weren't used anywhere in the view.

Fixes angular#18313.
@crisbeto crisbeto force-pushed the 18313/autocomplete-touched-blur branch from f63b699 to 8806d31 Compare February 25, 2022 15:41
@josephperrott josephperrott removed the request for review from a team March 1, 2022 16:33
@andrewseguin andrewseguin removed the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mat Autocomplete With Required Input Flashes Error on Option Select
5 participants