Skip to content

Commit

Permalink
fix(common): don't generate srcset if noopImageLoader is used
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
atcastle authored and AndrewKushnir committed Oct 19, 2022
1 parent b31091b commit e716a69
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 3 deletions.
Expand Up @@ -417,7 +417,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 e716a69

Please sign in to comment.