fix(icon): reverse for loop when removing child nodes from mat-icon #12073

pavel-agarkov · 2018-07-05T12:03:59Z

when mat-icon element has more than one child node (like when mat-badge is used) then child nodes should be removed in the reversed order otherwise you will attempt to get node by index which is not correct any more since you've deleted some nodes already in the previous iterations and it will cause an error.

…gle (#10990)

Currently we use an rgba color for the table's sorting arrow which ends up looking weird, because the arrow is made up of multiple stacked elements. These changes add some logic that convert the arrow's color into a solid one so the individual pieces can blend into each other. Fixes #11340.

… is opened (#11537) Considers a select as focused as long as a its panel is open, even if the trigger loses focus. This avoids cases where the label can be seen blinking in the background when an option is toggled in multi-select mode.

…first open (#11707) Fixes the menu's panel position not being accurate if it's `xPosition` or `yPosition` is updated after it has been opened for the first time. Fixes #11668.

… always be on (#11462)

`OverlayRef` structurally matches `OverlayReference`, but does not explicitly implement it. That works in TypeScript, and should work in Closure Compiler through the use of `@record`, but for unclear reasons doesn't. Explicitly implementing the interface makes this code clearer, and works around the problem in Closure Compiler while that is being fixed.

…rst time (#11859)

…ver (#11900)

#11938)

…1951)

…ers (#10589)

…tion is RTL (#11877)

…anges (#4469)

…9132)

… on init (#7665)

)

when `mat-icon` element has more than one child node (like when `mat-badge` is used) then child nodes should be removed in the reversed order otherwise you will attempt to get node by index which is not correct any more since you've deleted some nodes already in the previous iterations.

googlebot · 2018-07-05T12:04:03Z

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.

pavel-agarkov · 2018-07-05T12:06:28Z

CLA is signed

googlebot · 2018-07-05T12:06:30Z

CLAs look good, thanks!

crisbeto · 2018-07-05T18:40:18Z

src/lib/icon/icon.ts

@@ -200,7 +200,7 @@ export class MatIcon extends _MatIconMixinBase implements OnChanges, OnInit, Can

    // Remove existing non-element child nodes and SVGs, and add the new SVG element. Note that
    // we can't use innerHTML, because IE will throw if the element has a data binding.
-    for (let i = 0; i < childCount; i++) {
+    for (let i = childCount - 1; i >= 0; i--) {


This could be simplified to while (childCount--)

the loop is simplified as you suggested

crisbeto · 2018-07-05T19:06:16Z

@pavel-agarkov looks like you opened the PR against the wrong branch. It should be against master, but you're targeting 6.3.x.

googlebot · 2018-07-05T19:15:15Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

pavel-agarkov · 2018-07-05T19:18:22Z

@crisbeto, do I need to start from the scratch?

pavel-agarkov · 2018-07-05T19:26:49Z

closed in favor of #12078

angular-automatic-lock-bot · 2019-09-09T05:01:19Z

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

tinayuangao and others added 30 commits June 20, 2018 11:05

fix(button-toggle): use native button and aria-pressed for button-tog…

b959e18

…gle (#10990)

fix(menu): panel positions not changing if position is updated after …

671ad82

…first open (#11707) Fixes the menu's panel position not being accurate if it's `xPosition` or `yPosition` is updated after it has been opened for the first time. Fixes #11668.

fix(progress-spinner): create internal default to force animations to…

11f3993

… always be on (#11462)

fix(button-toggle): emit change event when button is clicked (#11886)

828b3c0

Update MatFormField outline DOM structure (#11771)

194b3c7

fix(menu): scrollable menu not scrolled to top when opened for the fi…

548266b

…rst time (#11859)

fix(select): reset option being marked as selected (#11934)

cd32d78

fix(list-key-manager): allow withWrap to be disabled (#11920)

ab14539

fix(aria-describer): clear duplicate container coming in from the ser…

31c8f6b

…ver (#11900)

fix(button): not resetting all outlines on firefox (#11911)

df80dc6

fix(card): don't override vertical divider styles (#11899)

8a9a8bc

fix(chips): ripples still showing up when globally disabled (#11918)

8d6a9ae

fix(tabs): unable to set aria-label or aria-labelledby on tab (#11898)

8e44f89

fix(overlay): clear timeout if the backdrop transition completes early (

c55c0e1

#11938)

feat(moment-dateadapter): add option to create utc dates (#11336)

3acd2a4

docs(ripple): add note about NoopAnimationsModule (#11909)

5aac678

fix(autocomplete): don't override native autocomplete attribute (#11926)

ee9ddfb

fix(schematics): add parse5 dep (#11647)

42dd824

fix(slide-toggle): not updating model from toggle method (#11846)

e69cf76

fix(icon): clearing all content when inserting a new SVG (#11956)

96b389c

fix(button-toggle): allow event bubbling for toggle button clicks (#1…

79415c4

…1951)

fix(checkbox): fix checkbox animation when moved between view contain…

0244afc

…ers (#10589)

Fix schematics collection JSON. (#11839)

47741ab

fix: default config check is not checking test builder (#11816)

c36e6f3

fix(schematics): error in generated unit test (#11884)

0e5b30e

fix(select): support ctrl+a shortcut for multi-select (#11799)

11d7080

fix(GlobalPositionStrategy): justifyContent center ignored when direc…

6b1f703

…tion is RTL (#11877)

crisbeto and others added 7 commits June 29, 2018 10:44

fix(autocomplete): reposition the panel when the amount of options ch…

a7c4d0c

…anges (#4469)

fix: parse5 not listed as dependency for material (#11981)

ce90bed

fix(tabs): maintain selected tab when new tabs are added or removed (#…

1e164b6

…9132)

fix(focus-trap): not attaching correctly if element is not in the DOM…

80d9a9a

… on init (#7665)

fix(sidenav): remove margin from content instead of setting zero (#11986

82016c2

)

chore: update changelog for 6.3.2 (#12019)

c4c4d5b

pavel-agarkov requested a review from jelbourn as a code owner July 5, 2018 12:04

googlebot added the cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla label Jul 5, 2018

googlebot added cla: yes PR author has agreed to Google's Contributor License Agreement and removed cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla labels Jul 5, 2018

pavel-agarkov changed the title ~~reverse for loop when removing child nodes from mat-icon~~ fix (mat-icon): reverse for loop when removing child nodes from mat-icon Jul 5, 2018

pavel-agarkov changed the title ~~fix (mat-icon): reverse for loop when removing child nodes from mat-icon~~ fix(icon): reverse for loop when removing child nodes from mat-icon Jul 5, 2018

crisbeto reviewed Jul 5, 2018

View reviewed changes

optimizing reverse loop

0f82c5e

pavel-agarkov changed the base branch from 6.3.x to master July 5, 2018 19:11

Merge branch 'master' into patch-1

08294bc

googlebot added cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels Jul 5, 2018

pavel-agarkov closed this Jul 5, 2018

pavel-agarkov deleted the patch-1 branch July 5, 2018 19:30

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(icon): reverse for loop when removing child nodes from mat-icon #12073

fix(icon): reverse for loop when removing child nodes from mat-icon #12073

Uh oh!

pavel-agarkov commented Jul 5, 2018

Uh oh!

googlebot commented Jul 5, 2018

Uh oh!

pavel-agarkov commented Jul 5, 2018

Uh oh!

googlebot commented Jul 5, 2018

Uh oh!

crisbeto Jul 5, 2018

Uh oh!

pavel-agarkov Jul 5, 2018

Uh oh!

crisbeto commented Jul 5, 2018

Uh oh!

googlebot commented Jul 5, 2018

Uh oh!

pavel-agarkov commented Jul 5, 2018

Uh oh!

pavel-agarkov commented Jul 5, 2018

Uh oh!

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

Uh oh!

Uh oh!

fix(icon): reverse for loop when removing child nodes from mat-icon #12073

fix(icon): reverse for loop when removing child nodes from mat-icon #12073

Uh oh!

Conversation

pavel-agarkov commented Jul 5, 2018

Uh oh!

googlebot commented Jul 5, 2018

What to do if you already signed the CLA

Individual signers

Corporate signers

Uh oh!

pavel-agarkov commented Jul 5, 2018

Uh oh!

googlebot commented Jul 5, 2018

Uh oh!

crisbeto Jul 5, 2018

Choose a reason for hiding this comment

Uh oh!

pavel-agarkov Jul 5, 2018

Choose a reason for hiding this comment

Uh oh!

crisbeto commented Jul 5, 2018

Uh oh!

googlebot commented Jul 5, 2018

Uh oh!

pavel-agarkov commented Jul 5, 2018

Uh oh!

pavel-agarkov commented Jul 5, 2018

Uh oh!

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

Uh oh!

Uh oh!