Skip to content

Commit

Permalink
feat: Picture element in media component to improve responsiveness (#…
Browse files Browse the repository at this point in the history
…17841)

Closes: CXSPA-4114
  • Loading branch information
rmch91 authored Nov 7, 2023
1 parent 567264e commit 06d299f
Show file tree
Hide file tree
Showing 11 changed files with 339 additions and 76 deletions.
12 changes: 10 additions & 2 deletions projects/storefrontapp/src/app/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,17 @@ import {
FeaturesConfig,
I18nConfig,
OccConfig,
provideConfig,
RoutingConfig,
TestConfigModule,
provideConfig,
} from '@spartacus/core';
import { StoreFinderConfig } from '@spartacus/storefinder/core';
import { GOOGLE_MAPS_DEVELOPMENT_KEY_CONFIG } from '@spartacus/storefinder/root';
import { AppRoutingModule, StorefrontComponent } from '@spartacus/storefront';
import {
AppRoutingModule,
StorefrontComponent,
USE_LEGACY_MEDIA_COMPONENT,
} from '@spartacus/storefront';
import { environment } from '../environments/environment';
import { TestOutletModule } from '../test-outlets/test-outlet.module';
import { SpartacusModule } from './spartacus/spartacus.module';
Expand Down Expand Up @@ -94,6 +98,10 @@ if (!environment.production) {
// without a key, for development or demo purposes.
googleMaps: { apiKey: GOOGLE_MAPS_DEVELOPMENT_KEY_CONFIG },
}),
{
provide: USE_LEGACY_MEDIA_COMPONENT,
useValue: false,
},
],
bootstrap: [StorefrontComponent],
})
Expand Down
2 changes: 2 additions & 0 deletions projects/storefrontlib/shared/components/media/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
* SPDX-License-Identifier: Apache-2.0
*/

export * from './media-sources.pipe';
export * from './media.component';
export * from './media.config';
export * from './media.model';
export * from './media.module';
export * from './media.service';
export * from './media.token';
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* SPDX-FileCopyrightText: 2023 SAP Spartacus team <[email protected]>
*
* SPDX-License-Identifier: Apache-2.0
*/

import { TestBed } from '@angular/core/testing';
import { MediaSourcesPipe } from './media-sources.pipe';

const mockImgUrl = 'https://test.url/image';
const mockSrcset = `${mockImgUrl} 96w, ${mockImgUrl} 284w`;

