From 03a00351662fdad98503a5daaf418569c788e00e Mon Sep 17 00:00:00 2001 From: Kirill Chirkov Date: Mon, 24 Oct 2022 14:33:37 +0300 Subject: [PATCH] fix: prefer native Web Animations API instead of @angular/animations package --- angular.json | 17 +++++- projects/ng-carousel/package.json | 1 - .../src/lib/carousel.component.spec.ts | 2 - .../ng-carousel/src/lib/carousel.component.ts | 3 +- .../ng-carousel/src/lib/carousel.module.ts | 6 +- .../lib/private/models/carousel-animation.ts | 6 +- .../procedure-environment.interface.ts | 3 - .../lib/private/service/carousel.service.ts | 3 - .../destroy-animation.spec.ts | 60 ++++++++++++------- .../destroy-animation/destroy-animation.ts | 8 +-- .../start-animation-procedure.ts | 1 - .../start-animation/start-animation.spec.ts | 56 +---------------- .../start-animation/start-animation.ts | 38 +++++------- src/app/app.component.html | 4 +- src/app/app.component.spec.ts | 2 - 15 files changed, 78 insertions(+), 132 deletions(-) diff --git a/angular.json b/angular.json index 361e1c9..34df73f 100644 --- a/angular.json +++ b/angular.json @@ -60,8 +60,17 @@ "maximumWarning": "6kb" } ] + }, + "development": { + "buildOptimizer": false, + "optimization": false, + "vendorChunk": true, + "extractLicenses": false, + "sourceMap": true, + "namedChunks": true } - } + }, + "defaultConfiguration": "development" }, "serve": { "builder": "@angular-devkit/build-angular:dev-server", @@ -71,8 +80,12 @@ "configurations": { "production": { "browserTarget": "ng-carousel-demo:build:production" + }, + "development": { + "browserTarget": "ng-carousel-demo:build:development" } - } + }, + "defaultConfiguration": "development" }, "extract-i18n": { "builder": "@angular-devkit/build-angular:extract-i18n", diff --git a/projects/ng-carousel/package.json b/projects/ng-carousel/package.json index c64083b..e8c6600 100644 --- a/projects/ng-carousel/package.json +++ b/projects/ng-carousel/package.json @@ -8,7 +8,6 @@ "peerDependencies": { "@angular/common": "^7.0.0 || ^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0 || ^12.0.0 || ^13.0.0 || ^14.0.0 || ^15.0.0", "@angular/core": "^7.0.0 || ^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0 || ^12.0.0 || ^13.0.0 || ^14.0.0 || ^15.0.0", - "@angular/animations": "^7.0.0 || ^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0 || ^12.0.0 || ^13.0.0 || ^14.0.0 || ^15.0.0", "@angular/cdk": "^7.0.0 || ^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0 || ^12.0.0 || ^13.0.0 || ^14.0.0 || ^15.0.0", "rxjs": "^6.5.0 || ^7.0.0" }, diff --git a/projects/ng-carousel/src/lib/carousel.component.spec.ts b/projects/ng-carousel/src/lib/carousel.component.spec.ts index 1786279..2ee5905 100644 --- a/projects/ng-carousel/src/lib/carousel.component.spec.ts +++ b/projects/ng-carousel/src/lib/carousel.component.spec.ts @@ -1,7 +1,6 @@ import { A11yModule } from '@angular/cdk/a11y'; import { CommonModule } from '@angular/common'; import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; -import { BrowserAnimationsModule } from '@angular/platform-browser/animations'; import { CarouselComponent } from '../public-api'; import { CarouselSlideDirective } from './carousel-slide.directive'; @@ -16,7 +15,6 @@ describe('VirtualCarouselComponent smoke test suite', () => { TestBed.configureTestingModule({ imports: [ CommonModule, - BrowserAnimationsModule, A11yModule, ], declarations: [ diff --git a/projects/ng-carousel/src/lib/carousel.component.ts b/projects/ng-carousel/src/lib/carousel.component.ts index 9a013c4..8401815 100644 --- a/projects/ng-carousel/src/lib/carousel.component.ts +++ b/projects/ng-carousel/src/lib/carousel.component.ts @@ -1,5 +1,5 @@ import { ChangeDetectionStrategy, Component, ContentChild, Input, Output, ViewEncapsulation } from '@angular/core'; -import { map } from 'rxjs/operators'; +import { distinctUntilChanged, map } from 'rxjs/operators'; import { CarouselConfig } from './carousel-config.type'; import { CarouselSlideDirective } from './carousel-slide.directive'; @@ -54,6 +54,7 @@ export class CarouselComponent { @Output() itemIndexChange = this.carousel.carouselStateChanges() .pipe( map((state: CarouselState) => state.activeItemIndex), + distinctUntilChanged(), ); get slideIndex(): number { diff --git a/projects/ng-carousel/src/lib/carousel.module.ts b/projects/ng-carousel/src/lib/carousel.module.ts index 8200b76..05d6b50 100644 --- a/projects/ng-carousel/src/lib/carousel.module.ts +++ b/projects/ng-carousel/src/lib/carousel.module.ts @@ -1,5 +1,5 @@ import { A11yModule } from '@angular/cdk/a11y'; -import { CommonModule } from '@angular/common'; +import { AsyncPipe, NgForOf, NgTemplateOutlet } from '@angular/common'; import { NgModule } from '@angular/core'; import { CarouselSlideDirective } from './carousel-slide.directive'; @@ -10,7 +10,9 @@ import { CarouselEngineComponent } from './private/views/carousel-engine.compone @NgModule({ imports: [ - CommonModule, + NgForOf, + AsyncPipe, + NgTemplateOutlet, A11yModule, CarouselPreventGhostClickModule, ], diff --git a/projects/ng-carousel/src/lib/private/models/carousel-animation.ts b/projects/ng-carousel/src/lib/private/models/carousel-animation.ts index 78a898e..d5afa1f 100644 --- a/projects/ng-carousel/src/lib/private/models/carousel-animation.ts +++ b/projects/ng-carousel/src/lib/private/models/carousel-animation.ts @@ -1,6 +1,3 @@ -import { AnimationPlayer } from '@angular/animations'; -import { Subscription } from 'rxjs'; - /** * Animation state that is currently in process */ @@ -9,8 +6,7 @@ export class CarouselAnimation { constructor( public from: number, public to: number, - public player?: AnimationPlayer | null, - public onDoneSubscription$?: Subscription, + public player?: Animation | null, public startTime = new Date().getTime(), ) { } diff --git a/projects/ng-carousel/src/lib/private/models/procedure/procedure-environment.interface.ts b/projects/ng-carousel/src/lib/private/models/procedure/procedure-environment.interface.ts index e7a3bbc..ab94440 100644 --- a/projects/ng-carousel/src/lib/private/models/procedure/procedure-environment.interface.ts +++ b/projects/ng-carousel/src/lib/private/models/procedure/procedure-environment.interface.ts @@ -1,5 +1,3 @@ -import { AnimationBuilder } from '@angular/animations'; - import { IdGenerator } from '../id-generator'; /** @@ -11,7 +9,6 @@ export interface ProcedureEnvironment { autoplayAction: () => void; afterAnimationAction: () => void; isBrowser: boolean; - animationBuilder: AnimationBuilder | null; animationBezierArgs: number[]; swipeThreshold: number; maxOverscroll: number; diff --git a/projects/ng-carousel/src/lib/private/service/carousel.service.ts b/projects/ng-carousel/src/lib/private/service/carousel.service.ts index c1a1bbe..91f277e 100644 --- a/projects/ng-carousel/src/lib/private/service/carousel.service.ts +++ b/projects/ng-carousel/src/lib/private/service/carousel.service.ts @@ -1,4 +1,3 @@ -import { AnimationBuilder } from '@angular/animations'; import { isPlatformBrowser } from '@angular/common'; import { Inject, Injectable, OnDestroy, PLATFORM_ID, TemplateRef } from '@angular/core'; import { BehaviorSubject, Observable } from 'rxjs'; @@ -48,14 +47,12 @@ export class CarouselService implements OnDestroy { isBrowser: isPlatformBrowser(this.platformId), autoplayAction: this.next.bind(this, true), afterAnimationAction: this.cleanup.bind(this), - animationBuilder: this.animationBuilder, animationBezierArgs: ANIMATION_BEZIER_ARGS, swipeThreshold: MAX_SWIPE_THRESHOLD, maxOverscroll: MAX_OVERSCROLL, }; constructor( - private animationBuilder: AnimationBuilder, @Inject(SLIDE_ID_GENERATOR) private slideIdGenerator: IdGenerator, // tslint:disable-next-line: ban-types @Inject(PLATFORM_ID) private platformId: Object, diff --git a/projects/ng-carousel/src/lib/private/service/helpers/destroy-animation/destroy-animation.spec.ts b/projects/ng-carousel/src/lib/private/service/helpers/destroy-animation/destroy-animation.spec.ts index 13ecf2b..54cb9d5 100644 --- a/projects/ng-carousel/src/lib/private/service/helpers/destroy-animation/destroy-animation.spec.ts +++ b/projects/ng-carousel/src/lib/private/service/helpers/destroy-animation/destroy-animation.spec.ts @@ -1,36 +1,52 @@ -import { NoopAnimationPlayer } from '@angular/animations'; -import { NEVER, Subscription } from 'rxjs'; - import { CarouselAnimation } from '../../../models/carousel-animation'; import { destroyAnimation } from './destroy-animation'; +class MockAnimation implements Animation { + currentTime: number | null = 0; + effect: AnimationEffect | null = null; + cancel(): void {} + commitStyles(): void {} + finish(): void {} + addEventListener(type: K, listener: (this: Animation, ev: AnimationEventMap[K]) => any, options?: boolean | AddEventListenerOptions | undefined): void; + addEventListener(type: string, listener: EventListenerOrEventListenerObject, options?: boolean | AddEventListenerOptions | undefined): void; + addEventListener(type: unknown, listener: unknown, options?: unknown): void {} + dispatchEvent(event: Event): boolean { return true; } + finished: Promise = Promise.resolve(this); + id: string = '1'; + oncancel: ((this: Animation, ev: AnimationPlaybackEvent) => any) | null = null; + onfinish: ((this: Animation, ev: AnimationPlaybackEvent) => any) | null = null; + onremove: ((this: Animation, ev: Event) => any) | null = null; + pending: boolean = false; + playState: AnimationPlayState = 'running'; + playbackRate: number = 1; + ready: Promise = Promise.resolve(this); + replaceState: AnimationReplaceState = 'active'; + startTime: number | null = null; + timeline: AnimationTimeline | null = null; + pause(): void {} + persist(): void {} + play(): void {} + reverse(): void {} + updatePlaybackRate(playbackRate: number): void {} + removeEventListener(type: K, listener: (this: Animation, ev: AnimationEventMap[K]) => any, options?: boolean | EventListenerOptions | undefined): void; + removeEventListener(type: string, listener: EventListenerOrEventListenerObject, options?: boolean | EventListenerOptions | undefined): void; + removeEventListener(type: unknown, listener: unknown, options?: unknown): void {} +} + describe('destroyAnimation test suite', () => { it('should destroy player', () => { - const animationPlayer = new NoopAnimationPlayer(); - spyOn(animationPlayer, 'finish'); - spyOn(animationPlayer, 'destroy'); - const animation = new CarouselAnimation( - 0, - 0, - animationPlayer, - new Subscription(), - ); - destroyAnimation(animation); - expect(animationPlayer.destroy).toHaveBeenCalledTimes(1); - expect(animationPlayer.finish).toHaveBeenCalledTimes(1); - }); - - it('should unsubscribe', () => { - const subscription$ = NEVER.subscribe(); + const mockAnimation = new MockAnimation(); + spyOn(mockAnimation, 'finish'); + spyOn(mockAnimation, 'cancel'); const animation = new CarouselAnimation( 0, 0, - null, - subscription$, + mockAnimation, ); destroyAnimation(animation); - expect(subscription$.closed).toBeTruthy('subscription not closed'); + expect(mockAnimation.cancel).toHaveBeenCalledTimes(1); + expect(mockAnimation.finish).toHaveBeenCalledTimes(0); }); it('should not crash upon null animation', () => { diff --git a/projects/ng-carousel/src/lib/private/service/helpers/destroy-animation/destroy-animation.ts b/projects/ng-carousel/src/lib/private/service/helpers/destroy-animation/destroy-animation.ts index b4da15c..17bde12 100644 --- a/projects/ng-carousel/src/lib/private/service/helpers/destroy-animation/destroy-animation.ts +++ b/projects/ng-carousel/src/lib/private/service/helpers/destroy-animation/destroy-animation.ts @@ -6,11 +6,5 @@ import { CarouselAnimation } from '../../../models/carousel-animation'; export function destroyAnimation( animation?: CarouselAnimation | null, ): void { - try { - animation?.player?.finish(); - animation?.player?.destroy(); - // Ignore exception since player might be already destroyed - // at this moment - } catch (e) {} - animation?.onDoneSubscription$?.unsubscribe(); + animation?.player?.cancel(); } diff --git a/projects/ng-carousel/src/lib/private/service/helpers/start-animation/start-animation-procedure.ts b/projects/ng-carousel/src/lib/private/service/helpers/start-animation/start-animation-procedure.ts index ad7042a..e76f63a 100644 --- a/projects/ng-carousel/src/lib/private/service/helpers/start-animation/start-animation-procedure.ts +++ b/projects/ng-carousel/src/lib/private/service/helpers/start-animation/start-animation-procedure.ts @@ -19,7 +19,6 @@ export function startAnimationProcedure(): Procedure { environment?.animationBezierArgs ?? [], environment?.isBrowser ?? false, environment?.afterAnimationAction ?? (() => {}), - environment?.animationBuilder ?? null, ); const modifiedState: CarouselState = { ...state, diff --git a/projects/ng-carousel/src/lib/private/service/helpers/start-animation/start-animation.spec.ts b/projects/ng-carousel/src/lib/private/service/helpers/start-animation/start-animation.spec.ts index 4a58bf8..ae912dd 100644 --- a/projects/ng-carousel/src/lib/private/service/helpers/start-animation/start-animation.spec.ts +++ b/projects/ng-carousel/src/lib/private/service/helpers/start-animation/start-animation.spec.ts @@ -1,6 +1,4 @@ -import { AnimationBuilder, AnimationFactory, AnimationPlayer, NoopAnimationPlayer } from '@angular/animations'; import { fakeAsync, tick } from '@angular/core/testing'; -import { Subscription } from 'rxjs'; import { CarouselWidthMode } from '../../../../carousel-width-mode'; import { CarouselAnimation } from '../../../models/carousel-animation'; @@ -8,24 +6,6 @@ import { startAnimation } from './start-animation'; describe('startAnimation test suite', () => { - class MockAnimationFactory extends AnimationFactory { - create(): AnimationPlayer { - return new NoopAnimationPlayer(); - } - } - - class MockAnimationBuilder extends AnimationBuilder { - build(): AnimationFactory { - return new MockAnimationFactory(); - } - } - - let animationBuilder: AnimationBuilder; - - beforeEach(() => { - animationBuilder = new MockAnimationBuilder(); - }); - it('should not run in browserless environment', () => { const container = null; const from = 0; @@ -44,13 +24,12 @@ describe('startAnimation test suite', () => { bezierArgs, isBrowser, afterAnimationAction, - animationBuilder, ); expect(result).toBeNull('result is created'); }); it('should create animation instance', () => { - const container = {} as HTMLElement; + const container = document ? document.createElement('div') : {} as HTMLElement; const from = -20; const to = 50; const widthMode = CarouselWidthMode.PX; @@ -67,43 +46,10 @@ describe('startAnimation test suite', () => { bezierArgs, isBrowser, afterAnimationAction, - animationBuilder, ); expect(result instanceof CarouselAnimation).toBeTruthy('result is not created'); - expect(result?.onDoneSubscription$ instanceof Subscription).toBeTruthy('subscription is not Subscription'); expect(result?.player).toBeTruthy('player is not created'); expect(result?.from).toBe(-20, 'incorrect from value'); expect(result?.to).toBe(50, 'incorrect to value'); - - // Cleanup - result?.onDoneSubscription$?.unsubscribe(); }); - - it('should emit onDone', fakeAsync(() => { - const container = {} as HTMLElement; - const from = 0; - const to = 0; - const widthMode = CarouselWidthMode.PX; - const duration = 0; - const bezierArgs = [0, 0, 0, 0]; - const isBrowser = true; - const afterAnimationActionObject = {action: () => {}}; - spyOn(afterAnimationActionObject, 'action'); - const result = startAnimation( - container, - from, - to, - widthMode, - duration, - bezierArgs, - isBrowser, - afterAnimationActionObject.action, - animationBuilder, - ); - tick(); - expect(afterAnimationActionObject.action).toHaveBeenCalledTimes(1); - - // Cleanup - result?.onDoneSubscription$?.unsubscribe(); - })); }); diff --git a/projects/ng-carousel/src/lib/private/service/helpers/start-animation/start-animation.ts b/projects/ng-carousel/src/lib/private/service/helpers/start-animation/start-animation.ts index f90b920..bb29ab2 100644 --- a/projects/ng-carousel/src/lib/private/service/helpers/start-animation/start-animation.ts +++ b/projects/ng-carousel/src/lib/private/service/helpers/start-animation/start-animation.ts @@ -1,6 +1,3 @@ -import { animate, AnimationBuilder, style } from '@angular/animations'; -import { bindCallback } from 'rxjs'; - import { CarouselWidthMode } from '../../../../carousel-width-mode'; import { CarouselAnimation } from '../../../models/carousel-animation'; @@ -13,38 +10,31 @@ export function startAnimation( bezierArgs: number[], isBrowser: boolean, afterAnimationAction: () => void, - animationBuilder: AnimationBuilder | null, ): CarouselAnimation | null { - if (!isBrowser || !container || from === null || !animationBuilder) { + if (!isBrowser || !container || from === null) { return null; } const cubicBezier = `cubic-bezier(${bezierArgs[0]},${bezierArgs[1]},${bezierArgs[2]},${bezierArgs[3]})`; - const animationFactory = animationBuilder.build([ - style({ - transform: `translateX(${from}${widthMode})`, - }), - animate(`${transitionDuration}ms ${cubicBezier}`, style({ - transform: `translateX(${to}${widthMode})`, - })), - ]); - const animationPlayer = animationFactory.create(container); - // Wrap onDone into observable - const boundFunction = bindCallback(animationPlayer.onDone); // Wrap function into function that returns observable - const onDone$ = boundFunction.call(animationPlayer); // Receive observable with context of animation player - const subscription$ = onDone$ - .subscribe(() => { - animationPlayer.destroy(); + const animationNative = container?.animate?.([ + { transform: `translateX(${from}${widthMode})` }, + { transform: `translateX(${to}${widthMode})` }, + ], { + duration: transitionDuration, + easing: cubicBezier, + }); + if (animationNative) { + animationNative.onfinish = () => { afterAnimationAction(); - }); + } + } const animation = new CarouselAnimation( from, to, - animationPlayer, - subscription$, + animationNative, ); - animationPlayer.play(); + animationNative?.play?.(); return animation; } diff --git a/src/app/app.component.html b/src/app/app.component.html index 5c759d3..a420f45 100644 --- a/src/app/app.component.html +++ b/src/app/app.component.html @@ -41,11 +41,11 @@

- 🠐 + ← Active slide this side - 🠒 + → diff --git a/src/app/app.component.spec.ts b/src/app/app.component.spec.ts index 2b05754..9b036df 100644 --- a/src/app/app.component.spec.ts +++ b/src/app/app.component.spec.ts @@ -6,7 +6,6 @@ import { MatCardModule } from '@angular/material/card'; import { MatCheckboxModule } from '@angular/material/checkbox'; import { MatSliderModule } from '@angular/material/slider'; import { MatToolbarModule } from '@angular/material/toolbar'; -import { NoopAnimationsModule } from '@angular/platform-browser/animations'; import { RouterTestingModule } from '@angular/router/testing'; import { CarouselModule } from '../../projects/ng-carousel/src/public-api'; @@ -16,7 +15,6 @@ describe('AppComponent', () => { beforeEach(waitForAsync(() => { TestBed.configureTestingModule({ imports: [ - NoopAnimationsModule, RouterTestingModule, MatToolbarModule, MatButtonModule,