Skip to content

Commit

Permalink
fix(common): don't generate srcset if noopImageLoader is used (angula…
Browse files Browse the repository at this point in the history
…r#47804)

Do not generate a srcset if the loader being used is the default noopImageLoader. This loader does not take width into account, so it does not make sense to use it with srcsets.

PR Close angular#47804
  • Loading branch information
atcastle authored and nouraellm committed Nov 6, 2022
1 parent b03e0e4 commit e0db33f
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 3 deletions.
Expand Up @@ -418,7 +418,8 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {

if (this.ngSrcset) {
rewrittenSrcset = this.getRewrittenSrcset();
} else if (!this._disableOptimizedSrcset && !this.srcset) {
} else if (
!this._disableOptimizedSrcset && !this.srcset && this.imageLoader !== noopImageLoader) {
rewrittenSrcset = this.getAutomaticSrcset();
}

Expand Down
19 changes: 17 additions & 2 deletions packages/common/test/directives/ng_optimized_image_spec.ts
Expand Up @@ -1401,6 +1401,20 @@ describe('Image directive', () => {
return `${IMG_BASE_URL}/${config.src}${width}`;
};

it('should not generate a srcset if the default noop loader is used', () => {
setupTestingModule({noLoader: true});

const template = `
<img ngSrc="img" width="100" height="50" sizes="100vw">
`;
const fixture = createTestComponent(template);
fixture.detectChanges();

const nativeElement = fixture.nativeElement as HTMLElement;
const img = nativeElement.querySelector('img')!;
expect(img.getAttribute('srcset')).toBeNull();
});

it('should add a responsive srcset to the img element if sizes attribute exists', () => {
setupTestingModule({imageLoader});

Expand Down Expand Up @@ -1538,6 +1552,7 @@ class TestComponent {
function setupTestingModule(config?: {
imageConfig?: ImageConfig,
imageLoader?: ImageLoader,
noLoader?: boolean,
extraProviders?: Provider[],
component?: Type<unknown>
}) {
Expand All @@ -1548,8 +1563,8 @@ function setupTestingModule(config?: {
const loader = config?.imageLoader || defaultLoader;
const extraProviders = config?.extraProviders || [];
const providers: Provider[] = [
{provide: DOCUMENT, useValue: window.document}, {provide: IMAGE_LOADER, useValue: loader},
...extraProviders
{provide: DOCUMENT, useValue: window.document},
...(config?.noLoader ? [] : [{provide: IMAGE_LOADER, useValue: loader}]), ...extraProviders
];
if (config?.imageConfig) {
providers.push({provide: IMAGE_CONFIG, useValue: config.imageConfig});
Expand Down

0 comments on commit e0db33f

Please sign in to comment.