fix: replace the PNG image with a self explaining SVG image without t…

…ext (#625, #1296) * remove/replace the PNG image with a self explaining SVG image without text * separate handling for the not available image * image mapper improvement for products without images + code cleanup * set width and height attributes inside the SVG image file to prevent display issues (image and surrounding element collapses to 0) * update migration documentation BREAKING CHANGES: We removed the `not_available.png` image and replaced it with an SVG image (without text inside the image) and adjusted file references. Co-authored-by: Stefan Hauke <s.hauke@intershop.de>
intershop · Oct 14, 2022 · 9a984da · 9a984da
1 parent ef8e2ba
commit 9a984da
Show file tree

Hide file tree

Showing 15 changed files with 83 additions and 54 deletions.
diff --git a/docs/guides/migrations.md b/docs/guides/migrations.md
@@ -24,6 +24,10 @@ For that reason we removed it.
 Use the validator `equalTo` instead.
 Find more information in the method description in the [`special-validators.ts`](https://github.com/intershop/intershop-pwa/blob/3.0.0/src/app/shared/forms/validators/special-validators.ts#L82-L87).
 
+The "Product Image Not Available" PNG image `not_available.png` is removed and replaced by an SVG image `not-available.svg` which does not include a text inside the image any more to avoid localization issues.
+The file references are updated accordingly, the product image component is updated to use the correct image attributes, a localized alternative text is added and the product and image mapper files are updated to provide the correct data.
+In case the current PNG image file and the handling is customized in a project, you have to make sure to keep the project changes.
+
 ## 2.4 to 3.0
 
 With the 2.4.1 Hotfix we introduced a more fixed Node.js version handling to the version used and tested by us.

diff --git a/src/app/core/models/image/image.mapper.ts b/src/app/core/models/image/image.mapper.ts
@@ -12,7 +12,6 @@ import { Image } from './image.model';
  * @example
  * ImageMapper.fromImages(images)
  * ImageMapper.fromImage(image)
- * ImageMapper.fromImages(images)
  */
 @Injectable({ providedIn: 'root' })
 export class ImageMapper {
@@ -26,7 +25,6 @@ export class ImageMapper {
    * Maps Images to Images.
    *
    * @param images The source images.
-   * @param icmBaseURL The prefix URL for building absolute URLs for each relative URL.
    * @returns The images.
    */
   fromImages(images: Image[]): Image[] {
@@ -40,7 +38,6 @@ export class ImageMapper {
    * Maps Image to Image.
    *
    * @param image The source image.
-   * @param icmBaseURL The prefix URL for building absolute URLs for each relative URL.
    * @returns The image.
    */
   private fromImage(image: Image): Image {
@@ -54,7 +51,6 @@ export class ImageMapper {
    * Builds absolute URL from relative URL and icmBaseURL or returns absolute URL.
    *
    * @param url The relative or absolute image URL.
-   * @param icmBaseURL The prefix URL for building absolute URLs for each relative URL.
    * @returns The URL.
    */
   private fromEffectiveUrl(url: string): string {
@@ -66,4 +62,38 @@ export class ImageMapper {
     }
     return `${this.icmBaseURL}${url}`;
   }
+
+  /**
+   * Maps a single product image URL to a minimum product images array.
+   *
+   * @param url The image URL.
+   * @returns The minimum images (M and S image).
+   */
+  fromImageUrl(url: string): Image[] {
+    if (!url) {
+      return;
+    }
+    return [
+      {
+        effectiveUrl: this.fromEffectiveUrl(url),
+        name: 'front M',
+        primaryImage: true,
+        type: 'Image',
+        typeID: 'M',
+        viewID: 'front',
+        imageActualHeight: 270,
+        imageActualWidth: 270,
+      },
+      {
+        effectiveUrl: this.fromEffectiveUrl(url),
+        name: 'front S',
+        primaryImage: true,
+        type: 'Image',
+        typeID: 'S',
+        viewID: 'front',
+        imageActualHeight: 110,
+        imageActualWidth: 110,
+      },
+    ];
+  }
 }
diff --git a/src/app/core/models/product/__snapshots__/product.mapper.spec.ts.snap b/src/app/core/models/product/__snapshots__/product.mapper.spec.ts.snap
@@ -20,8 +20,8 @@ Object {
   "images": Array [
     Object {
       "effectiveUrl": "http://www.example.org/assets/product_img/a.jpg",
-      "imageActualHeight": undefined,
-      "imageActualWidth": undefined,
+      "imageActualHeight": 270,
+      "imageActualWidth": 270,
       "name": "front M",
       "primaryImage": true,
       "type": "Image",
@@ -30,8 +30,8 @@ Object {
     },
     Object {
       "effectiveUrl": "http://www.example.org/assets/product_img/a.jpg",
-      "imageActualHeight": undefined,
-      "imageActualWidth": undefined,
+      "imageActualHeight": 110,
+      "imageActualWidth": 110,
       "name": "front S",
       "primaryImage": true,
       "type": "Image",

diff --git a/src/app/core/models/product/product.mapper.spec.ts b/src/app/core/models/product/product.mapper.spec.ts
@@ -188,7 +188,7 @@ describe('Product Mapper', () => {
       expect(stub.name).toEqual('productName');
       expect(stub.shortDescription).toEqual('productDescription');
       expect(stub.sku).toEqual('productSKU');
-      verify(imageMapper.fromImages(anything())).once();
+      verify(imageMapper.fromImageUrl(anything())).once();
       verify(attachmentMapper.fromAttachments(anything())).never();
     });
 

diff --git a/src/app/core/models/product/product.mapper.ts b/src/app/core/models/product/product.mapper.ts
@@ -122,28 +122,7 @@ export class ProductMapper {
       shortDescription: data.description,
       name: data.title,
       sku,
-      images: this.imageMapper.fromImages([
-        {
-          effectiveUrl: retrieveStubAttributeValue(data, 'image'),
-          name: 'front M',
-          primaryImage: true,
-          type: 'Image',
-          typeID: 'M',
-          viewID: 'front',
-          imageActualHeight: undefined,
-          imageActualWidth: undefined,
-        },
-        {
-          effectiveUrl: retrieveStubAttributeValue(data, 'image'),
-          name: 'front S',
-          primaryImage: true,
-          type: 'Image',
-          typeID: 'S',
-          viewID: 'front',
-          imageActualHeight: undefined,
-          imageActualWidth: undefined,
-        },
-      ]),
+      images: this.imageMapper.fromImageUrl(retrieveStubAttributeValue(data, 'image')),
       manufacturer: retrieveStubAttributeValue(data, 'manufacturer'),
       available: this.calculateAvailable(
         retrieveStubAttributeValue(data, 'availability'),

diff --git a/src/app/pages/category/category-image/category-image.component.ts b/src/app/pages/category/category-image/category-image.component.ts
@@ -16,7 +16,7 @@ export class CategoryImageComponent implements OnChanges {
    */
   @Input() category: Category;
 
-  categoryImageUrl = '/assets/img/not_available.png';
+  categoryImageUrl = '/assets/img/not-available.svg';
 
   ngOnChanges() {
     this.setCategoryImageUrl();

diff --git a/src/app/pages/product/product-images/product-images.component.html b/src/app/pages/product/product-images/product-images.component.html
@@ -1,6 +1,6 @@
 <div *ngIf="product$ | async" class="row">
   <div class="col-lg-3 d-none d-lg-block">
-    <div class="product-img-thumbs" data-type="S">
+    <div class="product-img-thumbs">
       <div
         *ngFor="let imageView of getImageViewIDs$('S') | async; index as i"
         class="product-thumb-set"

diff --git a/src/app/shared/components/product/product-image/product-image.component.html b/src/app/shared/components/product/product-image/product-image.component.html
@@ -9,16 +9,25 @@
 </div>
 
 <ng-template #image>
-  <img
-    loading="lazy"
-    *ngIf="{ image: productImage$ | async } as product"
-    class="product-image"
-    [src]="product.image?.effectiveUrl || '/assets/img/not_available.png'"
-    [attr.data-type]="product.image?.typeID"
-    [attr.data-view]="product.image?.viewID"
-    [attr.height]="product.image?.imageActualHeight"
-    [attr.width]="product.image?.imageActualWidth"
-    [attr.alt]="altText || (defaultAltText$ | async)"
-    itemprop="image"
-  />
+  <ng-container *ngIf="productImage$ | async as image; else noImage">
+    <img
+      *ngIf="image.effectiveUrl; else noImage"
+      loading="lazy"
+      class="product-image"
+      [src]="image.effectiveUrl"
+      [attr.height]="image.imageActualHeight"
+      [attr.width]="image.imageActualWidth"
+      [attr.alt]="altText || (defaultAltText$ | async)"
+      itemprop="image"
+    />
+  </ng-container>
+
+  <ng-template #noImage>
+    <img
+      loading="lazy"
+      class="product-image"
+      src="/assets/img/not-available.svg"
+      [attr.alt]="'product.image.not_available.alttext' | translate"
+    />
+  </ng-template>
 </ng-template>
diff --git a/src/app/shared/components/product/product-image/product-image.component.spec.ts b/src/app/shared/components/product/product-image/product-image.component.spec.ts
@@ -1,10 +1,11 @@
 import { ComponentFixture, TestBed } from '@angular/core/testing';
 import { RouterTestingModule } from '@angular/router/testing';
 import { TranslateModule, TranslateService } from '@ngx-translate/core';
-import { EMPTY, of } from 'rxjs';
+import { of } from 'rxjs';
 import { anything, instance, mock, when } from 'ts-mockito';
 
 import { ProductContextFacade } from 'ish-core/facades/product-context.facade';
+import { Image } from 'ish-core/models/image/image.model';
 import { ProductView } from 'ish-core/models/product-view/product-view.model';
 
 import { ProductImageComponent } from './product-image.component';
@@ -18,7 +19,9 @@ describe('Product Image Component', () => {
 
   beforeEach(async () => {
     context = mock(ProductContextFacade);
-    when(context.getProductImage$(anything(), anything())).thenReturn(EMPTY);
+    when(context.getProductImage$(anything(), anything())).thenReturn(
+      of({ effectiveUrl: '/assets/product_img/a.jpg' } as Image)
+    );
     when(context.select('product')).thenReturn(of({} as ProductView));
     when(context.select('productURL')).thenReturn(of('/product/TEST'));
 
@@ -38,6 +41,7 @@ describe('Product Image Component', () => {
     translate.setDefaultLang('en');
     translate.use('en');
     translate.set('product.image.text.alttext', 'product photo');
+    translate.set('product.image.not_available.alttext', 'no product image available');
 
     component.imageType = 'S';
   });
@@ -48,17 +52,16 @@ describe('Product Image Component', () => {
     expect(() => fixture.detectChanges()).not.toThrow();
   });
 
-  it('should render N/A image when images is not available', () => {
+  it('should render N/A image when images are not available', () => {
     when(context.getProductImage$(anything(), anything())).thenReturn(of(undefined));
 
     fixture.detectChanges();
     expect(element.querySelector('img')?.attributes).toMatchInlineSnapshot(`
       NamedNodeMap {
-        "alt": "product photo",
+        "alt": "no product image available",
         "class": "product-image",
-        "itemprop": "image",
         "loading": "lazy",
-        "src": "/assets/img/not_available.png",
+        "src": "/assets/img/not-available.svg",
       }
     `);
   });
@@ -82,8 +85,6 @@ describe('Product Image Component', () => {
       NamedNodeMap {
         "alt": "product photo",
         "class": "product-image",
-        "data-type": "S",
-        "data-view": "front",
         "height": "110",
         "itemprop": "image",
         "loading": "lazy",
@@ -108,7 +109,7 @@ describe('Product Image Component', () => {
     );
 
     fixture.detectChanges();
-    expect(element.querySelector('img').getAttribute('src')).toMatchInlineSnapshot(`"/assets/img/not_available.png"`);
+    expect(element.querySelector('img').getAttribute('src')).toMatchInlineSnapshot(`"/assets/img/not-available.svg"`);
   });
 
   describe('image alt attribute', () => {

diff --git a/src/assets/i18n/de_DE.json b/src/assets/i18n/de_DE.json
@@ -855,6 +855,7 @@
   "product.description.heading": "Beschreibung",
   "product.details.heading": "Details",
   "product.email_a_friend.link": "Per E-Mail an einen Freund",
+  "product.image.not_available.alttext": "Kein Produktbild vorhanden",
   "product.image.text.alttext": "Produktbild",
   "product.instock.text": "Verfügbar",
   "product.itemNumber.label": "Artikelnummer:",

diff --git a/src/assets/i18n/en_US.json b/src/assets/i18n/en_US.json
@@ -855,6 +855,7 @@
   "product.description.heading": "Description",
   "product.details.heading": "Details",
   "product.email_a_friend.link": "E-mail a friend",
+  "product.image.not_available.alttext": "No product image available",
   "product.image.text.alttext": "product photo",
   "product.instock.text": "In Stock",
   "product.itemNumber.label": "Product ID:",

diff --git a/src/assets/i18n/fr_FR.json b/src/assets/i18n/fr_FR.json
@@ -855,6 +855,7 @@
   "product.description.heading": "Description",
   "product.details.heading": "Détails",
   "product.email_a_friend.link": "Envoyer un courriel à un(e) ami(e)",
+  "product.image.not_available.alttext": "Aucune image du produit disponible",
   "product.image.text.alttext": "photo du produit",
   "product.instock.text": "En stock",
   "product.itemNumber.label": "ID de produit :",

diff --git a/src/assets/img/not-available.svg b/src/assets/img/not-available.svg
diff --git a/src/assets/img/not_available.png b/src/assets/img/not_available.png
diff --git a/src/styles/pages/category/category-page.scss b/src/styles/pages/category/category-page.scss
@@ -72,6 +72,8 @@ ul {
     img {
       width: 270px;
       height: auto;
+      // fix layout shift at the image
+      aspect-ratio: 1/1;
     }
   }