describe('MediaSourcesPipe', () => {
let pipe: MediaSourcesPipe;

beforeEach(() => {
TestBed.configureTestingModule({
providers: [MediaSourcesPipe],
});

pipe = TestBed.inject(MediaSourcesPipe);
});

it('should create an instance', () => {
expect(pipe).toBeTruthy();
});

it('should transform the srcset string into an array of sources with widths', () => {
const result = pipe.transform(mockSrcset);

expect(result.length).toBe(2);
expect(result[0].srcset).toBe(mockImgUrl);
expect(result[0].media).toBe('(min-width: 284px)');
expect(result[1].srcset).toBe(mockImgUrl);
expect(result[1].media).toBe('(min-width: 96px)');
});

it('should handle srcset with missing widths', () => {
const srcset = `${mockImgUrl}, ${mockImgUrl} 284w`;

const result = pipe.transform(srcset);

expect(result.length).toBe(1);
expect(result[0].srcset).toBe(mockImgUrl);
expect(result[0].media).toBe('(min-width: 284px)');
});

it('should handle srcset with missing URLs', () => {
const srcset = `96w, ${mockImgUrl} 284w`;

const result = pipe.transform(srcset);
console.log(result);

expect(result.length).toBe(1);
expect(result[0].srcset).toBe(mockImgUrl);
expect(result[0].media).toBe('(min-width: 284px)');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* SPDX-FileCopyrightText: 2023 SAP Spartacus team <[email protected]>
*
* SPDX-License-Identifier: Apache-2.0
*/

import { Pipe, PipeTransform } from '@angular/core';

@Pipe({
name: 'cxMediaSources',
})
export class MediaSourcesPipe implements PipeTransform {
transform(sizes: string) {
const sources: Pick<HTMLSourceElement, 'srcset' | 'media'>[] = [];

return sizes.split(/,\s*/).reduceRight((acc, set) => {
let [srcset, media] = set.split(' ');

if (!srcset || !media) {
return acc;
}

media = `(min-width: ${media.replace('w', 'px')})`;

acc.push({ srcset, media });

return acc;
}, sources);
}
}
35 changes: 25 additions & 10 deletions projects/storefrontlib/shared/components/media/media.component.html
Original file line number Diff line number Diff line change
@@ -1,10 +1,25 @@
<img
*ngIf="media && media.src"
[attr.src]="media.src"
[attr.srcset]="media.srcset"
[attr.alt]="media.alt"
[attr.role]="media.role"
[attr.loading]="loading"
(load)="loadHandler()"
(error)="errorHandler()"
/>
<picture *ngIf="media?.srcset && !isLegacy; else tmpImg">
<source
*ngFor="
let source of media!.srcset! | cxMediaSources;
trackBy: trackByMedia
"
[srcset]="source.srcset"
[media]="source.media"
/>

<ng-container [ngTemplateOutlet]="tmpImg"></ng-container>
</picture>

<ng-template #tmpImg>
<img
*ngIf="media && media.src"
[alt]="media.alt"
[src]="media.src"
[srcset]="media.srcset || media.src"
[attr.role]="media.role"
[loading]="loading"
(load)="loadHandler()"
(error)="errorHandler()"
/>
</ng-template>
149 changes: 125 additions & 24 deletions projects/storefrontlib/shared/components/media/media.component.spec.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,50 @@
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { Pipe, PipeTransform } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import { By } from '@angular/platform-browser';
import { MediaComponent } from './media.component';
import { ImageLoadingStrategy, Media } from './media.model';
import { MediaService } from './media.service';
import { USE_LEGACY_MEDIA_COMPONENT } from './media.token';

const mediaUrl = 'mockProductImageUrl.jpg';

@Pipe({
name: 'cxMediaSources',
})
export class MockMediaSourcesPipe implements PipeTransform {
transform() {
return [
{
srcset: mediaUrl,
media: '(min-width: 1400px)',
},
{
srcset: mediaUrl,
media: '(min-width: 1140px)',
},
{
srcset: mediaUrl,
media: '(min-width: 770px)',
},
{
srcset: mediaUrl,
media: '(min-width: 400px)',
},
];
}
}

class MockMediaService {
getMedia(media): Media {
srcset: string | undefined;

constructor(srcset?: string) {
this.srcset = srcset;
}

getMedia(media: any): Media {
return {
src: media ? media.product.url : undefined,
srcset: undefined,
srcset: this.srcset,
alt: undefined,
};
}
Expand All @@ -28,37 +62,56 @@ const mockImageContainer = {
product: { url: mediaUrl },
};

const mockMissingImageContainer = null;
const mockMissingImageContainer = undefined;

function configureTestingModule(
mockMediaService: MockMediaService,
isLegacy: boolean = true
): void {
TestBed.configureTestingModule({
declarations: [MediaComponent, MockMediaSourcesPipe],
providers: [
{ provide: MediaService, useValue: mockMediaService },
{
provide: USE_LEGACY_MEDIA_COMPONENT,
useValue: isLegacy,
},
],
}).compileComponents();
}

describe('MediaComponent', () => {
let component: MediaComponent;
let fixture: ComponentFixture<MediaComponent>;
let service: MediaService;

beforeEach(() => {
TestBed.configureTestingModule({
declarations: [MediaComponent],
providers: [{ provide: MediaService, useClass: MockMediaService }],
}).compileComponents();

fixture = TestBed.createComponent(MediaComponent);
service = TestBed.inject(MediaService);
component = fixture.componentInstance;
component.container = mockImageContainer;
function createComponent() {
const service = TestBed.inject(MediaService);
const fixture = TestBed.createComponent(MediaComponent);
const component = fixture.componentInstance;

component.ngOnChanges();
fixture.detectChanges();
});
component.container = mockImageContainer;

component.ngOnChanges();
fixture.detectChanges();

return { service, fixture, component };
}

describe('MediaComponent', () => {
it('should create', () => {
configureTestingModule(new MockMediaService());
const { component } = createComponent();

expect(component).toBeTruthy();
});

it('should create media object with valid image url', () => {
expect(component.media.src).toEqual(mediaUrl);
configureTestingModule(new MockMediaService());
const { component } = createComponent();

expect(component?.media?.src).toEqual(mediaUrl);
});

it('should update the img element with image url', () => {
configureTestingModule(new MockMediaService());
const { fixture } = createComponent();

expect(
(<HTMLImageElement>(
fixture.debugElement.query(By.css('img')).nativeElement
Expand All @@ -67,14 +120,21 @@ describe('MediaComponent', () => {
});

it('should not contain the loading attribute for the image element', () => {
configureTestingModule(new MockMediaService());
const { fixture } = createComponent();

const el: HTMLElement = <HTMLImageElement>(
fixture.debugElement.query(By.css('img')).nativeElement
);

fixture.detectChanges();
expect(el.getAttribute('loading')).toBeNull();
expect(JSON.parse(el.getAttribute('loading') as string)).toBeNull();
});

it('should contain loading="lazy" for the image element', () => {
configureTestingModule(new MockMediaService());
const { service } = createComponent();

spyOnProperty(service, 'loadingStrategy').and.returnValue(
ImageLoadingStrategy.LAZY
);
Expand All @@ -90,12 +150,18 @@ describe('MediaComponent', () => {
});

it('should contain is-loading classes while loading', () => {
configureTestingModule(new MockMediaService());
const { fixture } = createComponent();

expect(
(<HTMLImageElement>fixture.debugElement.nativeElement).classList
).toContain('is-loading');
});

it('should update classes when loaded', () => {
configureTestingModule(new MockMediaService());
const { fixture } = createComponent();

const load = new UIEvent('load');
fixture.debugElement.query(By.css('img')).nativeElement.dispatchEvent(load);

Expand All @@ -110,6 +176,14 @@ describe('MediaComponent', () => {
});

it('should have is-missing class when there is no image', () => {
configureTestingModule(new MockMediaService());
const { service, fixture, component } = createComponent();

component.container = mockImageContainer;

component.ngOnChanges();
fixture.detectChanges();

spyOn(service, 'getMedia').and.returnValue(null);
component.container = mockMissingImageContainer;

Expand All @@ -120,4 +194,31 @@ describe('MediaComponent', () => {
(<HTMLImageElement>fixture.debugElement.nativeElement).classList
).toContain('is-missing');
});

it('should not have picture element if there is no srcset in media', () => {
configureTestingModule(new MockMediaService());
const { fixture } = createComponent();

const picture = fixture.debugElement.query(By.css('picture'));

expect(picture).toBeNull();
});

it('should have picture element if there is srcset in media', () => {
configureTestingModule(new MockMediaService('srcset'), false);
const { fixture } = createComponent();

const picture = fixture.debugElement.query(By.css('picture'));

expect(picture).not.toBeNull();
});

it('should not have picture element if there is srcset in media but isLegacy mode', () => {
configureTestingModule(new MockMediaService('srcset'));
const { fixture } = createComponent();

const picture = fixture.debugElement.query(By.css('picture'));

expect(picture).toBeNull();
});
});
Loading

0 comments on commit 06d299f

Please sign in to comment.