From a05aafead4be26a34283bf93e825934874aef751 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 11 Jul 2018 17:22:08 +0200 Subject: [PATCH] fix(drag-drop): add fallback if the placeholder transition doesn't complete (#12121) Since a lot of functionality depends on the animation completing, these changes add a fallback for the case where there's a transition on the preview, but it doesn't complete for some reason (e.g. it being too short). Currently clicking rapidly on the drag handle can cause it to get stuck and not complete the drop sequence correctly. --- src/cdk-experimental/drag-drop/drag.spec.ts | 32 ++++++++++++++++++++- src/cdk-experimental/drag-drop/drag.ts | 22 +++++++++++--- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/src/cdk-experimental/drag-drop/drag.spec.ts b/src/cdk-experimental/drag-drop/drag.spec.ts index 7174af7d957..19146dfaff9 100644 --- a/src/cdk-experimental/drag-drop/drag.spec.ts +++ b/src/cdk-experimental/drag-drop/drag.spec.ts @@ -9,7 +9,7 @@ import { Provider, ViewEncapsulation, } from '@angular/core'; -import {TestBed, ComponentFixture, fakeAsync, flush} from '@angular/core/testing'; +import {TestBed, ComponentFixture, fakeAsync, flush, tick} from '@angular/core/testing'; import {DragDropModule} from './drag-drop-module'; import {dispatchMouseEvent, dispatchTouchEvent} from '@angular/cdk/testing'; import {Directionality} from '@angular/cdk/bidi'; @@ -403,6 +403,36 @@ describe('CdkDrag', () => { .toBe('rtl', 'Expected preview element to inherit the directionality.'); })); + it('should remove the preview if its `transitionend` event timed out', fakeAsync(() => { + const fixture = createComponent(DraggableInDropZone); + fixture.detectChanges(); + const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement; + + dispatchMouseEvent(item, 'mousedown'); + fixture.detectChanges(); + + const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement; + + // Add a duration since the tests won't include one. + preview.style.transitionDuration = '500ms'; + + // Move somewhere so the draggable doesn't exit immediately. + dispatchTouchEvent(document, 'mousemove', 50, 50); + fixture.detectChanges(); + + dispatchMouseEvent(document, 'mouseup'); + fixture.detectChanges(); + tick(250); + + expect(preview.parentNode) + .toBeTruthy('Expected preview to be in the DOM mid-way through the transition'); + + tick(500); + + expect(preview.parentNode) + .toBeFalsy('Expected preview to be removed from the DOM if the transition timed out'); + })); + it('should create a placeholder element while the item is dragged', fakeAsync(() => { const fixture = createComponent(DraggableInDropZone); fixture.detectChanges(); diff --git a/src/cdk-experimental/drag-drop/drag.ts b/src/cdk-experimental/drag-drop/drag.ts index 22c54e89509..6093f2ffe38 100644 --- a/src/cdk-experimental/drag-drop/drag.ts +++ b/src/cdk-experimental/drag-drop/drag.ts @@ -436,21 +436,26 @@ export class CdkDrag implements AfterContentInit, OnDestroy { // we need to trigger a style recalculation in order for the `cdk-drag-animating` class to // apply its style, we take advantage of the available info to figure out whether we need to // bind the event in the first place. - const duration = getComputedStyle(this._preview).getPropertyValue('transition-duration'); + const duration = this._getTransitionDurationInMs(this._preview); - if (parseFloat(duration) === 0) { + if (duration === 0) { return Promise.resolve(); } return this._ngZone.runOutsideAngular(() => { return new Promise(resolve => { - const handler = (event: Event) => { - if (event.target === this._preview) { + const handler = (event: TransitionEvent) => { + if (!event || event.target === this._preview) { this._preview.removeEventListener('transitionend', handler); resolve(); + clearTimeout(timeout); } }; + // If a transition is short enough, the browser might not fire the `transitionend` event. + // Since we know how long it's supposed to take, add a timeout with a 50% buffer that'll + // fire if the transition hasn't completed when it was supposed to. + const timeout = setTimeout(handler, duration * 1.5); this._preview.addEventListener('transitionend', handler); }); }); @@ -551,6 +556,15 @@ export class CdkDrag implements AfterContentInit, OnDestroy { this._document.addEventListener(isTouchEvent ? 'touchend' : 'mouseup', this._pointerUp); }); } + + /** Gets the `transition-duration` of an element in milliseconds. */ + private _getTransitionDurationInMs(element: HTMLElement): number { + const rawDuration = getComputedStyle(element).getPropertyValue('transition-duration'); + + // Some browsers will return it in seconds, whereas others will return milliseconds. + const multiplier = rawDuration.toLowerCase().indexOf('ms') > -1 ? 1 : 1000; + return parseFloat(rawDuration) * multiplier; + } } /** Point on the page or within an element. */