Skip to content

Commit

Permalink
fix(carousel): change slides on prev/next/select API calls with OnPush
Browse files Browse the repository at this point in the history
Regression introduced after marking carousel OnPush due to lack of tests

Closes #2776
Fixes #2766
  • Loading branch information
maxokorokov committed Oct 11, 2018
1 parent 48bd36c commit 99049e7
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 6 deletions.
89 changes: 87 additions & 2 deletions src/carousel/carousel.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,99 @@ describe('ngb-carousel', () => {
discardPeriodicTasks();
}));

it('should change slide on prev/next API calls', fakeAsync(() => {
const html = `
<ngb-carousel #c [interval]="0">
<ng-template ngbSlide>foo</ng-template>
<ng-template ngbSlide>bar</ng-template>
<ng-template ngbSlide id="s3">baz</ng-template>
</ngb-carousel>
<button id="next" (click)="c.next()">Next</button>
<button id="prev" (click)="c.prev()">Prev</button>
<button id="select" (click)="c.select('s3')">Select 3</button>
`;

it('should change slide on indicator click', fakeAsync(() => {
const fixture = createTestComponent(html);
const next = fixture.nativeElement.querySelector('#next');
const prev = fixture.nativeElement.querySelector('#prev');
const select = fixture.nativeElement.querySelector('#select');

expectActiveSlides(fixture.nativeElement, [true, false, false]);

next.click();
fixture.detectChanges();
expectActiveSlides(fixture.nativeElement, [false, true, false]);

prev.click();
fixture.detectChanges();
expectActiveSlides(fixture.nativeElement, [true, false, false]);

select.click();
fixture.detectChanges();
expectActiveSlides(fixture.nativeElement, [false, false, true]);
}));

it('should pause/resume slide change on API calls', fakeAsync(() => {
const html = `
<ngb-carousel>
<ngb-carousel #c [interval]="1000">
<ng-template ngbSlide>foo</ng-template>
<ng-template ngbSlide>bar</ng-template>
</ngb-carousel>
<button id="pause" (click)="c.pause()">Next</button>
<button id="cycle" (click)="c.cycle()">Prev</button>
`;

const fixture = createTestComponent(html);
const pause = fixture.nativeElement.querySelector('#pause');
const cycle = fixture.nativeElement.querySelector('#cycle');

expectActiveSlides(fixture.nativeElement, [true, false]);

tick(1000);
fixture.detectChanges();
expectActiveSlides(fixture.nativeElement, [false, true]);

pause.click();
tick(1000);
fixture.detectChanges();
expectActiveSlides(fixture.nativeElement, [false, true]);

cycle.click();
tick(1000);
fixture.detectChanges();
expectActiveSlides(fixture.nativeElement, [true, false]);

discardPeriodicTasks();
}));

it('should mark component for check for API calls', fakeAsync(() => {
const html = `
<ngb-carousel #c [interval]="0">
<ng-template ngbSlide>foo</ng-template>
<ng-template ngbSlide>bar</ng-template>
<ng-template ngbSlide *ngIf="addNewSlide">baz</ng-template>
</ngb-carousel>
<button id="next" (click)="c.next(); addNewSlide = true">Next</button>
`;

const fixture = createTestComponent(html);
const next = fixture.nativeElement.querySelector('#next');

expectActiveSlides(fixture.nativeElement, [true, false]);

next.click();
fixture.detectChanges();
expectActiveSlides(fixture.nativeElement, [false, true, false]);
}));

it('should change slide on indicator click', fakeAsync(() => {
const html = `
<ngb-carousel>
<ng-template ngbSlide>foo</ng-template>
<ng-template ngbSlide>bar</ng-template>
</ngb-carousel>
`;

const fixture = createTestComponent(html);
const indicatorElms = fixture.nativeElement.querySelectorAll('ol.carousel-indicators > li');

Expand Down Expand Up @@ -646,6 +730,7 @@ describe('ngb-carousel', () => {

@Component({selector: 'test-cmp', template: ''})
class TestComponent {
addNewSlide = false;
interval;
activeSlideId;
keyboard = true;
Expand Down
8 changes: 4 additions & 4 deletions src/carousel/carousel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,7 @@ export class NgbCarousel implements AfterContentChecked,
.pipe(
map(() => this.interval), filter(interval => interval > 0 && this.slides.length > 0),
switchMap(interval => timer(interval).pipe(takeUntil(this._stop$))))
.subscribe(() => this._ngZone.run(() => {
this.next();
this._cd.detectChanges();
}));
.subscribe(() => this._ngZone.run(() => this.next()));

this._start$.next();
});
Expand Down Expand Up @@ -202,6 +199,9 @@ export class NgbCarousel implements AfterContentChecked,
this._start$.next();
this.activeId = selectedSlide.id;
}

// we get here after the interval fires or any external API call like next(), prev() or select()
this._cd.markForCheck();
}

private _getSlideEventDirection(currentActiveSlideId: string, nextActiveSlideId: string): NgbSlideEventDirection {
Expand Down

0 comments on commit 99049e7

Please sign in to comment.