From 80cce049ff9dd330d8741cecb20baf18fff2ac4f Mon Sep 17 00:00:00 2001
From: Daniel Macak <daniel.macak@email.cz>
Date: Thu, 10 Aug 2023 22:24:21 +0200
Subject: [PATCH] change!: turn item width into visible item count

This makes it declarative unlike before when the item count had to be
calculated from the item and viewer width, which not only saves code but
also resources.

One observation of mine is that this relatively simple change required
quite a few changes in the unit tests. It only shows that testing components
in Angular is pain (calling ngOnChanges, calling tick in fakeAsync etc.)
which makes a good argument for cy's component testing.
---
 apps/demo/src/app/app.component.html          |  4 +-
 apps/demo/src/app/app.component.ts            |  5 --
 .../demo-multiple-items.component.html        | 12 +---
 .../demo-multiple-items.component.ts          | 11 +---
 .../demo-whole-config.component.html          | 17 ++----
 .../demo-whole-config.component.ts            | 14 +----
 .../components/gallery/gallery.component.html |  2 +-
 .../components/gallery/gallery.component.ts   |  2 +-
 .../components/viewer/viewer.component.html   |  4 +-
 .../viewer/viewer.component.simple.spec.ts    | 27 ++------
 .../viewer/viewer.component.spec.ts           | 46 +++++++++++---
 .../lib/components/viewer/viewer.component.ts | 61 ++++++++-----------
 12 files changed, 82 insertions(+), 123 deletions(-)

diff --git a/apps/demo/src/app/app.component.html b/apps/demo/src/app/app.component.html
index bbc1cd4b..e043e638 100644
--- a/apps/demo/src/app/app.component.html
+++ b/apps/demo/src/app/app.component.html
@@ -67,10 +67,10 @@ <h2 id="demos">Demos</h2>
       stackblitz="https://stackblitz.com/edit/ngx-gallery-demo-infinite-loop?file=src%2Fapp%2Fapp.component.html"
     >
       <gallery
-        [items]="smallerImages"
+        [items]="images"
         [loop]="true"
         loading="lazy"
-        [itemWidth]="mobile ? '75%' : '34%'"
+        [visibleItems]="mobile ? 1.5 : 3"
       ></gallery>
     </app-showcase>
 
diff --git a/apps/demo/src/app/app.component.ts b/apps/demo/src/app/app.component.ts
index 8a7bfd8c..6dc41777 100644
--- a/apps/demo/src/app/app.component.ts
+++ b/apps/demo/src/app/app.component.ts
@@ -67,11 +67,6 @@ export class AppComponent {
     },
   ];
 
