get drag impact refactor #92
Conversation
src/state/get-drag-impact.js
Outdated
| } | ||
|
|
||
| const insideDroppable: DraggableDimension[] = getDraggablesInsideDroppable( | ||
| droppable, | ||
| const source: DroppableDimension = droppables[draggable.droppableId]; |
There was a problem hiding this comment.
hopefully this is much clearer now
| @@ -79,55 +77,38 @@ const move = ({ | |||
|
|
|||
| const previous: CurrentDrag = state.drag.current; | |||
| const initial: InitialDrag = state.drag.initial; | |||
| const droppable: DroppableDimension = state.dimension.droppable[initial.source.droppableId]; | |||
| const currentWindowScroll: Position = windowScroll || previous.windowScroll; | |||
There was a problem hiding this comment.
rather than everywhere calculating page - it does just done here based on the window scroll
| }; | ||
| return result; | ||
| })(); | ||
|
|
||
| const scrollDiff: Position = subtract(droppable.scroll.initial, droppable.scroll.current); | ||
|
|
||
| const withinDroppable: WithinDroppable = { |
There was a problem hiding this comment.
within droppable is gone. It was only used for get-drag-impact and the new function is much clearer
…o handling-page-refactor
| @@ -69,7 +69,6 @@ export type CompleteLiftAction = {| | |||
| id: DraggableId, | |||
| type: TypeId, | |||
| client: InitialDragLocation, | |||
| page: InitialDragLocation, | |||
There was a problem hiding this comment.
Removed page from all these actions - easy to compute once with windowScroll in the reducer
| @@ -1,182 +0,0 @@ | |||
| // @flow | |||
There was a problem hiding this comment.
now split into different files for clarity and simplification
| |} | ||
|
|
||
| export default ({ | ||
| pageCenter, |
There was a problem hiding this comment.
now calling this out explicitly
| }: Args): DragImpact => { | ||
| const axis: Axis = home.axis; | ||
| const homeScrollDiff: Position = subtract(home.scroll.current, home.scroll.initial); | ||
| const originalCenter: Position = draggable.page.withoutMargin.center; |
There was a problem hiding this comment.
TODO: check this position
There was a problem hiding this comment.
This is correct - the ordering of the variables was a little funny so i have cleaned that up
There was a problem hiding this comment.
This is looking good now @jaredcrowe . I have updated all the tests that where using custom home inHome etc to use the new presets. Other files could be changed but i won't do it in this PR
This PR is designed to simplify how we handle
pagevalues in the system - as well as theget-drag-impactfunction. I noticed that theget-drag-impactfunction was more complicated then it needed to be so I have simplified how it works.This pull request does involve a functional change: currently the impact of a drag is based on the mouse selection point and not the center point which was not intended. Fixing that is as simple as changing the argument to getDragImpact from page.selection to page.center. However, I have gone all out and decided to clean the whole thing up a big before we move on.
I am providing all this detail in case i am unavailable to continue with it @jaredcrowe.