fix(icon): error when toggling icon with binding in IE11 #6102
Conversation
googlebot
added
the
cla: yes
src/lib/icon/icon.ts
Outdated
| svg => this._setSvgElement(svg), | ||
| (err: Error) => console.log(`Error retrieving icon: ${err.message}`)); | ||
| } | ||
| if ((changes.svgIcon || changes.svgSrc) && this.svgIcon) { |
There was a problem hiding this comment.
The svgSrc bits can go away; I removed support for svgSrc a long time ago but apparently missed this spot (and also the class description).
src/lib/icon/icon.ts
Outdated
|
|
||
| // Remove existing child nodes 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 child of layoutElement.childNodes) { |
src/lib/icon/icon.ts
Outdated
|
|
||
| // Remove existing child nodes 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 child of layoutElement.childNodes) { |
There was a problem hiding this comment.
TypeScript will compile this to a for loop that checks childNodes.length on every iteration. I know that used to be something to avoid, but not sure if it's necessary any more.
There was a problem hiding this comment.
Hmm, I guess it makes sense for the browser to recompute it. I'll cache it in a variable.
crisbeto
force-pushed
the
6093/icon-binding-error
branch
from
451638f to
56936a1
Compare
|
Addressed the feedback @jelbourn. |
jelbourn
added
pr: lgtm
action: merge
* Fixes an error that was being logged by IE11 for icons that are toggled via `ngIf` and have a binding expression. * Some minor readability improvements in the icon component. Fixes angular#6093.
crisbeto
force-pushed
the
6093/icon-binding-error
branch
from
56936a1 to
bf1ff01
Compare
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
ngIfand have a binding expression.Fixes #6093.