Skip to content

Commit

Permalink
new strategy for draggable dimension correcting
Browse files Browse the repository at this point in the history
  • Loading branch information
alexreardon committed Jun 12, 2018
1 parent 59c3dc3 commit 8275cc7
Show file tree
Hide file tree
Showing 13 changed files with 183 additions and 81 deletions.
2 changes: 0 additions & 2 deletions src/state/auto-scroller/jump-scroller.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,6 @@ export default ({
return;
}

console.warn('manual keyboard move', windowRemainder);

// The entire scroll could not be absorbed by the droppable and window
// so we manually move whatever is left
moveByOffset(state, windowRemainder);
Expand Down
12 changes: 9 additions & 3 deletions src/state/dimension-marshal/collector.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import type {

type CollectArgs = {|
includeCritical: boolean,
initialWindowScroll: Position,
|}

type CollectFromDOMArgs = {|
Expand Down Expand Up @@ -55,7 +56,11 @@ export default ({
// tmep
// let timerId: ?TimeoutID = null;

const collectFromDOM = ({ windowScroll, includeCritical }: CollectFromDOMArgs): DimensionMap => {
const collectFromDOM = ({
windowScroll,
initialWindowScroll,
includeCritical,
}: CollectFromDOMArgs): DimensionMap => {
const critical: Critical = getCritical();
const scrollOptions: ScrollOptions = getScrollOptions();
const entries: Entries = getEntries();
Expand Down Expand Up @@ -118,7 +123,7 @@ export default ({
}, {});

const draggableDimensions: DraggableDimensionMap = draggables
.map((entry: DraggableEntry): DraggableDimension => entry.getDimension(windowScroll))
.map((entry: DraggableEntry): DraggableDimension => entry.getDimension(windowScroll, initialWindowScroll))
.reduce((previous: DraggableDimensionMap, current: DraggableDimension) => {
previous[current.descriptor.id] = current;
return previous;
Expand All @@ -141,7 +146,7 @@ export default ({
frameId = null;
};

const collect = ({ includeCritical }: CollectArgs) => {
const collect = ({ includeCritical, initialWindowScroll }: CollectArgs) => {
abortFrame();
// clearTimeout(timerId);

Expand All @@ -153,6 +158,7 @@ export default ({
const dimensions: DimensionMap = collectFromDOM({
windowScroll: viewport.scroll.current,
includeCritical,
initialWindowScroll,
});
timings.finish('DOM collection');

Expand Down
11 changes: 9 additions & 2 deletions src/state/dimension-marshal/dimension-marshal-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,15 @@ import type {
LiftRequest,
} from '../../types';

export type GetDraggableDimensionFn = (windowScroll: Position) => DraggableDimension;
export type GetDroppableDimensionFn = (windowScroll: Position, options: ScrollOptions) => DroppableDimension;
export type GetDraggableDimensionFn = (
windowScroll: Position,
windowScrollDiff: Position
) => DraggableDimension;

export type GetDroppableDimensionFn = (
windowScroll: Position,
options: ScrollOptions
) => DroppableDimension;

export type DroppableCallbacks = {|
getDimensionAndWatchScroll: GetDroppableDimensionFn,
Expand Down
7 changes: 6 additions & 1 deletion src/state/dimension-marshal/dimension-marshal.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import type {
type Collection = {|
scrollOptions: ScrollOptions,
critical: Critical,
initialWindowScroll: Position,
|}

export default (callbacks: Callbacks) => {
Expand Down Expand Up @@ -86,7 +87,10 @@ export default (callbacks: Callbacks) => {
// Let the application know a bulk collection is starting
callbacks.bulkCollectionStarting();

collector.collect({ includeCritical });
collector.collect({
includeCritical,
initialWindowScroll: collection.initialWindowScroll,
});
};

const registerDraggable = (
Expand Down Expand Up @@ -315,6 +319,7 @@ export default (callbacks: Callbacks) => {
collection = {
scrollOptions: request.scrollOptions,
critical,
initialWindowScroll: windowScroll,
};

const dimensions: DimensionMap = getCritical(windowScroll);
Expand Down
2 changes: 0 additions & 2 deletions src/state/get-drag-impact/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ export default ({
previousDroppableOverId,
});

console.log('destination', destinationId);

// not dragging over anything
if (!destinationId) {
return noImpact;
Expand Down
4 changes: 0 additions & 4 deletions src/state/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,6 @@ const moveWithPositionUpdates = ({
client, page,
};

console.warn('moved to page center', page.borderBoxCenter);

// Not updating impact while bulk collecting
if (state.phase === 'BULK_COLLECTING') {
return {
Expand Down Expand Up @@ -148,8 +146,6 @@ export default (state: State = idle, action: Action): State => {
},
};

console.warn('initial page center', initial.page.borderBoxCenter);

// Calculating initial impact
const impact: DragImpact = getHomeImpact(critical, dimensions);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import PropTypes from 'prop-types';
import memoizeOne from 'memoize-one';
import invariant from 'tiny-invariant';
import { calculateBox, withScroll, getBox, offset, type BoxModel, type Position } from 'css-box-model';
import { negate, subtract } from '../../state/position';
import { negate, subtract, add } from '../../state/position';
import { dimensionMarshalKey } from '../context-keys';
import type {
DraggableDescriptor,
Expand All @@ -23,7 +23,6 @@ type Props = {|
isDragging: boolean,
offset: Position,
getDraggableRef: () => ?HTMLElement,
getPlaceholderRef: () => ?HTMLElement,
children: Node,
|}

Expand Down Expand Up @@ -91,7 +90,7 @@ export default class DraggableDimensionPublisher extends Component<Props> {
this.publishedDescriptor = null;
}

getDimension = (windowScroll: Position): DraggableDimension => {
getDimension = (windowScroll: Position, windowScrollDiff: Position): DraggableDimension => {
const targetRef: ?HTMLElement = this.props.getDraggableRef();
const descriptor: ?DraggableDescriptor = this.publishedDescriptor;

Expand All @@ -111,23 +110,17 @@ export default class DraggableDimensionPublisher extends Component<Props> {
}

// When dragging, position: fixed will avoid any client changes based on scroll.
// We are manually undoing that
return subtract(undoTransform, windowScroll);
})();

// Object.assign(ref.style, previous);
// We are manually applying these client changes based on the change in window scroll
// from when the drag started
const undoWindowScroll: Position = negate(windowScrollDiff);

const client: BoxModel = offset(calculateBox(borderBox, computedStyles), change);
return add(undoTransform, undoWindowScroll);
})();

const box: BoxModel = calculateBox(borderBox, computedStyles);
const client: BoxModel = offset(box, change);
const page: BoxModel = withScroll(client, windowScroll);

console.warn(descriptor.id, 'collected pageBorderBoxCenter', page.borderBox.center, 'height', client.borderBox.height);
// if (placeholderRef) {
// const box = getBox(targetRef);
// console.warn(descriptor.id, 'targetRef (not placeholder)
// pageBorderBoxCenter', box.borderBox.center, 'height', box.borderBox.height);
// }

const boxSizing: BoxSizing = computedStyles.boxSizing === 'border-box' ? 'border-box' : 'content-box';

const placeholder: Placeholder = {
Expand Down
11 changes: 2 additions & 9 deletions src/view/draggable/draggable.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ export default class Draggable extends Component<Props> {
callbacks: DragHandleCallbacks
styleContext: string
ref: ?HTMLElement = null
placeholderRef: ?HTMLElement = null

// Need to declare contextTypes without flow
// https://github.com/brigand/babel-plugin-flow-react-proptypes/issues/22
Expand Down Expand Up @@ -197,12 +196,6 @@ export default class Draggable extends Component<Props> {

getDraggableRef = (): ?HTMLElement => this.ref;

setPlaceholderRef = (ref: ?HTMLElement) => {
this.placeholderRef = ref;
}

getPlaceholderRef = (): ?HTMLElement => this.placeholderRef;

getDraggingStyle = memoizeOne(
(dimension: DraggableDimension,
isDropAnimating: boolean,
Expand Down Expand Up @@ -343,7 +336,7 @@ export default class Draggable extends Component<Props> {
return null;
}

return <Placeholder placeholder={dimension.placeholder} innerRef={this.setPlaceholderRef} />;
return <Placeholder placeholder={dimension.placeholder} />;
})();

return (
Expand Down Expand Up @@ -380,8 +373,8 @@ export default class Draggable extends Component<Props> {
droppableId={droppableId}
index={index}
offset={offset}
isDragging={isDragging}
getDraggableRef={this.getDraggableRef}
getPlaceholderRef={this.getPlaceholderRef}
>
<Moveable
speed={speed}
Expand Down
5 changes: 0 additions & 5 deletions src/view/placeholder/placeholder.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import type { Placeholder as PlaceholderType } from '../../types';

type Props = {|
placeholder: PlaceholderType,
innerRef: (ref: ?HTMLElement) => void,
|}

export default class Placeholder extends PureComponent<Props> {
Expand All @@ -28,10 +27,6 @@ export default class Placeholder extends PureComponent<Props> {

setRef = (ref: ?HTMLElement) => {
this.ref = ref;

if (this.props.innerRef) {
this.props.innerRef(this.ref);
}
}

render() {
Expand Down
1 change: 0 additions & 1 deletion test/unit/view/dimension-marshal/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,4 +111,3 @@ export const getCallbacksStub = (): Callbacks => ({
updateDroppableIsEnabled: jest.fn(),
bulkCollectionStarting: jest.fn(),
});

Loading

0 comments on commit 8275cc7

Please sign in to comment.