fix(autocomplete): update template when changing autocomplete in trigger #13814

thomaspink · 2018-10-25T14:32:21Z

googlebot · 2018-10-25T14:32:26Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

thomaspink · 2018-10-25T14:34:44Z

I signed it!

googlebot · 2018-10-25T14:34:46Z

CLAs look good, thanks!

crisbeto · 2018-10-25T17:39:36Z

src/lib/autocomplete/autocomplete-trigger.ts

@@ -569,8 +569,15 @@ export class MatAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
      throw getMatAutocompleteMissingPanelError();
    }

-    if (!this._overlayRef) {
+    if (!this._portal || this._portal.templateRef !== this.autocomplete.template) {


A cleaner approach to this would be to move the _portal to MatAutocomplete and have the trigger just call OverlayRef.attach.

Do you prefer this (cleaner) approach?

I see you are also going with the current solution of not moving the portal on this PR:
https://github.com/angular/material2/pull/13819/files#diff-39ddcd24dbc36104e27f79186c93d87aR547

I would've preferred to move it for the menu as well, but it has a different set of requirements, because the consumer can pass in any object that implements the MatMenuPanel interface.

I moved the portal to the mat-autocomplete. But we still have to detach first in the trigger before attaching a new portal, otherwise it will throw the "already attached" error.
I am also un- and resubscribing to closing actions now when attaching a new portal, so we get the correct option changes events.

src/lib/autocomplete/autocomplete.spec.ts

fix(autocomplete): removed debug command in test

…lete

crisbeto · 2018-10-29T12:52:11Z

src/lib/autocomplete/autocomplete-trigger.ts

@@ -596,9 +595,19 @@ export class MatAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
      this._overlayRef.updateSize({width: this._getPanelWidth()});
    }

-    if (this._overlayRef && !this._overlayRef.hasAttached()) {
-      this._overlayRef.attach(this._portal);
+    if (this._currentPortalAttached !== this.autocomplete._portal) {


I'm not sure why you'd need to do this. You can just call detach and then attach again to set the new portal.

@crisbeto I see your point there, but i am currently working on an issue (appears with and without this check) where a new panel will be created, even if the template has not changed. This especially happens in the tests, thats why a lot of them are failing.

For example take a look at this test: autocomplete.spec.ts#L267.
It is failing because we are creating a new panel on focusin and later on typeInElement (_attachOverlay gets called in multiple times).
So i need to find a way to check if we should detach/reatach (when a new mat-autocomplete has been set or not.
Maybe you have a good solution/idea for this one? Detach only if the template has changed?

You don't have to do it in _attachPortal though. Keeping _attachPortal as it is in master, aside from taking the portal from the panel, and calling detach when the autocomplete input changes should do it.

Changed it to a detach in the autocomplete input. Thanks for your help

Adds the yarn-related logs to a gitignore so people don't try to commit them accidentally. See angular#13814.

crisbeto

LGTM, can be marked as merge ready once the last set of feedback is addressed and the yarn-error.log file is deleted.

crisbeto · 2018-10-30T18:40:18Z

src/lib/autocomplete/autocomplete.spec.ts

+    fixture.componentInstance.trigger.autocomplete = fixture.componentInstance.autoTow;
+    fixture.detectChanges();
+
+    // reopen agian


agian -> again. Alternately these comments can be removed since they don't contribute much.

Adds the yarn-related logs to a gitignore so people don't try to commit them accidentally. See #13814.

jelbourn

LGTM

Adds the yarn-related logs to a gitignore so people don't try to commit them accidentally. See angular#13814.

Adds the yarn-related logs to a gitignore so people don't try to commit them accidentally. See #13814.

… in trigger (#13814)" This reverts commit 904a5ea.

… in trigger" (#14398) * Revert "Revert "fix(autocomplete): auto-highlighted first option not display correctly if the floating label is disabled" (#14396)" This reverts commit aa17fbd. * Revert "fix(drag-drop): error on touch end (#14392)" This reverts commit 53cecbb. * Revert "fix(menu): reduce specificity of icon selector (#14389)" This reverts commit 74e945a. * Revert "fix(drag-drop): throw better error when attaching to non-element node (#14221)" This reverts commit 31f0e6d. * Revert "fix(autocomplete): auto-highlighted first option not display correctly if the floating label is disabled (#13774)" This reverts commit c99c512. * Revert "fix(autocomplete): update template when changing autocomplete in trigger (#13814)" This reverts commit 904a5ea.

mmalerba · 2018-12-05T23:33:41Z

This was accidentally merged despite breaking some applications inside Google, so I had to revert it.

@thomaspink feel free to open a new PR and we can try to resolve the breakages before merging again

thomaspink · 2018-12-06T08:26:01Z

@mmalerba Reopened the PR: #14402

…ger (#13814) * fix(autocomplete): update template when changing autocomplete in trigger * fix(autocomplet): improved test for changing autocomplete panel fix(autocomplete): removed debug command in test * fix(autocomplete): moved portal from autocomplete-trigger to autocomplete * fix(autocomplete): setting currentPortal in trigger * fix(autocomplete): fixes issue when detaching when panel updates * feat(autocomplete): detaching portal when autocomplete input changes * feat(autocomplete): moved overlay detach to its own method * feat(autocomplete): typo & removed yarn-error.log

… in trigger" (#14398) * Revert "Revert "fix(autocomplete): auto-highlighted first option not display correctly if the floating label is disabled" (#14396)" This reverts commit aa17fbd. * Revert "fix(drag-drop): error on touch end (#14392)" This reverts commit 53cecbb. * Revert "fix(menu): reduce specificity of icon selector (#14389)" This reverts commit 74e945a. * Revert "fix(drag-drop): throw better error when attaching to non-element node (#14221)" This reverts commit 31f0e6d. * Revert "fix(autocomplete): auto-highlighted first option not display correctly if the floating label is disabled (#13774)" This reverts commit c99c512. * Revert "fix(autocomplete): update template when changing autocomplete in trigger (#13814)" This reverts commit 904a5ea.

…ger (angular#13814) * fix(autocomplete): update template when changing autocomplete in trigger * fix(autocomplet): improved test for changing autocomplete panel fix(autocomplete): removed debug command in test * fix(autocomplete): moved portal from autocomplete-trigger to autocomplete * fix(autocomplete): setting currentPortal in trigger * fix(autocomplete): fixes issue when detaching when panel updates * feat(autocomplete): detaching portal when autocomplete input changes * feat(autocomplete): moved overlay detach to its own method * feat(autocomplete): typo & removed yarn-error.log

… in trigger" (angular#14398) * Revert "Revert "fix(autocomplete): auto-highlighted first option not display correctly if the floating label is disabled" (angular#14396)" This reverts commit aa17fbd. * Revert "fix(drag-drop): error on touch end (angular#14392)" This reverts commit 53cecbb. * Revert "fix(menu): reduce specificity of icon selector (angular#14389)" This reverts commit 74e945a. * Revert "fix(drag-drop): throw better error when attaching to non-element node (angular#14221)" This reverts commit 31f0e6d. * Revert "fix(autocomplete): auto-highlighted first option not display correctly if the floating label is disabled (angular#13774)" This reverts commit c99c512. * Revert "fix(autocomplete): update template when changing autocomplete in trigger (angular#13814)" This reverts commit 904a5ea.

angular-automatic-lock-bot · 2019-09-10T05:03:28Z

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.}

fix(autocomplete): update template when changing autocomplete in trigger

05e2ca3

thomaspink requested a review from crisbeto as a code owner October 25, 2018 14:32

googlebot added the cla: no label Oct 25, 2018

googlebot added cla: yes and removed cla: no labels Oct 25, 2018

thomaspink mentioned this pull request Oct 25, 2018

fix(autocomplete): update template when changing autocomplete in trigger #13813

Closed

crisbeto reviewed Oct 25, 2018

View reviewed changes

thomaspink added 3 commits October 25, 2018 23:42

fix(autocomplet): improved test for changing autocomplete panel

fd6feb5

fix(autocomplete): removed debug command in test

fix(autocomplete): moved portal from autocomplete-trigger to autocomp…

6454ffc

…lete

fix(autocomplete): setting currentPortal in trigger

89391c5

crisbeto reviewed Oct 29, 2018

View reviewed changes

thomaspink added 3 commits October 29, 2018 16:32

fix(autocomplete): fixes issue when detaching when panel updates

89496d8

feat(autocomplete): detaching portal when autocomplete input changes

1e32f4b

feat(autocomplete): moved overlay detach to its own method

8831fc8

thomaspink requested a review from jelbourn as a code owner October 30, 2018 13:09

crisbeto added a commit to crisbeto/material2 that referenced this pull request Oct 30, 2018

chore: add yarn logs to gitignore

ea9eaa4

Adds the yarn-related logs to a gitignore so people don't try to commit them accidentally. See angular#13814.

crisbeto mentioned this pull request Oct 30, 2018

chore: add yarn logs to gitignore #13884

Merged

crisbeto approved these changes Oct 30, 2018

View reviewed changes

crisbeto added the pr: lgtm label Oct 30, 2018

jelbourn pushed a commit that referenced this pull request Oct 30, 2018

chore: add yarn logs to gitignore (#13884)

7cfa0c5

Adds the yarn-related logs to a gitignore so people don't try to commit them accidentally. See #13814.

feat(autocomplete): typo & removed yarn-error.log

471d2f4

crisbeto added action: merge target: patch labels Oct 31, 2018

jelbourn approved these changes Oct 31, 2018

View reviewed changes

atscott pushed a commit to atscott/components that referenced this pull request Nov 5, 2018

chore: add yarn logs to gitignore (angular#13884)

57479f5

Adds the yarn-related logs to a gitignore so people don't try to commit them accidentally. See angular#13814.

jelbourn pushed a commit that referenced this pull request Nov 6, 2018

chore: add yarn logs to gitignore (#13884)

06b01a3

Adds the yarn-related logs to a gitignore so people don't try to commit them accidentally. See #13814.

mmalerba merged commit 904a5ea into angular:master Dec 4, 2018

mmalerba added a commit that referenced this pull request Dec 5, 2018

Revert "fix(autocomplete): update template when changing autocomplete…

f6cf946

… in trigger (#13814)" This reverts commit 904a5ea.

mmalerba mentioned this pull request Dec 5, 2018

Revert "fix(autocomplete): update template when changing autocomplete in trigger" #14398

Merged

thomaspink mentioned this pull request Dec 6, 2018

Fix/dynamically changing autocomplete #14402

Closed

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(autocomplete): update template when changing autocomplete in trigger #13814

fix(autocomplete): update template when changing autocomplete in trigger #13814

thomaspink commented Oct 25, 2018 •

edited

Loading

googlebot commented Oct 25, 2018

thomaspink commented Oct 25, 2018

googlebot commented Oct 25, 2018

crisbeto Oct 25, 2018

thomaspink Oct 25, 2018

crisbeto Oct 25, 2018

thomaspink Oct 29, 2018

crisbeto Oct 29, 2018

thomaspink Oct 29, 2018 •

edited

Loading

crisbeto Oct 29, 2018

thomaspink Oct 30, 2018

crisbeto left a comment

crisbeto Oct 30, 2018

jelbourn left a comment

mmalerba commented Dec 5, 2018

thomaspink commented Dec 6, 2018

angular-automatic-lock-bot bot commented Sep 10, 2019

fix(autocomplete): update template when changing autocomplete in trigger #13814

fix(autocomplete): update template when changing autocomplete in trigger #13814

Conversation

thomaspink commented Oct 25, 2018 • edited Loading

googlebot commented Oct 25, 2018

What to do if you already signed the CLA

Individual signers

Corporate signers

thomaspink commented Oct 25, 2018

googlebot commented Oct 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomaspink Oct 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crisbeto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jelbourn left a comment

Choose a reason for hiding this comment

mmalerba commented Dec 5, 2018

thomaspink commented Dec 6, 2018

angular-automatic-lock-bot bot commented Sep 10, 2019

thomaspink commented Oct 25, 2018 •

edited

Loading

thomaspink Oct 29, 2018 •

edited

Loading