Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disabling dynamic changes #610

Merged
merged 4 commits into from
Jul 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions .size-snapshot.json
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
{
"dist/react-beautiful-dnd.js": {
"bundled": 359505,
"minified": 135309,
"gzipped": 37912
"bundled": 351441,
"minified": 131710,
"gzipped": 36933
},
"dist/react-beautiful-dnd.min.js": {
"bundled": 319661,
"minified": 117855,
"gzipped": 32234
"bundled": 312114,
"minified": 114558,
"gzipped": 31398
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a small gain for the release @TrySound

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1kb gzip. I could go lower by removing more unused code for this feature but i won't as we will be adding it right back in 8.1

},
"dist/react-beautiful-dnd.esm.js": {
"bundled": 188421,
"minified": 96739,
"gzipped": 23951,
"bundled": 180861,
"minified": 92833,
"gzipped": 23023,
"treeshaked": {
"rollup": {
"code": 71895,
"import_statements": 726
"code": 68774,
"import_statements": 668
},
"webpack": {
"code": 74369
"code": 71059
}
}
}
Expand Down
7 changes: 3 additions & 4 deletions src/state/create-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import hooks from './middleware/hooks';
import dropAnimationFinish from './middleware/drop-animation-finish';
import dimensionMarshalStopper from './middleware/dimension-marshal-stopper';
import autoScroll from './middleware/auto-scroll';
import pendingDrop from './middleware/pending-drop';
// import pendingDrop from './middleware/pending-drop';
import maxScrollUpdater from './middleware/max-scroll-updater';
import type { DimensionMarshal } from './dimension-marshal/dimension-marshal-types';
import type { StyleMarshal } from '../view/style-marshal/style-marshal-types';
Expand Down Expand Up @@ -71,12 +71,11 @@ export default ({
dimensionMarshalStopper(getDimensionMarshal),
// Fire application hooks in response to drag changes
lift(getDimensionMarshal),
// When a drop is pending and a bulk publish occurs, we need
// pendingDrop,
drop,
// When a drop animation finishes - fire a drop complete
dropAnimationFinish,
pendingDrop,
// TODO: enable for dynamic dimensions
// pendingDrop,
maxScrollUpdater,
autoScroll(getScroller),
// Fire hooks for consumers
Expand Down
16 changes: 11 additions & 5 deletions src/state/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { Position } from 'css-box-model';
import invariant from 'tiny-invariant';
import { scrollDroppable } from './droppable-dimension';
import getDragImpact from './get-drag-impact';
import publish from './publish';
// import publish from './publish';
import moveInDirection, {
type Result as MoveInDirectionResult,
} from './move-in-direction';
Expand Down Expand Up @@ -192,10 +192,16 @@ export default (state: State = idle, action: Action): State => {
`Unexpected ${action.type} received in phase ${state.phase}`,
);

return publish({
state,
publish: action.payload,
});
invariant(
false,
`Dynamic additions and removals of Draggable and Droppable components
is currently not supported. But will be soon!`,
);

// return publish({
// state,
// publish: action.payload,
// });
}

if (action.type === 'MOVE') {
Expand Down
4 changes: 3 additions & 1 deletion test/unit/state/middleware/hooks.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,9 @@ describe('update', () => {
expect(hooks.onDragUpdate).toHaveBeenCalledTimes(1);
});

describe('updates caused by dynamic changes', () => {
// TODO: enable when we use dynamic dimensions
// eslint-disable-next-line jest/no-disabled-tests
describe.skip('updates caused by dynamic changes', () => {
it('should not call onDragUpdate if the destination or source have not changed', () => {
const hooks: Hooks = createHooks();
const store: Store = createStore(middleware(() => hooks, getAnnounce()));
Expand Down
95 changes: 49 additions & 46 deletions test/unit/state/middleware/pending-drop.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,58 +22,61 @@ import {
publishAdditionArgs,
} from '../../../utils/preset-action-args';

it('should trigger a drop on a bulk replace if a drop pending is waiting', () => {
const mock = jest.fn();
const store: Store = createStore(
passThrough(mock),
// will fire the pending drop action
dropMiddleware,
middleware,
);
// eslint-disable-next-line jest/no-disabled-tests
describe.skip('skipping pending drop', () => {
it('should trigger a drop on a dynamic publish if a drop pending is waiting', () => {
const mock = jest.fn();
const store: Store = createStore(
passThrough(mock),
// will fire the pending drop action
dropMiddleware,
middleware,
);

store.dispatch(prepare());
store.dispatch(initialPublish(initialPublishArgs));
store.dispatch(collectionStarting());
store.dispatch(drop({ reason: 'DROP' }));
store.dispatch(prepare());
store.dispatch(initialPublish(initialPublishArgs));
store.dispatch(collectionStarting());
store.dispatch(drop({ reason: 'DROP' }));

const postDrop: State = store.getState();
invariant(
postDrop.phase === 'DROP_PENDING',
`Incorrect phase : ${postDrop.phase}`,
);
expect(postDrop.isWaiting).toBe(true);
const postDrop: State = store.getState();
invariant(
postDrop.phase === 'DROP_PENDING',
`Incorrect phase : ${postDrop.phase}`,
);
expect(postDrop.isWaiting).toBe(true);

// This will finish the drag
mock.mockReset();
store.dispatch(publish(publishAdditionArgs));
// This will finish the drag
mock.mockReset();
store.dispatch(publish(publishAdditionArgs));

expect(mock).toHaveBeenCalledWith(drop({ reason: 'DROP' }));
const expected: DropResult = {
...getDragStart(),
destination: getHomeLocation(critical),
reason: 'DROP',
};
expect(mock).toHaveBeenCalledWith(completeDrop(expected));
expect(mock).toHaveBeenCalledTimes(3);
expect(store.getState().phase).toBe('IDLE');
});
expect(mock).toHaveBeenCalledWith(drop({ reason: 'DROP' }));
const expected: DropResult = {
...getDragStart(),
destination: getHomeLocation(critical),
reason: 'DROP',
};
expect(mock).toHaveBeenCalledWith(completeDrop(expected));
expect(mock).toHaveBeenCalledTimes(3);
expect(store.getState().phase).toBe('IDLE');
});

it('should not trigger a drop on a publish if a drop is not pending', () => {
const mock = jest.fn();
const store: Store = createStore(
passThrough(mock),
// will fire the pending drop action
dropMiddleware,
middleware,
);
it('should not trigger a drop on a publish if a drop is not pending', () => {
const mock = jest.fn();
const store: Store = createStore(
passThrough(mock),
// will fire the pending drop action
dropMiddleware,
middleware,
);

store.dispatch(prepare());
store.dispatch(initialPublish(initialPublishArgs));
store.dispatch(collectionStarting());
store.dispatch(prepare());
store.dispatch(initialPublish(initialPublishArgs));
store.dispatch(collectionStarting());

mock.mockReset();
store.dispatch(publish(publishAdditionArgs));
mock.mockReset();
store.dispatch(publish(publishAdditionArgs));

expect(mock).toHaveBeenCalledWith(publish(publishAdditionArgs));
expect(mock).toHaveBeenCalledTimes(1);
expect(mock).toHaveBeenCalledWith(publish(publishAdditionArgs));
expect(mock).toHaveBeenCalledTimes(1);
});
});
4 changes: 3 additions & 1 deletion test/unit/state/middleware/style.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ it('should use the dragging styles when a dynamic collection is starting', () =>
expect(marshal.collecting).toHaveBeenCalled();
});

it('should use the dragging styles after a dynamic publish', () => {
// TODO: enable when we support dynamic changes
// eslint-disable-next-line jest/no-disabled-tests
it.skip('should use the dragging styles after a dynamic publish', () => {
const marshal: StyleMarshal = getMarshalStub();
const store: Store = createStore(middleware(marshal));

Expand Down