From 3bd85fb7b0723ed807bca771e9fa95af60a3cfaf Mon Sep 17 00:00:00 2001 From: Alex Castle Date: Mon, 30 Oct 2023 14:25:24 -0700 Subject: [PATCH] fix(common): apply fixed_srcset_width value only to fixed srcsets (#52459) add logic to NgOptimizedImage to keep fixed_srcset_width from applying to large responsive images, which is incorrect behavior PR Close #52459 --- .../ng_optimized_image/ng_optimized_image.ts | 7 +++- .../directives/ng_optimized_image_spec.ts | 32 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts b/packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts index 4152cbea591f2..6ca58c38605c5 100644 --- a/packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts +++ b/packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts @@ -513,8 +513,13 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy { } private shouldGenerateAutomaticSrcset(): boolean { + let oversizedImage = false; + if (!this.sizes) { + oversizedImage = + this.width! > FIXED_SRCSET_WIDTH_LIMIT || this.height! > FIXED_SRCSET_HEIGHT_LIMIT; + } return !this.disableOptimizedSrcset && !this.srcset && this.imageLoader !== noopImageLoader && - !(this.width! > FIXED_SRCSET_WIDTH_LIMIT || this.height! > FIXED_SRCSET_HEIGHT_LIMIT); + !oversizedImage; } /** @nodoc */ diff --git a/packages/common/test/directives/ng_optimized_image_spec.ts b/packages/common/test/directives/ng_optimized_image_spec.ts index 3cd1f03b2012c..302a49c93c7ab 100644 --- a/packages/common/test/directives/ng_optimized_image_spec.ts +++ b/packages/common/test/directives/ng_optimized_image_spec.ts @@ -1797,6 +1797,38 @@ describe('Image directive', () => { expect(img.getAttribute('srcset')).toBeNull(); }); + it('should add a responsive srcset to the img element if height is too large', () => { + setupTestingModule({imageLoader}); + + const template = ``; + const fixture = createTestComponent(template); + fixture.detectChanges(); + + const nativeElement = fixture.nativeElement as HTMLElement; + const img = nativeElement.querySelector('img')!; + expect(img.getAttribute('srcset')) + .toBe(`${IMG_BASE_URL}/img?w=640 640w, ${IMG_BASE_URL}/img?w=750 750w, ${ + IMG_BASE_URL}/img?w=828 828w, ${IMG_BASE_URL}/img?w=1080 1080w, ${ + IMG_BASE_URL}/img?w=1200 1200w, ${IMG_BASE_URL}/img?w=1920 1920w, ${ + IMG_BASE_URL}/img?w=2048 2048w, ${IMG_BASE_URL}/img?w=3840 3840w`); + }); + + it('should add a responsive srcset to the img element if width is too large', () => { + setupTestingModule({imageLoader}); + + const template = ``; + const fixture = createTestComponent(template); + fixture.detectChanges(); + + const nativeElement = fixture.nativeElement as HTMLElement; + const img = nativeElement.querySelector('img')!; + expect(img.getAttribute('srcset')) + .toBe(`${IMG_BASE_URL}/img?w=640 640w, ${IMG_BASE_URL}/img?w=750 750w, ${ + IMG_BASE_URL}/img?w=828 828w, ${IMG_BASE_URL}/img?w=1080 1080w, ${ + IMG_BASE_URL}/img?w=1200 1200w, ${IMG_BASE_URL}/img?w=1920 1920w, ${ + IMG_BASE_URL}/img?w=2048 2048w, ${IMG_BASE_URL}/img?w=3840 3840w`); + }); + it('should use a custom breakpoint set if one is provided', () => { const imageConfig = { breakpoints: [16, 32, 48, 64, 96, 128, 256, 384, 640, 1280, 3840],