Skip to content

Commit

Permalink
fix: make items always defined
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
daelmaak committed Dec 2, 2023
1 parent ff7c86b commit 319ad43
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -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<GalleryComponent>;
let fixture: ComponentFixture<GalleryComponent>;
let de: DebugElement;

Expand All @@ -23,6 +25,7 @@ describe('GalleryComponent', () => {
beforeEach(() => {
fixture = TestBed.createComponent(GalleryComponent);
component = fixture.componentInstance;
componentRef = fixture.componentRef;
de = fixture.debugElement;
});

Expand All @@ -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' }];
Expand Down
12 changes: 10 additions & 2 deletions libs/gallery/src/lib/components/gallery/gallery.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import {
HostBinding,
HostListener,
Input,
OnChanges,
Output,
SimpleChanges,
TemplateRef,
ViewChild,
} from '@angular/core';
Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -189,6 +191,12 @@ export class GalleryComponent {
: OrientationFlag.VERTICAL;
}

ngOnChanges({ items }: SimpleChanges) {
if (!items.currentValue) {
this.items = [];
}
}

focus() {
this._viewerElRef.nativeElement.focus();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ describe('ViewerComponent', () => {
fixture = TestBed.createComponent(ViewerComponent);
component = fixture.componentInstance;
viewerDe = fixture.debugElement;

component.items = [];
});

describe('initialization', () => {
Expand Down
2 changes: 1 addition & 1 deletion libs/gallery/src/lib/components/viewer/viewer.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 6 additions & 0 deletions libs/gallery/src/lib/core/ng.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { ComponentRef } from '@angular/core';

export interface StrictComponentRef<C> extends ComponentRef<C> {
// `& string` because otherwise keyof would return string | number
setInput(name: keyof C & string, value: unknown): void;
}

0 comments on commit 319ad43

Please sign in to comment.