fix(common): allow mixed pixel and responsive values in NgOptimizedIm… - #67070
fix(common): allow mixed pixel and responsive values in NgOptimizedIm…#67070Rajveer173 wants to merge 1 commit into
Conversation
|
It looked like an interesting change, any reason for closing it ? |
3331e9c to
d085b9e
Compare
…age sizes The assertNoComplexSizes validation in NgOptimizedImage previously rejected any sizes attribute containing pixel values, even when those pixel values appeared alongside responsive units like vw. This prevented valid use cases where a pixel fallback is intentionally combined with responsive media queries. The heuristic now only throws when the sizes attribute contains exclusively pixel values with no responsive units, which indicates the user likely wants fixed-size mode and should omit sizes to get proper 1x/2x density descriptors instead. Fixes angular#59495
d085b9e to
34c4c72
Compare
|
Hi @JeanMeche . Thanks for reopening! Rebased on latest main and the formatting issue is fixed. Happy to address any feedback. |
JeanMeche
left a comment
There was a problem hiding this comment.
AGENT: Great fix! Relaxing this constraint is the right approach to support responsive modes that use pixel fallbacks. The tests cover the new cases perfectly. I just have a minor suggestion to improve the robustness and performance of the regex used.
|
|
||
| const hasPixelValues = sizes.match(/((\)|,)\s|^)\d+px/); | ||
| const hasResponsiveValues = sizes.match(/\d+(vw|vh|vmin|vmax|%)/); | ||
|
|
There was a problem hiding this comment.
AGENT: The original regex /((\)|,)\s|^)\d+px/ actually has a bug: it fails to match if there is a leading space or multiple spaces (e.g., sizes=" 500px"). We can fix this, simplify the syntax, and use .test() instead of .match() for slightly better performance since we only need a boolean.
| const hasPixelValues = /(?:^|[),])\s*\d+px/.test(sizes); | |
| const hasResponsiveValues = /\d+(?:vw|vh|vmin|vmax|%)/.test(sizes); |
Fixes #59495
PR Checklist
PR Type
What is the current behavior?
The assertNoComplexSizes validation in NgOptimizedImage rejects any
sizesattribute containing pixel (px) values, even when those pixel values are used alongside responsive units such asvw.This prevents valid use cases where a pixel fallback is intentionally combined with responsive media queries.
What is the new behavior?
The heuristic now throws only when the
sizesattribute contains exclusively pixel values and no responsive units.If responsive units (e.g.,
vw) are present alongside pixel values, validation passes.This ensures:
sizesand rely on 1x/2x density descriptors.Does this PR introduce a breaking change?
Other information
This change aligns validation behavior with real-world responsive image patterns while preserving guidance for fixed-size usage.