From 319ad43b5fc2f357293eb9f0b53fdb56403cd950 Mon Sep 17 00:00:00 2001 From: Daniel Macak Date: Sat, 2 Dec 2023 12:25:34 +0100 Subject: [PATCH] fix: make `items` always defined The whole codebase relies on `items` being always defined yet this contract was broken in v3 release. I use ngOnChanges since a simple default field value is not enough in cases where `undefined` is assigned to GalleryComponent explicitly. In the test I started using `setInput` which, unlike simple assignment, correctly runs ngOnChanges and marks even OnPush component for check. The default ng's interface for the method is poor however as there is no `keyof Component` checking so I provided my own to cover this blind spot. --- .../components/gallery/gallery.component.spec.ts | 13 ++++++++++++- .../src/lib/components/gallery/gallery.component.ts | 12 ++++++++++-- .../lib/components/viewer/viewer.component.spec.ts | 2 ++ .../src/lib/components/viewer/viewer.component.ts | 2 +- libs/gallery/src/lib/core/ng.ts | 6 ++++++ 5 files changed, 31 insertions(+), 4 deletions(-) create mode 100644 libs/gallery/src/lib/core/ng.ts diff --git a/libs/gallery/src/lib/components/gallery/gallery.component.spec.ts b/libs/gallery/src/lib/components/gallery/gallery.component.spec.ts index 146869c6..7c1fadbf 100644 --- a/libs/gallery/src/lib/components/gallery/gallery.component.spec.ts +++ b/libs/gallery/src/lib/components/gallery/gallery.component.spec.ts @@ -1,16 +1,18 @@ import { DebugElement } from '@angular/core'; import { ComponentFixture, - fakeAsync, TestBed, + fakeAsync, tick, } from '@angular/core/testing'; import { By } from '@angular/platform-browser'; import { NoopAnimationsModule } from '@angular/platform-browser/animations'; +import type { StrictComponentRef } from '../../core/ng'; import { GalleryComponent } from './gallery.component'; describe('GalleryComponent', () => { let component: GalleryComponent; + let componentRef: StrictComponentRef; let fixture: ComponentFixture; let de: DebugElement; @@ -23,6 +25,7 @@ describe('GalleryComponent', () => { beforeEach(() => { fixture = TestBed.createComponent(GalleryComponent); component = fixture.componentInstance; + componentRef = fixture.componentRef; de = fixture.debugElement; }); @@ -31,6 +34,14 @@ describe('GalleryComponent', () => { expect(component).toBeTruthy(); }); + it('should handle empty items input in looping mode', () => { + componentRef.setInput('items', undefined); + componentRef.setInput('loop', true); + fixture.detectChanges(); + + expect().nothing(); + }); + describe('emitters', () => { beforeEach(fakeAsync(() => { component.items = [{ src: 'src1' }, { src: 'src2' }]; diff --git a/libs/gallery/src/lib/components/gallery/gallery.component.ts b/libs/gallery/src/lib/components/gallery/gallery.component.ts index cd3e4072..ca0f48b4 100644 --- a/libs/gallery/src/lib/components/gallery/gallery.component.ts +++ b/libs/gallery/src/lib/components/gallery/gallery.component.ts @@ -7,7 +7,9 @@ import { HostBinding, HostListener, Input, + OnChanges, Output, + SimpleChanges, TemplateRef, ViewChild, } from '@angular/core'; @@ -36,11 +38,11 @@ import { ViewerComponent } from '../viewer/viewer.component'; standalone: true, imports: [CommonModule, ThumbsComponent, ViewerComponent], }) -export class GalleryComponent { +export class GalleryComponent implements OnChanges { /** * Gallery items to display */ - @Input() items: GalleryItem[]; + @Input() items: GalleryItem[] = []; /** * Initially selected item, 0-based */ @@ -189,6 +191,12 @@ export class GalleryComponent { : OrientationFlag.VERTICAL; } + ngOnChanges({ items }: SimpleChanges) { + if (!items.currentValue) { + this.items = []; + } + } + focus() { this._viewerElRef.nativeElement.focus(); } 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 fa069131..b761f31a 100644 --- a/libs/gallery/src/lib/components/viewer/viewer.component.spec.ts +++ b/libs/gallery/src/lib/components/viewer/viewer.component.spec.ts @@ -38,6 +38,8 @@ describe('ViewerComponent', () => { fixture = TestBed.createComponent(ViewerComponent); component = fixture.componentInstance; viewerDe = fixture.debugElement; + + component.items = []; }); describe('initialization', () => { diff --git a/libs/gallery/src/lib/components/viewer/viewer.component.ts b/libs/gallery/src/lib/components/viewer/viewer.component.ts index 3a8aada7..b654a727 100644 --- a/libs/gallery/src/lib/components/viewer/viewer.component.ts +++ b/libs/gallery/src/lib/components/viewer/viewer.component.ts @@ -156,7 +156,7 @@ export class ViewerComponent implements OnChanges, OnInit, AfterViewInit { setTimeout(this.updateDimensions); } if (loop || items) { - this.loop = this.items?.length > 1 ? this.loop : false; + this.loop = this.items.length > 1 ? this.loop : false; this.displayedItems = this.getItemsToBeDisplayed(this.fringeCount); if (this.loop) { diff --git a/libs/gallery/src/lib/core/ng.ts b/libs/gallery/src/lib/core/ng.ts new file mode 100644 index 00000000..13572fd4 --- /dev/null +++ b/libs/gallery/src/lib/core/ng.ts @@ -0,0 +1,6 @@ +import { ComponentRef } from '@angular/core'; + +export interface StrictComponentRef extends ComponentRef { + // `& string` because otherwise keyof would return string | number + setInput(name: keyof C & string, value: unknown): void; +}