Skip to content

Conversation

@arturovt
Copy link
Contributor

Prior to this commit, the ImagePerformanceWarning class was checking all img elements in the DOM to determine whether they were oversized after the DOM loading was complete. We should not check SVGs because they are vector-based and can scale infinitely without losing quality.

Closes #57941

@angular-robot angular-robot bot added the area: common Issues related to APIs in the @angular/common package label Sep 26, 2024
@ngbot ngbot bot added this to the Backlog milestone Sep 26, 2024
@arturovt arturovt marked this pull request as ready for review September 26, 2024 05:59
@pullapprove pullapprove bot requested a review from dylhunn September 26, 2024 06:00
@JeanMeche JeanMeche requested review from AndrewKushnir and removed request for dylhunn September 26, 2024 09:40
@arturovt arturovt force-pushed the fix/issue_57941 branch 2 times, most recently from 8ffb188 to cc99730 Compare September 26, 2024 13:25
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

@arturovt thanks for the PR, the code looks good 👍 Could you please also add a test to verify the new behavior and avoid regressions?

@AndrewKushnir AndrewKushnir added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Sep 26, 2024
@AndrewKushnir AndrewKushnir added action: presubmit The PR is in need of a google3 presubmit and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Sep 27, 2024
@AndrewKushnir
Copy link
Contributor

Presubmit.

@AndrewKushnir AndrewKushnir added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Sep 27, 2024
@AndrewKushnir
Copy link
Contributor

@arturovt thanks for adding tests 👍 It looks like one of them is failing on CI (see test CI job):

packages/core/test/bundling/image-directive/index.ts:21:51 - error TS2307: Cannot find module './e2e/image-perf-warnings-oversized/svg-no-perf-oversized-warnings' or its corresponding type declarations.

21 import {SvgNoOversizedPerfWarningsComponent} from './e2e/image-perf-warnings-oversized/svg-no-perf-oversized-warnings';
                                                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Could you please take a look when you get a chance?

Prior to this commit, the `ImagePerformanceWarning` class was checking all `img`
elements in the DOM to determine whether they were oversized after the DOM loading
was complete. We should not check SVGs because they are vector-based and can scale
infinitely without losing quality.

Closes angular#57941
@AndrewKushnir AndrewKushnir added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews action: presubmit The PR is in need of a google3 presubmit labels Sep 28, 2024
@AndrewKushnir
Copy link
Contributor

Caretaker note: presubmit is "green", this PR is ready for merge.

@alxhub
Copy link
Member

alxhub commented Sep 30, 2024

This PR was merged into the repository by commit e8b2d5f.

The changes were merged into the following branches: main, 18.2.x

@alxhub alxhub closed this in e8b2d5f Sep 30, 2024
alxhub pushed a commit that referenced this pull request Sep 30, 2024
Prior to this commit, the `ImagePerformanceWarning` class was checking all `img`
elements in the DOM to determine whether they were oversized after the DOM loading
was complete. We should not check SVGs because they are vector-based and can scale
infinitely without losing quality.

Closes #57941

PR Close #57966
@arturovt arturovt deleted the fix/issue_57941 branch September 30, 2024 20:35
@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 Oct 31, 2024
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 area: common Issues related to APIs in the @angular/common package merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Runtime warning NG0913 triggered for 300 kB svg file

4 participants