-  smallerImages = this.images.map(i => ({
-    ...i,
-    src: i.src.replace('-lg', '-md'),
-  }));
-
   extendedImages: GalleryItem[] = [
     ...this.images,
     {
diff --git a/apps/demo/src/app/components/demo-multiple-items/demo-multiple-items.component.html b/apps/demo/src/app/components/demo-multiple-items/demo-multiple-items.component.html
index 22ce95c5..21c24648 100644
--- a/apps/demo/src/app/components/demo-multiple-items/demo-multiple-items.component.html
+++ b/apps/demo/src/app/components/demo-multiple-items/demo-multiple-items.component.html
@@ -1,16 +1,6 @@
-<mat-form-field>
-  <mat-label>Item width</mat-label>
-  <input
-    name="itemWidth"
-    matInput
-    [(ngModel)]="itemWidth"
-    (change)="reloadGallery()"
-  />
-</mat-form-field>
-
 <gallery
   *ngIf="showGallery"
   [items]="items"
-  [itemWidth]="itemWidth"
+  [visibleItems]="mobile ? 1.5 : 2.5"
   loading="lazy"
 ></gallery>
diff --git a/apps/demo/src/app/components/demo-multiple-items/demo-multiple-items.component.ts b/apps/demo/src/app/components/demo-multiple-items/demo-multiple-items.component.ts
index f9ad5d6d..39c7ea08 100644
--- a/apps/demo/src/app/components/demo-multiple-items/demo-multiple-items.component.ts
+++ b/apps/demo/src/app/components/demo-multiple-items/demo-multiple-items.component.ts
@@ -3,8 +3,6 @@ import {
   ChangeDetectorRef,
   Component,
   Input,
-  OnChanges,
-  SimpleChanges,
 } from '@angular/core';
 import { GalleryItem } from '@daelmaak/ngx-gallery';
 
@@ -14,21 +12,14 @@ import { GalleryItem } from '@daelmaak/ngx-gallery';
   styleUrls: ['./demo-multiple-items.component.scss'],
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class DemoMultipleItemsComponent implements OnChanges {
+export class DemoMultipleItemsComponent {
   @Input() items: GalleryItem[];
   @Input() mobile: boolean;
 
-  itemWidth: string;
   showGallery = true;
 
   constructor(private cd: ChangeDetectorRef) {}
 
-  ngOnChanges({ mobile }: SimpleChanges) {
-    if (mobile && mobile.firstChange) {
-      this.itemWidth = this.mobile ? 'calc(100% / 1.5)' : 'calc(100% / 2.5)';
-    }
-  }
-
   reloadGallery() {
     this.showGallery = false;
     this.cd.detectChanges();
diff --git a/apps/demo/src/app/components/demo-whole-config/demo-whole-config.component.html b/apps/demo/src/app/components/demo-whole-config/demo-whole-config.component.html
index 4cd1f1ab..19aae67e 100644
--- a/apps/demo/src/app/components/demo-whole-config/demo-whole-config.component.html
+++ b/apps/demo/src/app/components/demo-whole-config/demo-whole-config.component.html
@@ -94,11 +94,11 @@ <h4>Gallery viewer properties</h4>
 
     <section>
       <mat-form-field>
-        <label>Item width</label>
+        <label>Number of displayed items</label>
         <input
-          name="itemWidth"
+          name="visibleItems"
           matInput
-          [(ngModel)]="galleryConfig.itemWidth"
+          [(ngModel)]="galleryConfig.visibleItems"
           (change)="reloadGallery()"
         />
       </mat-form-field>
@@ -203,7 +203,7 @@ <h4>Auxiliary</h4>
     [touchGestures]="galleryConfig.touchGestures"
     [counter]="galleryConfig.counter"
     [counterOrientation]="galleryConfig.counterOrientation"
-    [itemWidth]="galleryConfig.itemWidth"
+    [visibleItems]="galleryConfig.visibleItems"
     [objectFit]="galleryConfig.objectFit"
     [loading]="galleryConfig.loading"
     [loop]="galleryConfig.loop"
@@ -215,7 +215,6 @@ <h4>Auxiliary</h4>
     [thumbsArrows]="galleryConfig.thumbsArrows"
     [thumbsArrowSlideByLength]="galleryConfig.thumbsArrowSlideByLength"
     [thumbsScrollBehavior]="galleryConfig.thumbsScrollBehavior"
-    (imageClick)="onImageClick($event)"
     (descriptionClick)="galleryConfig.descriptions = false"
   ></gallery>
 </div>
@@ -234,12 +233,7 @@ <h4>Auxiliary</h4>
   <ng-container *ngIf="item.video; then videoTemplate; else imgTemplate">
   </ng-container>
   <ng-template #imgTemplate>
-    <img
-      #img
-      class="custom-media"
-      [src]="seen ? item.src : ''"
-      (load)="onImageLoad()"
-    />
+    <img #img class="custom-media" [src]="seen ? item.src : ''" />
     <div class="custom-loading" *ngIf="seen && img.naturalHeight <= 0">
       Custom item loading ...
     </div>
@@ -250,7 +244,6 @@ <h4>Auxiliary</h4>
       class="custom-media"
       [src]="seen ? item.src : ''"
       controls
-      (loadedmetadata)="onImageLoad()"
     ></video>
   </ng-template>
 </ng-template>
diff --git a/apps/demo/src/app/components/demo-whole-config/demo-whole-config.component.ts b/apps/demo/src/app/components/demo-whole-config/demo-whole-config.component.ts
index 152d3284..2e960bdd 100644
--- a/apps/demo/src/app/components/demo-whole-config/demo-whole-config.component.ts
+++ b/apps/demo/src/app/components/demo-whole-config/demo-whole-config.component.ts
@@ -11,7 +11,6 @@ import { delay, switchMap } from 'rxjs/operators';
 import {
   GalleryComponent,
   GalleryItem,
-  GalleryItemEvent,
   Loading,
   ObjectFit,
   Orientation,
@@ -26,7 +25,7 @@ interface GalleryConfig {
   touchGestures: boolean;
   counter: boolean;
   counterOrientation: VerticalOrientation;
-  itemWidth: string;
+  visibleItems: number;
   objectFit: ObjectFit;
   loading: Loading;
   loop: boolean;
@@ -62,7 +61,7 @@ export class DemoWholeConfigComponent implements OnInit {
     touchGestures: true,
     counter: true,
     counterOrientation: 'bottom',
-    itemWidth: '',
+    visibleItems: 1,
     objectFit: 'cover',
     loading: 'lazy',
     loop: false,
@@ -92,14 +91,7 @@ export class DemoWholeConfigComponent implements OnInit {
     window.addEventListener('pagehide', this.storeGalleryConfig);
   }
 
-  async onImageClick(event: GalleryItemEvent) {}
-
-  onImageLoad() {}
-
-  onConfigChange(
-    prop: keyof GalleryConfig,
-    value: GalleryConfig[keyof GalleryConfig]
-  ) {
+  onConfigChange(prop: keyof GalleryConfig, value: unknown) {
     if (value === '') {
       return;
     }
diff --git a/libs/gallery/src/lib/components/gallery/gallery.component.html b/libs/gallery/src/lib/components/gallery/gallery.component.html
index 722e2a11..e41064e0 100644
--- a/libs/gallery/src/lib/components/gallery/gallery.component.html
+++ b/libs/gallery/src/lib/components/gallery/gallery.component.html
@@ -30,7 +30,7 @@
   [touchGestures]="touchGestures"
   [counter]="counter"
   [counterOrientation]="counterOrientation"
-  [itemWidth]="itemWidth"
+  [visibleItems]="visibleItems"
   [objectFit]="objectFit"
   [loading]="loading"
   [itemTemplate]="itemTemplate"
diff --git a/libs/gallery/src/lib/components/gallery/gallery.component.ts b/libs/gallery/src/lib/components/gallery/gallery.component.ts
index 623aa032..aa3262a6 100644
--- a/libs/gallery/src/lib/components/gallery/gallery.component.ts
+++ b/libs/gallery/src/lib/components/gallery/gallery.component.ts
@@ -48,7 +48,7 @@ export class GalleryComponent {
   @Input() touchGestures = true;
   @Input() counter = true;
   @Input() counterOrientation: VerticalOrientation = 'bottom';
-  @Input() itemWidth: string;
+  @Input() visibleItems = 1;
   @Input() loading: Loading = 'lazy';
   @Input() loop = false;
   @Input() objectFit: ObjectFit = 'cover';
diff --git a/libs/gallery/src/lib/components/viewer/viewer.component.html b/libs/gallery/src/lib/components/viewer/viewer.component.html
index 309d8f9d..193e81b0 100644
--- a/libs/gallery/src/lib/components/viewer/viewer.component.html
+++ b/libs/gallery/src/lib/components/viewer/viewer.component.html
@@ -8,10 +8,10 @@
 </button>
 
 <ul #itemList [attr.aria-label]="aria?.viewerLabel">
-  <li *ngIf="!_displayedItems?.length" class="viewer-initial-item"></li>
+  <li *ngIf="!displayedItems?.length" class="viewer-initial-item"></li>
   <li
     #itemsRef
-    *ngFor="let item of _displayedItems; let i = index"
+    *ngFor="let item of displayedItems; let i = index"
     media
     [class.viewer-item--selected]="i === selectedIndex"
     [attr.tabindex]="itemTabbable(i)"
diff --git a/libs/gallery/src/lib/components/viewer/viewer.component.simple.spec.ts b/libs/gallery/src/lib/components/viewer/viewer.component.simple.spec.ts
index 520922b1..eee32be4 100644
--- a/libs/gallery/src/lib/components/viewer/viewer.component.simple.spec.ts
+++ b/libs/gallery/src/lib/components/viewer/viewer.component.simple.spec.ts
@@ -1,4 +1,4 @@
-import { ChangeDetectorRef } from '@angular/core';
+import { ChangeDetectorRef, SimpleChange } from '@angular/core';
 import { GalleryItemInternal } from '../../core/gallery-item';
 import { ViewerComponent } from './viewer.component';
 
@@ -26,32 +26,24 @@ describe('ViewerComponent', () => {
         { src: 'src3' },
         { src: 'src4' },
       ];
-      viewer['_viewerWidth'] = 600;
     });
 
     it('should be 1 when just 1 item visible in the viewport', () => {
-      viewer['_itemWidth'] = 600;
-      const fringeCount = viewer['getFringeCount']();
-
-      expect(fringeCount).toBe(1);
-    });
-
-    it('should be 1 when 1 item is visible but is by a fraction of a pixel slimmer than the scrollport', () => {
-      viewer['_itemWidth'] = 599.5;
+      viewer.visibleItems = 1;
       const fringeCount = viewer['getFringeCount']();
 
       expect(fringeCount).toBe(1);
     });
 
     it('should be 2 when gallery item is a little slimmer than scrollport', () => {
-      viewer['_itemWidth'] = 550;
+      viewer.visibleItems = 1.2;
       const fringeCount = viewer['getFringeCount']();
 
       expect(fringeCount).toBe(2);
     });
 
     it('should be 3 when 3 items visible', () => {
-      viewer['_itemWidth'] = 200;
+      viewer.visibleItems = 3;
       const fringeCount = viewer['getFringeCount']();
 
       expect(fringeCount).toBe(3);
@@ -123,15 +115,4 @@ describe('ViewerComponent', () => {
       });
     });
   });
-
-  describe('looping', () => {
-    beforeEach(() => {
-      viewer.loop = true;
-      viewer.items = [{ src: 'src1' }];
-    });
-
-    it('should disabled looping if there is just 1 item', () => {
-      expect(viewer.loop).toBe(false);
-    });
-  });
 });
diff --git a/libs/gallery/src/lib/components/viewer/viewer.component.spec.ts b/libs/gallery/src/lib/components/viewer/viewer.component.spec.ts
index c16ea46b..af27b585 100644
--- a/libs/gallery/src/lib/components/viewer/viewer.component.spec.ts
+++ b/libs/gallery/src/lib/components/viewer/viewer.component.spec.ts
@@ -3,7 +3,6 @@ import {
   Component,
   DebugElement,
   SimpleChange,
-  SimpleChanges,
   TemplateRef,
   ViewChild,
 } from '@angular/core';
@@ -83,10 +82,12 @@ describe('ViewerComponent', () => {
     }));
 
     it('should preselect item based on given index', fakeAsync(() => {
+      component.visibleItems = 1;
       component.selectedIndex = 1;
       component.items = [{ src: 'src1' }, { src: 'src2' }];
       const changes = {
-        items: new SimpleChange(null, component.items, true),
+        visibleItems: new SimpleChange(undefined, component.visibleItems, true),
+        items: new SimpleChange(undefined, component.items, true),
       };
       component.ngOnChanges(changes);
       fixture.detectChanges();
@@ -95,6 +96,21 @@ describe('ViewerComponent', () => {
 
       expect(getSlidePx().index).toBe(-1);
     }));
+
+    it('looping should be disabled if there is just 1 item', fakeAsync(() => {
+      component.loop = true;
+      component.items = [{ src: 'src1' }];
+      component.visibleItems = 1;
+      const changes = {
+        items: new SimpleChange(null, component.items, true),
+      };
+      component.ngOnChanges(changes);
+      fixture.detectChanges();
+      tick();
+
+      expect(component.loop).toBe(false);
+      expect(component.displayedItems.length).toBe(1);
+    }));
   });
 
   describe('class attribute', () => {
@@ -245,8 +261,6 @@ describe('ViewerComponent', () => {
 
     describe('with looping on', () => {
       beforeEach(() => {
-        spyOn(component, 'shift' as any);
-        spyOn(component, 'center' as any);
         component.loop = true;
         fixture.detectChanges();
       });
@@ -313,12 +327,18 @@ describe('ViewerComponent', () => {
       component.mouseGestures = true;
       component.touchGestures = true;
       component.selectedIndex = 0;
+      component.visibleItems = 1;
     });
 
     describe('in loop mode with 2 (1 on each side) fringe items', () => {
       beforeEach(fakeAsync(() => {
         component.loop = true;
         component.ngOnChanges({
+          visibleItems: new SimpleChange(
+            undefined,
+            component.visibleItems,
+            true
+          ),
           items: new SimpleChange(null, component.items, true),
         });
         fixture.detectChanges();
@@ -413,9 +433,15 @@ describe('ViewerComponent', () => {
 
     describe('with looping off', () => {
       beforeEach(fakeAsync(() => {
+        component.visibleItems = 1;
         component.loop = false;
         component.ngOnChanges({
-          items: new SimpleChange(null, component.items, true),
+          visibleItems: new SimpleChange(
+            undefined,
+            component.visibleItems,
+            true
+          ),
+          items: new SimpleChange(undefined, component.items, true),
         });
         fixture.detectChanges();
         viewerDe.nativeElement.style.width = ITEM_WIDTH + 'px';
@@ -531,9 +557,15 @@ describe('ViewerComponent', () => {
 
     describe('in right to left mode', () => {
       beforeEach(fakeAsync(() => {
+        component.visibleItems = 1;
         component.loop = false;
         component.isRtl = true;
         component.ngOnChanges({
+          visibleItems: new SimpleChange(
+            undefined,
+            component.visibleItems,
+            true
+          ),
           items: new SimpleChange(null, component.items, true),
         });
         fixture.detectChanges();
@@ -587,9 +619,7 @@ describe('ViewerComponent', () => {
       };
     }
 
-    const [_, index, tweak] = transform.match(
-      /3d\(calc\((\-?\d+).*\s(\d+)px\)/
-    );
+    const [, index, tweak] = transform.match(/3d\(calc\((-?\d+).*\s(\d+)px\)/);
 
     return {
       index: +index,
diff --git a/libs/gallery/src/lib/components/viewer/viewer.component.ts b/libs/gallery/src/lib/components/viewer/viewer.component.ts
index 4e84f003..88a86ec6 100644
--- a/libs/gallery/src/lib/components/viewer/viewer.component.ts
+++ b/libs/gallery/src/lib/components/viewer/viewer.component.ts
@@ -67,7 +67,7 @@ export const NEXT_THRESHOLD_PX = 25;
   ],
 })
 export class ViewerComponent implements OnChanges, OnInit {
-  @Input() items: GalleryItemInternal[];
+  @Input() items?: GalleryItemInternal[];
   @Input() arrows: boolean;
   @Input() selectedIndex: number;
   @Input() descriptions: boolean;
@@ -85,20 +85,9 @@ export class ViewerComponent implements OnChanges, OnInit {
   @Input() contentTemplate: TemplateRef<ContentTemplateContext>;
   @Input() thumbsOrientation: OrientationFlag;
   @Input() aria: Aria;
-  @Input()
-  set loop(val: boolean) {
-    this._loop = val;
-  }
-  get loop() {
-    return this.items && this.items.length > 1 && this._loop;
-  }
+  @Input() loop: boolean;
 
-  @Input() set itemWidth(width: string) {
-    this.itemListRef.nativeElement.style.setProperty(
-      '--item-width',
-      width ?? '100%'
-    );
-  }
+  @Input() visibleItems: number;
   @Input() touched: boolean;
 
   @HostBinding('class.rtl')
@@ -112,13 +101,9 @@ export class ViewerComponent implements OnChanges, OnInit {
   @ViewChild('itemList', { static: true }) itemListRef: ElementRef<HTMLElement>;
   @ViewChildren('itemsRef') itemsRef: QueryList<ElementRef<HTMLElement>>;
 
-  UA = UA;
-
-  _displayedItems: GalleryItemInternal[];
-  _fringeCount: number;
+  displayedItems: GalleryItemInternal[];
+  fringeCount: number;
   private _itemWidth: number;
-  private _loop: boolean;
-  private _viewerWidth: number;
   private pointerDeltaX = 0;
 
   set _noAnimation(value: boolean) {
@@ -149,8 +134,17 @@ export class ViewerComponent implements OnChanges, OnInit {
     private _zone: NgZone
   ) {}
 
-  ngOnChanges({ items }: SimpleChanges) {
-    if (items?.currentValue?.length) {
+  ngOnChanges({ visibleItems, items, loop }: SimpleChanges) {
+    if (loop || items) {
+      this.loop = this.items?.length > 1 ? this.loop : false;
+      this.fringeCount = this.getFringeCount();
+      this.displayedItems = this.getItemsToBeDisplayed(this.fringeCount);
+    }
+    if (visibleItems) {
+      this.itemListRef.nativeElement.style.setProperty(
+        '--item-width',
+        `calc(100% / ${this.visibleItems})`
+      );
       this.onResize();
     }
   }
@@ -209,7 +203,7 @@ export class ViewerComponent implements OnChanges, OnInit {
   }
 
   onTab(nextItemIndex: number) {
-    nextItemIndex = nextItemIndex - this._fringeCount;
+    nextItemIndex = nextItemIndex - this.fringeCount;
     // allow focus to escape viewer
     if (nextItemIndex >= 0 && nextItemIndex < this.items.length) {
       this.select(nextItemIndex);
@@ -238,7 +232,7 @@ export class ViewerComponent implements OnChanges, OnInit {
   }
 
   itemTabbable(index: number) {
-    index = index - this._fringeCount;
+    index = index - this.fringeCount;
     return index >= 0 && index < this.items.length ? 0 : -1;
   }
 
@@ -252,19 +246,16 @@ export class ViewerComponent implements OnChanges, OnInit {
 
   private getFringeCount() {
     return this.loop
-      ? Math.min(
-          Math.ceil(this._viewerWidth / (this._itemWidth + 1)),
-          this.items.length
-        )
+      ? Math.min(Math.ceil(this.visibleItems), this.items.length)
       : 0;
   }
 
-  private getItemsToBeDisplayed() {
+  private getItemsToBeDisplayed(fringeCount: number) {
     return this.loop
       ? [
-          ...this.items.slice(-this._fringeCount),
+          ...this.items.slice(-fringeCount),
           ...this.items,
-          ...this.items.slice(0, this._fringeCount),
+          ...this.items.slice(0, fringeCount),
         ]
       : this.items;
   }
@@ -448,11 +439,8 @@ export class ViewerComponent implements OnChanges, OnInit {
   };
 
   private readDimensions() {
-    this._viewerWidth = this._hostRef.nativeElement.offsetWidth;
     this._itemWidth =
       this._hostRef.nativeElement.querySelector('li').offsetWidth;
-    this._fringeCount = this.getFringeCount();
-    this._displayedItems = this.getItemsToBeDisplayed();
   }
 
   private selectBySwipeStats(distance: number) {
@@ -464,7 +452,7 @@ export class ViewerComponent implements OnChanges, OnInit {
 
   private shift(delta = 0) {
     const multiplier = this.isRtl ? 1 : -1;
-    const index = (this.selectedIndex + this._fringeCount) * multiplier;
+    const index = (this.selectedIndex + this.fringeCount) * multiplier;
     delta *= -multiplier;
     const shift = `calc(${index} * var(--item-width) + ${delta}px)`;
 
@@ -487,9 +475,8 @@ export class ViewerComponent implements OnChanges, OnInit {
   }
 
   private updateDimensions() {
-    if (this.items && this.items.length) {
+    if (this.items?.length) {
       this.readDimensions();
-      this._cd.detectChanges();
     }
   }
 }