Skip to content

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