-
Notifications
You must be signed in to change notification settings - Fork 39
Drag Meta Provider #637
Drag Meta Provider #637
Changes from 6 commits
e680af1
2dce035
ca2c502
f0e41b9
ba7afc7
ddc825d
5d637fe
6538b05
9a318b7
9ba4fae
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,170 @@ | ||
import global from '@dojo/shim/global'; | ||
import { assign } from '@dojo/shim/object'; | ||
import WeakMap from '@dojo/shim/WeakMap'; | ||
import { Base } from './Base'; | ||
|
||
export interface DragResults { | ||
/** | ||
* The movement of pointer during the duration of the drag state | ||
*/ | ||
delta: Position; | ||
|
||
/** | ||
* Is the DOM node currently in a drag state | ||
*/ | ||
isDragging: boolean; | ||
} | ||
|
||
interface NodeData { | ||
dragResults: DragResults; | ||
invalidate: () => void; | ||
last: Position; | ||
start: Position; | ||
} | ||
|
||
export interface Position { | ||
x: number; | ||
y: number; | ||
} | ||
|
||
/** | ||
* A frozen empty result object, frozen to ensure that no one downstream modifies it | ||
*/ | ||
const emptyResults = Object.freeze({ | ||
delta: Object.freeze({ x: 0, y: 0 }), | ||
isDragging: false | ||
}); | ||
|
||
/** | ||
* Return the x/y position for an event | ||
* @param e The MouseEvent or TouchEvent | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. now the |
||
*/ | ||
function getPosition(e: PointerEvent): Position { | ||
return { | ||
x: e.pageX, | ||
y: e.pageY | ||
}; | ||
} | ||
|
||
/** | ||
* Return the delta position between two positions | ||
* @param start The first posistion | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. posistion -> position |
||
* @param current The second posistion | ||
*/ | ||
function getDelta(start: Position, current: Position): Position { | ||
return { | ||
x: current.x - start.x, | ||
y: current.y - start.y | ||
}; | ||
} | ||
|
||
class DragController { | ||
private _nodeMap = new WeakMap<HTMLElement, NodeData>(); | ||
private _dragging: HTMLElement | undefined = undefined; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Couldn't this be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It could be, but considering I was explicitly assigning |
||
|
||
private _getData(target: HTMLElement): { state: NodeData, target: HTMLElement } | undefined { | ||
if (this._nodeMap.has(target)) { | ||
return { state: this._nodeMap.get(target)!, target }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it would be so good if TS could know that we are in a block narrowed by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not surprised there’s an issue. That would be nice. |
||
} | ||
if (target.parentElement) { | ||
return this._getData(target.parentElement); | ||
} | ||
} | ||
|
||
private _onDragStart = (e: PointerEvent) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any reason why There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: could we have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why the lambda, is because the handler needs to be bound to the instance. When used as a handler, the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gotcha re scope. Over having a separate variable that binds scope to the method (we have used both patterns) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah... I like this better for class methods where it effects the code readability less, IMO. Using the bound variable makes a bit of indirection when reading the code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, for private methods. |
||
const data = this._getData(e.target as HTMLElement); | ||
if (data) { | ||
const { state, target } = data; | ||
this._dragging = target; | ||
state.dragResults.isDragging = true; | ||
state.last = state.start = getPosition(e); | ||
state.dragResults.delta = { x: 0, y: 0 }; | ||
state.invalidate(); | ||
} // else, we are ignoring the event | ||
} | ||
|
||
private _onDrag = (e: PointerEvent) => { | ||
const { _dragging } = this; | ||
if (!_dragging) { | ||
return; | ||
} | ||
// state cannot be unset, using ! operator | ||
const state = this._nodeMap.get(_dragging)!; | ||
state.last = getPosition(e); | ||
state.dragResults.delta = getDelta(state.start, state.last); | ||
state.invalidate(); | ||
} | ||
|
||
private _onDragStop = (e: PointerEvent) => { | ||
const { _dragging } = this; | ||
if (!_dragging) { | ||
return; | ||
} | ||
// state cannot be unset, using ! operator | ||
const state = this._nodeMap.get(_dragging)!; | ||
state.last = getPosition(e); | ||
state.dragResults = { | ||
delta: getDelta(state.start, state.last), | ||
isDragging: false | ||
}; | ||
state.invalidate(); | ||
this._dragging = undefined; | ||
} | ||
|
||
constructor() { | ||
const win: Window = global.window; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't want to mask the global. We have tended to now do that, using |
||
win.addEventListener('pointerdown', this._onDragStart); | ||
win.addEventListener('pointermove', this._onDrag, true); | ||
win.addEventListener('pointerup', this._onDragStop, true); | ||
} | ||
|
||
public get(node: HTMLElement, invalidate: () => void): DragResults { | ||
const { _nodeMap } = this; | ||
// first time we see a node, we will initialize its state | ||
if (!_nodeMap.has(node)) { | ||
_nodeMap.set(node, { | ||
dragResults: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. apart from it being frozen... could we just deep assign the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well, empty results is valid here, I will use that |
||
delta: { x: 0, y: 0 }, | ||
isDragging: false | ||
}, | ||
invalidate, | ||
last: { x: 0, y: 0 }, | ||
start: { x: 0, y: 0 } | ||
}); | ||
return emptyResults; | ||
} | ||
|
||
const state = _nodeMap.get(node)!; | ||
// we are offering up an accurate delta, so we need to take the last event position and move it to the start so | ||
// that our deltas are calculated from the last time they are read | ||
state.start = state.last; | ||
// shallow "clone" the results, so no downstream manipulation can occur | ||
const dragResults = assign({}, state.dragResults); | ||
|
||
// reset the delta after we have read any last delta while not dragging | ||
if (!dragResults.isDragging && dragResults.delta.x !== 0 && dragResults.delta.y !== 0) { | ||
// future reads of the delta will be blank | ||
state.dragResults.delta = { x: 0, y: 0 }; | ||
} | ||
|
||
return dragResults; | ||
} | ||
} | ||
|
||
const controller = new DragController(); | ||
|
||
export default class Drag extends Base { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. generally we do a named export and a default export across widget-core. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no problem |
||
private boundInvalidate = this.invalidate.bind(this); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
||
public get(key: string): Readonly<DragResults> { | ||
const node = this.getNode(key); | ||
|
||
// if we don't have a reference to the node yet, return an empty set of results | ||
if (!node) { | ||
return emptyResults; | ||
} | ||
|
||
// otherwise we will ask the controller for our results | ||
return controller.get(node, this.boundInvalidate); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: should we declare the interface before we use it? (I thought it was an from an import)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was favouring alphabetisation over use before declaration... one form of clarity causes another issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess so.