Skip to content

Commit

Permalink
fix(module:carousel): fix carousel in entry components (NG-ZORRO#3367)
Browse files Browse the repository at this point in the history
* fix(module:carousel): fix carousel in entry components

* fix: test

* fix: typo
  • Loading branch information
Wendell authored and hsuanxyz committed May 17, 2019
1 parent 60b970a commit ca5a1a5
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 36 deletions.
28 changes: 19 additions & 9 deletions components/carousel/nz-carousel.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import {
import { fromEvent, Subject } from 'rxjs';

import { isTouchEvent, InputBoolean, InputNumber } from 'ng-zorro-antd/core';
import { take, takeUntil, throttleTime } from 'rxjs/operators';
import { takeUntil, throttleTime } from 'rxjs/operators';

import { NzCarouselContentDirective } from './nz-carousel-content.directive';
import { FromToInterface, NzCarouselEffects, PointerVector } from './nz-carousel-definitions';
Expand Down Expand Up @@ -131,7 +131,7 @@ export class NzCarouselComponent implements AfterContentInit, AfterViewInit, OnD

this.carouselContents.changes.pipe(takeUntil(this.destroy$)).subscribe(() => {
this.markContentActive(0);
this.strategy.withCarouselContents(this.carouselContents);
this.syncStrategy();
});

this.ngZone.runOutsideAngular(() => {
Expand All @@ -141,13 +141,18 @@ export class NzCarouselComponent implements AfterContentInit, AfterViewInit, OnD
throttleTime(16)
)
.subscribe(() => {
this.strategy.withCarouselContents(this.carouselContents);
this.syncStrategy();
});
});

this.ngZone.onStable.pipe(take(1)).subscribe(() => {
this.switchStrategy();
this.strategy.withCarouselContents(this.carouselContents);
this.switchStrategy();
this.markContentActive(0);
this.syncStrategy();

// If embedded in an entry component, it may do initial render at a inappropriate time.
// ngZone.onStable won't do this trick
Promise.resolve().then(() => {
this.syncStrategy();
});
}

Expand All @@ -156,6 +161,8 @@ export class NzCarouselComponent implements AfterContentInit, AfterViewInit, OnD

if (nzEffect && !nzEffect.isFirstChange()) {
this.switchStrategy();
this.markContentActive(0);
this.syncStrategy();
}

if (!this.nzAutoPlay || !this.nzAutoPlaySpeed) {
Expand Down Expand Up @@ -220,9 +227,6 @@ export class NzCarouselComponent implements AfterContentInit, AfterViewInit, OnD
this.nzEffect === 'scrollx'
? new NzCarouselTransformStrategy(this, this.cdr, this.renderer)
: new NzCarouselOpacityStrategy(this, this.cdr, this.renderer);

this.markContentActive(0);
this.strategy.withCarouselContents(this.carouselContents);
}

private scheduleNextTransition(): void {
Expand Down Expand Up @@ -296,6 +300,12 @@ export class NzCarouselComponent implements AfterContentInit, AfterViewInit, OnD
}
};

private syncStrategy(): void {
if (this.strategy) {
this.strategy.withCarouselContents(this.carouselContents);
}
}

private dispose(): void {
this.document.removeEventListener('mousemove', this.pointerMove);
this.document.removeEventListener('touchmove', this.pointerMove);
Expand Down
54 changes: 27 additions & 27 deletions components/carousel/nz-carousel.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe('carousel', () => {

it('should dynamic change content work', fakeAsync(() => {
fixture.detectChanges();
tick();
tick(3000);
fixture.detectChanges();
expect(carouselContents.length).toBe(4);
testComponent.array = [];
Expand Down Expand Up @@ -73,7 +73,6 @@ describe('carousel', () => {
});

it('should click content change', () => {
fixture.detectChanges();
expect(carouselContents[0].nativeElement.classList).toContain('slick-active');
carouselWrapper.nativeElement.querySelector('.slick-dots').lastElementChild.click();
fixture.detectChanges();
Expand All @@ -87,16 +86,16 @@ describe('carousel', () => {
expect(carouselContents[0].nativeElement.classList).toContain('slick-active');

dispatchKeyboardEvent(list, 'keydown', LEFT_ARROW);
tickATransition(fixture);
tickMilliseconds(fixture, 700);
expect(carouselContents[3].nativeElement.classList).toContain('slick-active');
dispatchKeyboardEvent(list, 'keydown', LEFT_ARROW);
tickATransition(fixture);
tickMilliseconds(fixture, 700);
expect(carouselContents[2].nativeElement.classList).toContain('slick-active');
dispatchKeyboardEvent(list, 'keydown', RIGHT_ARROW);
tickATransition(fixture);
tickMilliseconds(fixture, 700);
expect(carouselContents[3].nativeElement.classList).toContain('slick-active');
dispatchKeyboardEvent(list, 'keydown', RIGHT_ARROW);
tickATransition(fixture);
tickMilliseconds(fixture, 700);
expect(carouselContents[0].nativeElement.classList).toContain('slick-active');
}));

Expand All @@ -122,22 +121,22 @@ describe('carousel', () => {
'translate3d(0px, 0px, 0px)'
);
carouselWrapper.nativeElement.querySelector('.slick-dots').lastElementChild.click();
tickATransition(fixture);
tickMilliseconds(fixture, 700);
expect(carouselWrapper.nativeElement.querySelector('.slick-track').style.transform).not.toBe('');

testComponent.effect = 'fade';
testComponent.vertical = true;
fixture.detectChanges();
expect(carouselContents[0].nativeElement.classList).toContain('slick-active');
carouselWrapper.nativeElement.querySelector('.slick-dots').lastElementChild.click();
tickATransition(fixture);
tickMilliseconds(fixture, 700);
expect(carouselWrapper.nativeElement.querySelector('.slick-track').style.transform).toBe('');

testComponent.effect = 'scrollx';
fixture.detectChanges();
expect(carouselContents[0].nativeElement.classList).toContain('slick-active');
carouselWrapper.nativeElement.querySelector('.slick-dots').lastElementChild.click();
tickATransition(fixture);
tickMilliseconds(fixture, 700);
expect(carouselWrapper.nativeElement.querySelector('.slick-track').style.transform).not.toBe(
'translate3d(0px, 0px, 0px)'
);
Expand Down Expand Up @@ -180,13 +179,13 @@ describe('carousel', () => {
fixture.detectChanges();
expect(carouselContents[0].nativeElement.classList).toContain('slick-active');
testComponent.nzCarouselComponent.next();
tickATransition(fixture);
tickMilliseconds(fixture, 700);
expect(carouselContents[1].nativeElement.classList).toContain('slick-active');
testComponent.nzCarouselComponent.pre();
tickATransition(fixture);
tickMilliseconds(fixture, 700);
expect(carouselContents[0].nativeElement.classList).toContain('slick-active');
testComponent.nzCarouselComponent.goTo(2);
tickATransition(fixture);
tickMilliseconds(fixture, 700);
expect(carouselContents[2].nativeElement.classList).toContain('slick-active');
}));

Expand All @@ -199,21 +198,21 @@ describe('carousel', () => {

it('should support swiping to switch', fakeAsync(() => {
swipe(testComponent.nzCarouselComponent, 500);
tickATransition(fixture);
tickMilliseconds(fixture, 700);
expect(carouselContents[0].nativeElement.classList).not.toContain('slick-active');
expect(carouselContents[1].nativeElement.classList).toContain('slick-active');

swipe(testComponent.nzCarouselComponent, -500);
tickATransition(fixture);
tickMilliseconds(fixture, 700);
swipe(testComponent.nzCarouselComponent, -500);
tickATransition(fixture);
tickMilliseconds(fixture, 700);
expect(carouselContents[0].nativeElement.classList).not.toContain('slick-active');
expect(carouselContents[3].nativeElement.classList).toContain('slick-active');
}));

it('should prevent swipes that are not long enough', fakeAsync(() => {
swipe(testComponent.nzCarouselComponent, 2);
tickATransition(fixture);
tickMilliseconds(fixture, 700);
expect(carouselContents[0].nativeElement.classList).toContain('slick-active');
}));
});
Expand All @@ -237,22 +236,22 @@ describe('carousel', () => {
swipe(testComponent.nzCarouselComponent, 500);
expect(testComponent.nzCarouselComponent.slickTrackEl.style.transform).not.toBe(`translate3d(0px, 0px, 0px)`);

tickATransition(fixture);
tickMilliseconds(fixture, 700);

swipe(testComponent.nzCarouselComponent, -500);
expect(testComponent.nzCarouselComponent.slickTrackEl.style.transform).toBe(`translate3d(0px, 0px, 0px)`);
tickATransition(fixture);
tickMilliseconds(fixture, 700);

// From first to last.
swipe(testComponent.nzCarouselComponent, -500);
expect(testComponent.nzCarouselComponent.slickTrackEl.style.transform).not.toBe(`translate3d(0px, 0px, 0px)`);
tickATransition(fixture);
tickMilliseconds(fixture, 700);
expect(testComponent.nzCarouselComponent.slickTrackEl.style.transform).not.toBe(`translate3d(0px, 0px, 0px)`);

// From last to first.
swipe(testComponent.nzCarouselComponent, 500);
expect(testComponent.nzCarouselComponent.slickTrackEl.style.transform).not.toBe(`translate3d(0px, 0px, 0px)`);
tickATransition(fixture);
tickMilliseconds(fixture, 700);
expect(testComponent.nzCarouselComponent.slickTrackEl.style.transform).toBe(`translate3d(0px, 0px, 0px)`);
}));

Expand All @@ -264,29 +263,30 @@ describe('carousel', () => {

swipe(testComponent.nzCarouselComponent, 500);
expect(testComponent.nzCarouselComponent.el.style.transform).not.toBe(`translate3d(0px, 0px, 0px)`);
tickATransition(fixture);
tickMilliseconds(fixture, 700);

swipe(testComponent.nzCarouselComponent, -500);
expect(testComponent.nzCarouselComponent.slickTrackEl.style.transform).toBe(`translate3d(0px, 0px, 0px)`);
tickATransition(fixture);
tickMilliseconds(fixture, 700);

// From first to last.
swipe(testComponent.nzCarouselComponent, -500);
expect(testComponent.nzCarouselComponent.slickTrackEl.style.transform).not.toBe(`translate3d(0px, 0px, 0px)`);
tickATransition(fixture);
tickMilliseconds(fixture, 700);
expect(testComponent.nzCarouselComponent.slickTrackEl.style.transform).not.toBe(`translate3d(0px, 0px, 0px)`);

// From last to first.
swipe(testComponent.nzCarouselComponent, 500);
expect(testComponent.nzCarouselComponent.slickTrackEl.style.transform).not.toBe(`translate3d(0px, 0px, 0px)`);
tickATransition(fixture);
tickMilliseconds(fixture, 700);
expect(testComponent.nzCarouselComponent.slickTrackEl.style.transform).toBe(`translate3d(0px, 0px, 0px)`);
}));

it('should disable dragging during transitioning', fakeAsync(() => {
tickMilliseconds(fixture, 700);
testComponent.nzCarouselComponent.goTo(1);
swipe(testComponent.nzCarouselComponent, 500);
tickATransition(fixture);
tickMilliseconds(fixture, 700);
expect(carouselContents[1].nativeElement.classList).toContain('slick-active');
}));
});
Expand Down Expand Up @@ -330,9 +330,9 @@ export class NzTestCarouselBasicComponent {
beforeChange = jasmine.createSpy('beforeChange callback');
}

function tickATransition<T>(fixture: ComponentFixture<T>): void {
function tickMilliseconds<T>(fixture: ComponentFixture<T>, seconds: number = 1): void {
fixture.detectChanges();
tick(700);
tick(seconds);
fixture.detectChanges();
}

Expand Down

0 comments on commit ca5a1a5

Please sign in to comment.