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

less dom reads #358

Merged
merged 12 commits into from
Mar 5, 2018
29 changes: 15 additions & 14 deletions src/state/action-creators.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type {
LiftRequest,
AutoScrollMode,
ScrollOptions,
Viewport,
} from '../types';
import noImpact from './no-impact';
import withDroppableDisplacement from './with-droppable-displacement';
Expand All @@ -37,8 +38,8 @@ const getScrollDiff = ({
droppable,
}: ScrollDiffArgs): Position => {
const windowScrollDiff: Position = subtract(
initial.windowScroll,
current.windowScroll
initial.viewport.scroll,
current.viewport.scroll,
);

if (!droppable) {
Expand All @@ -63,22 +64,22 @@ export type CompleteLiftAction = {|
payload: {|
id: DraggableId,
client: InitialDragPositions,
windowScroll: Position,
viewport: Viewport,
autoScrollMode: AutoScrollMode,
|}
|}

export const completeLift = (
id: DraggableId,
client: InitialDragPositions,
windowScroll: Position,
viewport: Viewport,
autoScrollMode: AutoScrollMode,
): CompleteLiftAction => ({
type: 'COMPLETE_LIFT',
payload: {
id,
client,
windowScroll,
viewport,
autoScrollMode,
},
});
Expand Down Expand Up @@ -163,22 +164,22 @@ export type MoveAction = {|
payload: {|
id: DraggableId,
client: Position,
windowScroll: Position,
viewport: Viewport,
shouldAnimate: boolean,
|}
|}

export const move = (
id: DraggableId,
client: Position,
windowScroll: Position,
viewport: Viewport,
shouldAnimate?: boolean = false,
): MoveAction => ({
type: 'MOVE',
payload: {
id,
client,
windowScroll,
viewport,
shouldAnimate,
},
});
Expand All @@ -187,16 +188,16 @@ export type MoveByWindowScrollAction = {|
type: 'MOVE_BY_WINDOW_SCROLL',
payload: {|
id: DraggableId,
windowScroll: Position,
viewport: Viewport,
|}
|}

export const moveByWindowScroll =
(id: DraggableId, windowScroll: Position): MoveByWindowScrollAction => ({
(id: DraggableId, viewport: Viewport): MoveByWindowScrollAction => ({
type: 'MOVE_BY_WINDOW_SCROLL',
payload: {
id,
windowScroll,
viewport,
},
});

Expand Down Expand Up @@ -451,7 +452,7 @@ export type LiftAction = {|
payload: {|
id: DraggableId,
client: InitialDragPositions,
windowScroll: Position,
viewport: Viewport,
autoScrollMode: AutoScrollMode,
|}
|}
Expand All @@ -460,7 +461,7 @@ export type LiftAction = {|
// as the descriptor might change after a drop is flushed
export const lift = (id: DraggableId,
client: InitialDragPositions,
windowScroll: Position,
viewport: Viewport,
autoScrollMode: AutoScrollMode,
) => (dispatch: Dispatch, getState: Function) => {
// Phase 1: Quickly finish any current drop animations
Expand Down Expand Up @@ -511,7 +512,7 @@ export const lift = (id: DraggableId,
return;
}

dispatch(completeLift(id, client, windowScroll, autoScrollMode));
dispatch(completeLift(id, client, viewport, autoScrollMode));
});
});
};
Expand Down
47 changes: 9 additions & 38 deletions src/state/auto-scroller/can-scroll.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
// @flow
import { add, apply, isEqual } from '../position';
import getWindowScroll from '../../window/get-window-scroll';
import getViewport from '../../window/get-viewport';
import getMaxScroll from '../get-max-scroll';
import type {
ClosestScrollable,
DroppableDimension,
Position,
Area,
Viewport,
} from '../../types';

type CanScrollArgs = {|
Expand Down Expand Up @@ -94,38 +91,12 @@ export const canPartiallyScroll = ({
return false;
};

const getMaxWindowScroll = (): Position => {
const el: ?HTMLElement = document.documentElement;

if (!el) {
return origin;
}

const viewport: Area = getViewport();

// window.innerWidth / innerHeight includes scrollbar
// however the scrollHeight / scrollWidth do not :(

const maxScroll: Position = getMaxScroll({
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is now computed in getViewport

scrollHeight: el.scrollHeight,
scrollWidth: el.scrollWidth,
width: viewport.width,
height: viewport.height,
});

return maxScroll;
};

export const canScrollWindow = (change: Position): boolean => {
const maxScroll: Position = getMaxWindowScroll();
const currentScroll: Position = getWindowScroll();

return canPartiallyScroll({
current: currentScroll,
max: maxScroll,
export const canScrollWindow = (viewport: Viewport, change: Position): boolean =>
canPartiallyScroll({
current: viewport.scroll,
max: viewport.maxScroll,
change,
});
};

export const canScrollDroppable = (
droppable: DroppableDimension,
Expand All @@ -144,13 +115,13 @@ export const canScrollDroppable = (
});
};

export const getWindowOverlap = (change: Position): ?Position => {
if (!canScrollWindow(change)) {
export const getWindowOverlap = (viewport: Viewport, change: Position): ?Position => {
if (!canScrollWindow(viewport, change)) {
return null;
}

const max: Position = getMaxWindowScroll();
const current: Position = getWindowScroll();
const max: Position = viewport.maxScroll;
const current: Position = viewport.scroll;

return getOverlap({
current,
Expand Down
8 changes: 4 additions & 4 deletions src/state/auto-scroller/fluid-scroller.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// @flow
import rafSchd from 'raf-schd';
import getViewport from '../../window/get-viewport';
import { add, apply, isEqual, patch } from '../position';
import getBestScrollableDroppable from './get-best-scrollable-droppable';
import { horizontal, vertical } from '../axis';
Expand All @@ -19,6 +18,7 @@ import type {
State,
DraggableDimension,
ClosestScrollable,
Viewport,
} from '../../types';

// Values used to control how the fluid auto scroll feels
Expand Down Expand Up @@ -261,14 +261,14 @@ export default ({

const draggable: DraggableDimension = state.dimension.draggable[drag.initial.descriptor.id];
const subject: Area = draggable.page.marginBox;
const viewport: Area = getViewport();
const viewport: Viewport = drag.current.viewport;
const requiredWindowScroll: ?Position = getRequiredScroll({
container: viewport,
container: viewport.subject,
subject,
center,
});

if (requiredWindowScroll && canScrollWindow(requiredWindowScroll)) {
if (requiredWindowScroll && canScrollWindow(viewport, requiredWindowScroll)) {
scheduleWindowScroll(requiredWindowScroll);
return;
}
Expand Down
3 changes: 2 additions & 1 deletion src/state/auto-scroller/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type {
DroppableId,
Position,
State,
Viewport,
} from '../../types';

type Args = {|
Expand All @@ -15,7 +16,7 @@ type Args = {|
move: (
id: DraggableId,
client: Position,
windowScroll: Position,
viewport: Viewport,
shouldAnimate?: boolean
) => void,
|}
Expand Down
15 changes: 8 additions & 7 deletions src/state/auto-scroller/jump-scroller.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// @flow
import { add, subtract } from '../position';
import getWindowScroll from '../../window/get-window-scroll';
import {
canScrollDroppable,
canScrollWindow,
Expand All @@ -15,6 +14,7 @@ import type {
Position,
State,
DraggableLocation,
Viewport,
} from '../../types';

type Args = {|
Expand All @@ -23,7 +23,7 @@ type Args = {|
move: (
id: DraggableId,
client: Position,
windowScroll: Position,
viewport: Viewport,
shouldAnimate?: boolean
) => void,
|}
Expand All @@ -45,7 +45,7 @@ export default ({
}

const client: Position = add(drag.current.client.selection, offset);
move(drag.initial.descriptor.id, client, getWindowScroll(), true);
move(drag.initial.descriptor.id, client, drag.current.viewport, true);
};

const scrollDroppableAsMuchAsItCan = (
Expand Down Expand Up @@ -73,13 +73,13 @@ export default ({
return remainder;
};

const scrollWindowAsMuchAsItCan = (change: Position): ?Position => {
const scrollWindowAsMuchAsItCan = (viewport: Viewport, change: Position): ?Position => {
// window cannot absorb any of the scroll
if (!canScrollWindow(change)) {
if (!canScrollWindow(viewport, change)) {
return change;
}

const overlap: ?Position = getWindowOverlap(change);
const overlap: ?Position = getWindowOverlap(viewport, change);

// window can absorb entire scroll
if (!overlap) {
Expand Down Expand Up @@ -128,7 +128,8 @@ export default ({
return;
}

const windowRemainder: ?Position = scrollWindowAsMuchAsItCan(droppableRemainder);
const viewport: Viewport = drag.current.viewport;
const windowRemainder: ?Position = scrollWindowAsMuchAsItCan(viewport, droppableRemainder);

// window could absorb all the droppable remainder
if (!windowRemainder) {
Expand Down
2 changes: 2 additions & 0 deletions src/state/create-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ export default (): Store => createStore(
// require('./debug-middleware/log-middleware').default,
// debugging timer
// require('./debug-middleware/timing-middleware').default,
// average action timer
// require('./debug-middleware/timing-average-middleware').default(20),
),
),
);
46 changes: 46 additions & 0 deletions src/state/debug-middleware/timing-average-middleware.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// @flow
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

another debug middleware - this one averages the time taken for actions and prints a result

/* eslint-disable no-console */
import type { Action } from '../../types';

type Bucket = {
[key: string]: number[],
}

const average = (values: number[]): number => {
const sum: number = values.reduce((previous: number, current: number) => previous + current, 0);
return sum / values.length;
};

export default (groupSize: number) => {
console.log('Starting average action timer middleware');
console.log(`Will take an average every ${groupSize} actions`);
const bucket: Bucket = {};

return () => (next: (Action) => mixed) => (action: Action): mixed => {
const start: number = performance.now();

const result: mixed = next(action);

const end: number = performance.now();

const duration: number = end - start;

if (!bucket[action.type]) {
bucket[action.type] = [duration];
return result;
}

bucket[action.type].push(duration);

if (bucket[action.type].length < groupSize) {
return result;
}

console.warn(`Average time for ${action.type}`, average(bucket[action.type]));

// reset
bucket[action.type] = [];

return result;
};
};
Loading