diff --git a/src/debug/timings.js b/src/debug/timings.js index 3cf4867ea2..b0b085fd3c 100644 --- a/src/debug/timings.js +++ b/src/debug/timings.js @@ -10,8 +10,8 @@ const flag: string = '__react-beautiful-dnd-debug-timings-hook__'; const isTimingsEnabled = (): boolean => Boolean(window[flag]); -// TEMP -window[flag] = true; +// Debug: uncomment to enable +// window[flag] = true; export const start = (key: string) => { if (!isTimingsEnabled()) { diff --git a/src/state/create-store.js b/src/state/create-store.js index dd0fb02159..17725eec2a 100644 --- a/src/state/create-store.js +++ b/src/state/create-store.js @@ -61,9 +61,6 @@ export default ({ // We need to stop the marshal before hooks fire as hooks can cause // dimension registration changes in response to reordering dimensionMarshalStopper(getDimensionMarshal), - // Fire application hooks - // TODO: where should this be? - hooks(getHooks, announce), // Fire application hooks in response to drag changes lift(getDimensionMarshal), // When a drop is pending and a bulk publish occurs, we need @@ -73,6 +70,8 @@ export default ({ dropAnimationFinish, pendingDrop, autoScroll, + // Fire hooks for consumers + hooks(getHooks, announce), ), ), ); diff --git a/src/state/dimension-marshal/dimension-marshal.js b/src/state/dimension-marshal/dimension-marshal.js index 91e60a7127..4dc7875e45 100644 --- a/src/state/dimension-marshal/dimension-marshal.js +++ b/src/state/dimension-marshal/dimension-marshal.js @@ -24,6 +24,30 @@ import type { Collection, } from './dimension-marshal-types'; +const advancedUsageWarning = () => (() => { + let hasAnnounced: boolean = false; + + return () => { + if (hasAnnounced) { + return; + } + + hasAnnounced = true; + + if (process.env.NODE_ENV !== 'production') { + console.warn(` + Warning: you are triggering a recollection of dimensions during a drag. + This is fairly advanced feature used to support interactions such as lazy loading lists. + You might not have intended to trigger this collection. A collection will be triggered + whenever a Droppable or Draggable is added or removed; or when: + + - Draggable: 'id' or 'index' change + - Droppable: 'id' change ('type' change is not permitted during a drag) + `.trim()); + } + }; +})(); + export default (callbacks: Callbacks) => { const entries: Entries = { droppables: {}, @@ -38,6 +62,11 @@ export default (callbacks: Callbacks) => { const collect = ({ includeCritical }: {| includeCritical: boolean |}) => { invariant(collection, 'Cannot collect without a collection occurring'); + + if (includeCritical) { + advancedUsageWarning(); + } + collector.collect({ collection, includeCritical, @@ -81,7 +110,7 @@ export default (callbacks: Callbacks) => { invariant(descriptor.id !== collection.critical.draggable.id, 'Cannot unregister dragging item during a drag'); - collect({ includeCritical: false }); + collect({ includeCritical: true }); }; const updateDraggable = ( @@ -104,7 +133,7 @@ export default (callbacks: Callbacks) => { const home: ?DroppableEntry = entries.droppables[descriptor.droppableId]; invariant(home, 'Cannot update a Draggable that does not have a home'); - collect({ includeCritical: false }); + collect({ includeCritical: true }); }; const unregisterDraggable = (descriptor: DraggableDescriptor) => { @@ -126,7 +155,7 @@ export default (callbacks: Callbacks) => { invariant(descriptor.id !== collection.critical.draggable.id, 'Cannot unregister dragging item during a drag'); - collect({ includeCritical: false }); + collect({ includeCritical: true }); }; const registerDroppable = ( @@ -157,7 +186,7 @@ export default (callbacks: Callbacks) => { invariant(descriptor.id !== collection.critical.droppable.id, 'Cannot register home droppable during a drag'); - collect({ includeCritical: false }); + collect({ includeCritical: true }); }; const updateDroppable = ( @@ -176,7 +205,7 @@ export default (callbacks: Callbacks) => { registerDroppable(descriptor, droppableCallbacks); - collect({ includeCritical: false }); + collect({ includeCritical: true }); }; const unregisterDroppable = (descriptor: DroppableDescriptor) => { @@ -202,7 +231,7 @@ export default (callbacks: Callbacks) => { invariant(descriptor.id !== collection.critical.droppable.id, 'Cannot unregister home droppable during a drag'); - collect({ includeCritical: false }); + collect({ includeCritical: true }); }; const updateDroppableIsEnabled = (id: DroppableId, isEnabled: boolean) => { @@ -268,15 +297,18 @@ export default (callbacks: Callbacks) => { if (!collection) { return; } - const home: DroppableDescriptor = collection.critical.droppable; + // Stop any pending dom collections or publish + collector.stop(); + // Tell all droppables to stop watching scroll // all good if they where not already listening + const home: DroppableDescriptor = collection.critical.droppable; Object.keys(entries.droppables) .filter((id: DroppableId): boolean => entries.droppables[id].descriptor.type === home.type) .forEach((id: DroppableId) => entries.droppables[id].callbacks.unwatchScroll()); + // Finally - clear our collection collection = null; - collector.stop(); }; const startPublishing = (request: LiftRequest, windowScroll: Position) => { diff --git a/src/state/middleware/hooks.js b/src/state/middleware/hooks.js index 462fb736b3..f4cc12f3f9 100644 --- a/src/state/middleware/hooks.js +++ b/src/state/middleware/hooks.js @@ -116,7 +116,7 @@ export default (getHooks: () => Hooks, announce: Announce) => { }; const move = (location: ?DraggableLocation) => { - invariant(publishedStart, 'Cannot fire onDragMove when onDragStart has not been called'); + invariant(isDragStartPublished(), 'Cannot fire onDragMove when onDragStart has not been called'); // No change to publish if (areLocationsEqual(lastLocation, location)) { @@ -133,7 +133,7 @@ export default (getHooks: () => Hooks, announce: Announce) => { }; const end = (result: DropResult) => { - invariant(isDragStartPublished, 'Cannot fire onDragEnd when there is no matching onDragStart'); + invariant(isDragStartPublished(), 'Cannot fire onDragEnd when there is no matching onDragStart'); publishedStart = null; lastLocation = null; withTimings('onDragEnd', () => execute(getHooks().onDragEnd, result, messagePreset.onDragEnd)); @@ -191,7 +191,7 @@ export default (getHooks: () => Hooks, announce: Announce) => { // we should fire a onDragEnd hook if (action.type === 'CLEAN') { // Unmatched drag start call - need to cancel - if (publisher.isDragStartPublished) { + if (publisher.isDragStartPublished()) { publisher.cancel(); } @@ -202,7 +202,7 @@ export default (getHooks: () => Hooks, announce: Announce) => { // ## Perform drag updates // No drag updates required - if (!publisher.isDragStartPublished) { + if (!publisher.isDragStartPublished()) { next(action); return; } diff --git a/test/unit/integration/hooks-integration.spec.js b/test/unit/integration/hooks-integration.spec.js index df9f3e118e..c668a58373 100644 --- a/test/unit/integration/hooks-integration.spec.js +++ b/test/unit/integration/hooks-integration.spec.js @@ -248,7 +248,7 @@ describe('hooks integration', () => { }); describe('drag end', () => { - it('should call the onDragEnd hook when a drag ends', () => { + it.only('should call the onDragEnd hook when a drag ends', () => { drag.perform(); wasDragCompleted(); diff --git a/test/unit/state/middleware/hooks/announcements.spec.js b/test/unit/state/middleware/hooks/announcements.spec.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/unit/state/middleware/hooks/drag-end.spec.js b/test/unit/state/middleware/hooks/drag-end.spec.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/unit/state/middleware/hooks/drag-start.spec.js b/test/unit/state/middleware/hooks/drag-start.spec.js new file mode 100644 index 0000000000..03c0870c92 --- /dev/null +++ b/test/unit/state/middleware/hooks/drag-start.spec.js @@ -0,0 +1,44 @@ +// @flow + +describe('start', () => { + it('should call the onDragStart hook when a initial publish occurs', () => { + + }); + + it('should throw an exception if an initial publish is called before a drag ends', () => { + + }); +}); + +describe('drop', () => { + it('should call the onDragEnd hook when a DROP_COMPLETE action occurs', () => { + + }); + + it('should throw an exception if there was no drag start published', () => { + + }); +}); + +describe('cancel', () => { + it('should publish an on drag end with no destination', () => { + + }); + + // the start location can change after a dynamic update + it('should use the current critical descriptor as the start location', () => { + + }); + + it('should not do anything if a drag start had not been published', () => { + + }); +}); + +describe('update', () => { + +}); + +describe('announcements', () => { + +}); diff --git a/test/unit/state/middleware/hooks/drag-update.spec.js b/test/unit/state/middleware/hooks/drag-update.spec.js new file mode 100644 index 0000000000..e69de29bb2