Improved drag flow #172
Improved drag flow #172
Conversation
| if (state.phase !== 'IDLE' && state.phase !== 'DRAG_COMPLETE') { | ||
| dispatch(clean()); | ||
| // drag cancelled before timeout finished | ||
| if (state.phase !== 'PREPARING') { |
There was a problem hiding this comment.
Previously we had no way of knowing if the drag was ended before this first timeout was fired. It is possible to do with a well orchestrated force press in touch dragging. By adding an extra phase we can know if something has changed in the elapsed time.
|
To be a good citizen I also wrote some tests for the lift action-creator. Hopefully this will assist in preventing regressions in this super important part of the code |
| @@ -55,14 +55,6 @@ export const requestDimensions = (type: TypeId): RequestDimensionsAction => ({ | |||
| payload: type, | |||
| }); | |||
|
|
|||
| export type BeginLiftAction = {| | |||
There was a problem hiding this comment.
no longer needed
| @@ -0,0 +1,249 @@ | |||
| // @flow | |||
There was a problem hiding this comment.
All new tests! 💃
| console.error('cannot drop if not dragging', state); | ||
| // We need to wrap this in a setTimeout to avoid a race | ||
| // condition with `lift`, which includes timeouts | ||
| setTimeout(() => dispatch(clean())); |
There was a problem hiding this comment.
yuck. this is gone! 🎉
| // When a drag starts we need to flush any existing animations | ||
| // that might be occurring. While this flush is occurring we | ||
| // are in this phase | ||
| 'PREPARING' | |
There was a problem hiding this comment.
new phase to allow for flushing
| expect(console.error).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should not begin a lift if the drag is cancelled while the animations are flushing', () => { |
There was a problem hiding this comment.
This test executes a current error case - which is what caused me to make this change
Less
setTimeoutcraziness - and a much more solid flow as a result!