less dom reads #358
less dom reads #358
Conversation
| // window.innerWidth / innerHeight includes scrollbar | ||
| // however the scrollHeight / scrollWidth do not :( | ||
|
|
||
| const maxScroll: Position = getMaxScroll({ |
There was a problem hiding this comment.
this is now computed in getViewport
| @@ -0,0 +1,46 @@ | |||
| // @flow | |||
There was a problem hiding this comment.
another debug middleware - this one averages the time taken for actions and prints a result
| @@ -49,8 +51,6 @@ export default ({ | |||
| sourceClipped[axis.start], | |||
| sourceClipped[axis.end] | |||
| ); | |||
| const viewport: Area = getViewport(); | |||
There was a problem hiding this comment.
that's what i'm talking about 👍
| @@ -74,7 +73,6 @@ export default ({ | |||
| return 'start'; | |||
| })(); | |||
|
|
|||
| const viewport: Area = getViewport(); | |||
There was a problem hiding this comment.
goodbye old friend
| const drag: ?DragState = state.drag; | ||
|
|
||
| if (!drag) { | ||
| console.error('cannot move with window scrolling if no current drag'); | ||
| return clean(); | ||
| } | ||
|
|
||
| if (isEqual(windowScroll, drag.current.windowScroll)) { | ||
| // TODO: need to improve this so that it compares the whole viewport |
There was a problem hiding this comment.
I think this check is good enough for now
| @@ -13,7 +13,7 @@ import type { | |||
| import DraggableDimensionPublisher from '../draggable-dimension-publisher/'; | |||
| import Moveable from '../moveable/'; | |||
| import DragHandle from '../drag-handle'; | |||
| import getWindowScroll from '../../window/get-window-scroll'; | |||
| import getViewport from '../window/get-viewport'; | |||
There was a problem hiding this comment.
this is now the only thing that publishes the viewport into the state
| }); | ||
|
|
||
| const maxScroll: Position = getMaxScroll({ | ||
| scrollHeight: doc.scrollHeight, |
There was a problem hiding this comment.
It turns out these properties are super slow to read sometimes
| @@ -49,6 +49,10 @@ export default class AuthorApp extends Component<Props, State> { | |||
| return; | |||
| } | |||
|
|
|||
| if (result.destination.index === result.source.index) { | |||
There was a problem hiding this comment.
A little optimisation for our storybooks
| @@ -30,7 +33,18 @@ import type { | |||
| } from '../../../src/types'; | |||
|
|
|||
| const preset = getPreset(); | |||
| const origin: Position = { x: 0, y: 0 }; | |||
|
|
|||
| const scrolledViewport: Viewport = createViewport({ | |||
There was a problem hiding this comment.
much clearer now
| scrollHeight: viewport.height, | ||
| scrollWidth: viewport.width, | ||
| }); | ||
| const unscrollableViewport: Viewport = { |
| @@ -0,0 +1,110 @@ | |||
| // @flow | |||
There was a problem hiding this comment.
improved helpers
Closes #329.
Fixing a minor performance regressed in lifting big data sets with our new auto scroller.
What this PR does:
statefolder (engineering health)The code changes in
srcare super basic. There was some more heavy lifting required to get everything working in the tests. Previously tests were setting viewport properties in anticipation of the viewport being read. Now these properties can be passed directly to functions