From d951a2d43406bc102201275bbe150469414549ce Mon Sep 17 00:00:00 2001 From: Alex Reardon Date: Fri, 4 May 2018 13:59:36 +1000 Subject: [PATCH] progress --- src/state/dimension-marshal/buffer-marshal.js | 62 ------------------- src/state/dimension-marshal/collector.js | 10 +-- .../dimension-marshal-types.js | 7 +-- .../dimension-marshal/dimension-marshal.js | 9 +-- .../draggable-dimension-publisher.jsx | 14 +++-- .../droppable-dimension-publisher.jsx | 21 +++---- stories/src/dynamic/quote-app.jsx | 3 +- 7 files changed, 31 insertions(+), 95 deletions(-) delete mode 100644 src/state/dimension-marshal/buffer-marshal.js diff --git a/src/state/dimension-marshal/buffer-marshal.js b/src/state/dimension-marshal/buffer-marshal.js deleted file mode 100644 index 1211635430..0000000000 --- a/src/state/dimension-marshal/buffer-marshal.js +++ /dev/null @@ -1,62 +0,0 @@ -// @flow -import invariant from 'tiny-invariant'; - -type Args = {| - collector: Collector, - publisher: Publisher, -|} - -type Phase = 'IDLE' | 'RUNNING'; - -const rafWait = () => new Promise(resolve => requestAnimationFrame(resolve)); - -export default ({ collector, publisher }: Args) => { - let phase: Phase = 'IDLE'; - let isRunQueued: boolean = false; - - const reset = () => { - // forcing phase to IDLE - phase = 'IDLE'; - isRunQueued = false; - }; - - const stopIfIdle = () => (phase === 'IDLE' ? Promise.reject() : Promise.resolve()); - - const run = () => { - phase = 'RUNNING'; - - // This would be easier to read with async/await but the runtime is 10kb - - rafWait() - .then(stopIfIdle) - .then(collector.perform) - .then(rafWait) - .then(stopIfIdle) - .then(publisher.perform) - // collection was stopped - we can just exit - .catch() - .then(() => { - if (isRunQueued) { - run(); - return; - } - reset(); - }); - }; - - const execute = () => { - // A run is already queued - if (isRunQueued) { - return; - } - - // We are already performing a run - if (phase === 'RUNNING') { - return; - } - - run(); - }; - - return { execute, reset }; -}; diff --git a/src/state/dimension-marshal/collector.js b/src/state/dimension-marshal/collector.js index f34289af92..311ebf6a2a 100644 --- a/src/state/dimension-marshal/collector.js +++ b/src/state/dimension-marshal/collector.js @@ -6,8 +6,6 @@ import type { DroppableId, DraggableDimension, DroppableDimension, - ScrollOptions, - Collection, } from '../../types'; import type { ToBeCollected } from './dimension-marshal-types'; @@ -49,20 +47,21 @@ export default ({ const collectFromDOM = (toBeCollected: ToBeCollected, options?: CollectionOptions): Collected => { invariant(isActive, 'Should not collect when not active'); + console.log('excluding', options); const droppables: DroppableDimension[] = toBeCollected.droppables - .filter((id: DroppableId): boolean => Boolean(options && options.exclude.droppableId === id)) + .filter((id: DroppableId): boolean => Boolean(options && options.exclude.droppableId !== id)) .map((id: DroppableId): DroppableDimension => getDroppable(id)); const draggables: DraggableDimension[] = toBeCollected.draggables - .filter((id: DraggableId): boolean => Boolean(options && options.exclude.draggableId === id)) + .filter((id: DraggableId): boolean => Boolean(options && options.exclude.draggableId !== id)) .map((id: DraggableId): DraggableDimension => getDraggable(id)); return { draggables, droppables }; }; const run = (options?: CollectionOptions) => { - invariant(isRunning, 'Cannot start a new run when a run is already occurring'); + invariant(!isRunning, 'Cannot start a new run when a run is already occurring'); isRunning = true; @@ -112,6 +111,7 @@ export default ({ // Queue another run if (isRunning) { isQueued = true; + return; } run(); diff --git a/src/state/dimension-marshal/dimension-marshal-types.js b/src/state/dimension-marshal/dimension-marshal-types.js index 8c15ec28b9..19466955ef 100644 --- a/src/state/dimension-marshal/dimension-marshal-types.js +++ b/src/state/dimension-marshal/dimension-marshal-types.js @@ -18,12 +18,7 @@ export type DroppableCallbacks = {| getDimensionAndWatchScroll: GetDroppableDimensionFn, // scroll a droppable scroll: (change: Position) => void, - // Droppable must listen to scroll events and publish them using the - // onChange callback. If the Droppable is not in a scroll container then - // it does not need to do anything - watchScroll: (options: ScrollOptions) => void, - // If the Droppable is listening for scrol events - it needs to stop! - // This may be called even if watchScroll was not previously called + // If the Droppable is listening for scroll events - it needs to stop! unwatchScroll: () => void, |} diff --git a/src/state/dimension-marshal/dimension-marshal.js b/src/state/dimension-marshal/dimension-marshal.js index b961e92b91..10723c748c 100644 --- a/src/state/dimension-marshal/dimension-marshal.js +++ b/src/state/dimension-marshal/dimension-marshal.js @@ -273,9 +273,8 @@ export default (callbacks: Callbacks) => { entry.callbacks.scroll(change); }; - const collectCriticalDimensions = (request: ?LiftRequest) => { - invariant(collection, 'Cannot start capturing dimensions for a drag it is already dragging'); - invariant(request, 'Cannot start capturing dimensions with an invalid request', request); + const collectCriticalDimensions = (request: LiftRequest) => { + invariant(!collection, 'Cannot start capturing critical dimensions as there is already a collection'); const draggables: DraggableEntryMap = entries.draggables; const droppables: DroppableEntryMap = entries.droppables; @@ -311,10 +310,11 @@ export default (callbacks: Callbacks) => { const stopCollecting = () => { invariant(collection, 'Cannot stop collecting when there is no collection'); - // Tell all droppables to stop watching scroll // all good if they where not already listening Object.keys(entries.droppables) + .filter((id: DroppableId): boolean => + entries.droppables[id].descriptor.type === collection.critical.droppable.type) .forEach((id: DroppableId) => entries.droppables[id].callbacks.unwatchScroll()); collection = null; @@ -325,6 +325,7 @@ export default (callbacks: Callbacks) => { const phase: Phase = current.phase; if (phase === 'COLLECTING_INITIAL_DIMENSIONS') { + invariant(current.dimension.request, 'Cannot start collecting dimensions without a request'); collectCriticalDimensions(current.dimension.request); return; } diff --git a/src/view/draggable-dimension-publisher/draggable-dimension-publisher.jsx b/src/view/draggable-dimension-publisher/draggable-dimension-publisher.jsx index fd5144a416..b81db9a789 100644 --- a/src/view/draggable-dimension-publisher/draggable-dimension-publisher.jsx +++ b/src/view/draggable-dimension-publisher/draggable-dimension-publisher.jsx @@ -51,23 +51,25 @@ export default class DraggableDimensionPublisher extends Component { })); publish = () => { + const marshal: DimensionMarshal = this.context[dimensionMarshalKey]; const descriptor: DraggableDescriptor = this.getMemoizedDescriptor( this.props.draggableId, this.props.droppableId, this.props.index ); - // No changes to the descriptor - if (descriptor === this.publishedDescriptor) { + if (!this.publishedDescriptor) { + marshal.registerDraggable(descriptor, this.getDimension); + this.publishedDescriptor = descriptor; return; } - if (this.publishedDescriptor) { - this.unpublish(); + // No changes to the descriptor + if (descriptor === this.publishedDescriptor) { + return; } - const marshal: DimensionMarshal = this.context[dimensionMarshalKey]; - marshal.registerDraggable(descriptor, this.getDimension); + marshal.updateDraggable(this.publishedDescriptor, descriptor, this.getDimension); this.publishedDescriptor = descriptor; } diff --git a/src/view/droppable-dimension-publisher/droppable-dimension-publisher.jsx b/src/view/droppable-dimension-publisher/droppable-dimension-publisher.jsx index 9816e2f262..a0311b6dfb 100644 --- a/src/view/droppable-dimension-publisher/droppable-dimension-publisher.jsx +++ b/src/view/droppable-dimension-publisher/droppable-dimension-publisher.jsx @@ -43,6 +43,7 @@ export default class DroppableDimensionPublisher extends Component { /* eslint-disable react/sort-comp */ closestScrollable: ?Element = null; isWatchingScroll: boolean = false; + isDragOccurring: boolean = false; scrollOptions: ?ScrollOptions = null; callbacks: DroppableCallbacks; publishedDescriptor: ?DroppableDescriptor = null; @@ -129,10 +130,11 @@ export default class DroppableDimensionPublisher extends Component { }; unwatchScroll = () => { - invariant(this.isWatchingScroll, - `Droppable should not be instructed to stop watching a scroll - when it was not listening to scroll changes` - ); + // It is possible for a Droppable to be asked to unwatch a scroll + // (Eg it has not been collected yet, and the drag ends) + if (!this.isWatchingScroll) { + return; + } this.isWatchingScroll = false; this.scrollOptions = null; @@ -196,6 +198,7 @@ export default class DroppableDimensionPublisher extends Component { if (!this.publishedDescriptor) { marshal.registerDroppable(descriptor, this.callbacks); + this.publishedDescriptor = descriptor; return; } @@ -205,15 +208,12 @@ export default class DroppableDimensionPublisher extends Component { } // already published and there has been changes - marshal.updateDroppable(this.publishedDescriptor.id, descriptor, this.callbacks); + marshal.updateDroppable(this.publishedDescriptor, descriptor, this.callbacks); this.publishedDescriptor = descriptor; } unpublish = () => { - if (!this.publishedDescriptor) { - console.error('Cannot unpublish descriptor when none is published'); - return; - } + invariant(this.publishedDescriptor, 'Cannot unpublish descriptor when none is published'); // Using the previously published id to unpublish. This is to guard // against the case where the id dynamically changes. This is not @@ -234,8 +234,7 @@ export default class DroppableDimensionPublisher extends Component { const targetRef: ?HTMLElement = getDroppableRef(); const descriptor: ?DroppableDescriptor = this.publishedDescriptor; - invariant(targetRef, 'DimensionPublisher cannot calculate a dimension when not attached to the DOM'); - invariant(!this.isWatchingScroll, 'Attempting to recapture Droppable dimension while already watching scroll on previous capture'); + invariant(targetRef, 'Cannot calculate a dimension when not attached to the DOM'); invariant(descriptor, 'Cannot get dimension for unpublished droppable'); const scrollableRef: ?Element = getClosestScrollable(targetRef); diff --git a/stories/src/dynamic/quote-app.jsx b/stories/src/dynamic/quote-app.jsx index 444e789312..bc726664c9 100644 --- a/stories/src/dynamic/quote-app.jsx +++ b/stories/src/dynamic/quote-app.jsx @@ -5,9 +5,9 @@ import QuoteList from '../primatives/quote-list'; import { DragDropContext } from '../../../src/'; import { generateQuoteMap, authors } from '../data'; import { reorderQuoteMap } from '../reorder'; +import { grid } from '../constants'; import type { Quote, QuoteMap, Author } from '../types'; import type { DropResult } from '../../../src/types'; -import { grid } from '../constants'; const intial: QuoteMap = generateQuoteMap(20); @@ -167,6 +167,7 @@ export default class QuoteApp extends React.Component<*, State> { {Object.keys(quoteMap).map((key: string) => (