-
Notifications
You must be signed in to change notification settings - Fork 6
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
Swap tiles based on moving direction #1608
base: master
Are you sure you want to change the base?
Conversation
This scenario is added in the Tile Manager specification for manual testing: https://github.com/IgniteUI/igniteui-webcomponents/wiki/Tile-Manager-Specification#manual |
src/components/tile-manager/tile.ts
Outdated
const movingDown = deltaY > 0; | ||
const movingUp = deltaY < 0; | ||
|
||
const isHorizontalMove = Math.abs(deltaX) > Math.abs(deltaY); |
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.
It's very edgy, but if they are equal?
src/components/tile-manager/tile.ts
Outdated
if (this._previousPointerPosition) { | ||
const deltaX = clientX - this._previousPointerPosition.x; | ||
const deltaY = clientY - this._previousPointerPosition.y; | ||
|
||
const movingRight = deltaX > 0; | ||
const movingLeft = deltaX < 0; | ||
const movingDown = deltaY > 0; | ||
const movingUp = deltaY < 0; | ||
|
||
const isHorizontalMove = Math.abs(deltaX) > Math.abs(deltaY); | ||
const isVerticalMove = Math.abs(deltaY) > Math.abs(deltaX); |
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 think you should abstract this state into the drag controller itself. It can hold the previous pointer coordinates when a drag is initiated and return the direction while dragging in its callback parameters. At least it will simplify the code inside the tile drag handler.
|
||
type DragCallback = (parameters: DragCallbackParameters) => unknown; | ||
type DragCancelCallback = (state: DragState) => unknown; | ||
|
||
export type DragCallbackParameters = { | ||
event: PointerEvent; | ||
state: DragState; | ||
direction: Direction | null; |
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.
IMO this should be a property of the DragState
object
@@ -249,7 +287,15 @@ class DragController implements ReactiveController { | |||
this._createDragGhost(); | |||
this._updatePosition(event); | |||
|
|||
const parameters = { event, state: this._stateParameters }; | |||
this._previousPointerPosition = { x: event.clientX, y: event.clientY }; | |||
this._direction = this._trackPointerMovement(event.clientX, event.clientY); |
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 think the direction should be computed in the pointermove handler
type Direction = { | ||
isHorizontalMove: boolean; | ||
movingDown: boolean; | ||
movingToEndHorizontally: boolean; | ||
movingToStartHorizontally: boolean; | ||
}; |
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.
Can this be a union type? Something along the lines of left
| top
| 'top-left`?
this._direction = this._trackPointerMovement(event.clientX, event.clientY); | ||
|
||
const parameters = { | ||
event, | ||
state: this._stateParameters, | ||
direction: this._direction, | ||
}; |
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.
If direction
is a property of the DragState
you can shove those calculations in the _stateParameters
getter and only update the previous pointer coordinates here
this._options.move?.call(this._host, parameters); | ||
|
||
this._assignPosition(this._dragItem); | ||
} | ||
|
||
private _handlePointerEnd(event: PointerEvent): void { | ||
this._previousPointerPosition = null; |
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.
Maybe just initialize the previous coordinates as {x: 0, y: 0}
on initialization of the controller and then just updated through the pointerdown -> pointermove cycle?
@@ -109,6 +122,8 @@ class DragController implements ReactiveController { | |||
private _hasPointerCapture = false; | |||
|
|||
private _ghost: HTMLElement | null = null; | |||
private _previousPointerPosition: { x: number; y: number } | null = null; | |||
private _direction: Direction | null = null; |
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.
Won't need it if it's a part of the DragState
object
Closes #1607