Skip to content

Fix/dynamically changing autocomplete #14402

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

Conversation

thomaspink
Copy link
Contributor

fixes #13812

This PR is a reopened copy of #13814 because of an accidental merge to master despite breaking some internal google apps.

@mmalerba can you provide the error or some information why this PR did break these apps?

@thomaspink thomaspink requested a review from crisbeto as a code owner December 6, 2018 08:01
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 6, 2018
@thomaspink thomaspink changed the title Fix/dynamically changing autocomplete Fix/dynamically changing autocomplete (reopend copy of #13814) Dec 6, 2018
crisbeto
crisbeto previously approved these changes Dec 6, 2018
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM. @thomaspink can you rename the PR to the same as the original one?

@crisbeto crisbeto added pr: lgtm action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Dec 6, 2018
@thomaspink thomaspink changed the title Fix/dynamically changing autocomplete (reopend copy of #13814) Fix/dynamically changing autocomplete Dec 6, 2018
@thomaspink
Copy link
Contributor Author

@crisbeto Sure; Done

@crisbeto crisbeto added the merge: fix commit message When the PR is merged, rewrites/fixups of the commit messages are needed label Dec 6, 2018
@mmalerba
Copy link
Contributor

mmalerba commented Dec 6, 2018

@thomaspink I think they had some custom logic to show loading spinners and things. It's probably their app that needs to be updated, not this PR.

@thomaspink
Copy link
Contributor Author

@mmalerba Good to know. thanks for the investigation.

@thomaspink
Copy link
Contributor Author

@crisbeto What do i have to do solve the "commit message fixup"? Squash the commits?

@crisbeto
Copy link
Member

If you hover over the label, it says that it's for the caretaker.

@vivian-hu-zz
Copy link
Contributor

Change commit message to fix(autocomplete): not updating if panel is changed after init.

@ngbot
Copy link

ngbot bot commented Jan 16, 2019

Hi @thomaspink! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

1 similar comment
@ngbot
Copy link

ngbot bot commented Jan 16, 2019

Hi @thomaspink! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

@thomaspink thomaspink force-pushed the fix/dynamically-changing-autocomplete branch 3 times, most recently from ef235f0 to e851199 Compare January 21, 2019 09:50
@thomaspink
Copy link
Contributor Author

@crisbeto after rebasing the api check fails. There are only minor differences between the golden file and the changed one in the PR.

autocomplete.ts

  • added internal _portal property
  • added _viewContainerRef: ViewContainerRef as a private in constructor
  • added ngAfterViewInit method

autocomplete-trigger.ts

  • removed _viewContainerRef: ViewContainerRef from the constructor

Is there something i have to change/do there?

@crisbeto
Copy link
Member

You can run bazel run //tools/public_api_guard:lib_autocomplete_api.accept to update the golden file.

@thomaspink thomaspink force-pushed the fix/dynamically-changing-autocomplete branch from e851199 to 1483b0f Compare January 21, 2019 11:02
@mmalerba mmalerba added aaa and removed aaa labels Apr 25, 2019
@andrewseguin andrewseguin added the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label May 30, 2019
@mmalerba mmalerba removed the lgtm label Jul 31, 2020
@MohammadRafik
Copy link

@thomaspink this PR has fallen out of date, could you please rebase this PR?

@andrewseguin andrewseguin removed the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 29, 2021
@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
@andrewseguin
Copy link
Contributor

This PR is being closed due to inactivity and needing rebase for a very long time. Please feel free to re-open the change after rebasing and we can take another look at getting it merged.

@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 Sep 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker merge: fix commit message When the PR is merged, rewrites/fixups of the commit messages are needed target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Changing the autocomplete on a trigger does not change the overlay (options)
7 participants