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.

Sign up for GitHub

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

Jump to bottom

feat: add preventClickOnDrag option to PanInput #199

Merged
merged 6 commits into from
Aug 30, 2022
Merged

feat: add preventClickOnDrag option to PanInput #199

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dade936
feat: add preventClickOnDrag option to PanInput
malangfox Aug 16, 2022
2c9b4b5
skip: apply review
malangfox Aug 26, 2022
133a1e5
skip: apply review
malangfox Aug 26, 2022
520d04a
fix: set preventClick capture to true
malangfox Aug 26, 2022
574fe21
chore: throw error when target element does not exist
malangfox Aug 30, 2022
da956d1
skip: apply review
malangfox Aug 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions packages/axes/src/inputType/MoveKeyInput.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export interface MoveKeyInputOption {
* @example
* ```js
* const moveKey = new eg.Axes.MoveKeyInput("#area", {
* scale: [1, 1]
* scale: [1, 1]
* });
*
* // Connect 'x', 'y' axes when the moveKey is pressed.
Expand Down Expand Up @@ -199,17 +199,22 @@ export class MoveKeyInput implements InputType {
}

private _attachEvent(observer: InputTypeObserver) {
const element = this.element;
if (!element) {
throw new Error("Element to connect input does not exist.");
}
this._observer = observer;
this.element.addEventListener("keydown", this._onKeydown, false);
this.element.addEventListener("keypress", this._onKeydown, false);
this.element.addEventListener("keyup", this._onKeyup, false);
element.addEventListener("keydown", this._onKeydown, false);
element.addEventListener("keypress", this._onKeydown, false);
element.addEventListener("keyup", this._onKeyup, false);
this._enabled = true;
}

private _detachEvent() {
this.element.removeEventListener("keydown", this._onKeydown, false);
this.element.removeEventListener("keypress", this._onKeydown, false);
this.element.removeEventListener("keyup", this._onKeyup, false);
const element = this.element;
element?.removeEventListener("keydown", this._onKeydown, false);
element?.removeEventListener("keypress", this._onKeydown, false);
element?.removeEventListener("keyup", this._onKeyup, false);
Copy link
Member

Choose a reason for hiding this comment

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

You'd rather do it like:

const element = this.element;
if (element) {
  element.removeEventListener("keydown", this._onKeydown, false);
  element.removeEventListener("keypress", this._onKeydown, false);
  element.removeEventListener("keyup", this._onKeyup, false);
}

Because simply, the one you wrote turns like this on tsc compiler:

var element = this.element;
element === null || element === void 0 ? void 0 : element.removeEventListener("keydown", this._onKeydown, false);
element === null || element === void 0 ? void 0 : element.removeEventListener("keypress", this._onKeydown, false);
element === null || element === void 0 ? void 0 : element.removeEventListener("keyup", this._onKeyup, false);

this._enabled = false;
this._observer = null;
}
Expand Down
33 changes: 29 additions & 4 deletions packages/axes/src/inputType/PanInput.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export interface PanInputOption {
scale?: number[];
thresholdAngle?: number;
threshold?: number;
preventClickOnDrag?: boolean;
iOSEdgeSwipeThreshold?: number;
releaseOnScroll?: boolean;
touchAction?: string;
Expand Down Expand Up @@ -77,6 +78,7 @@ export const getDirectionByAngle = (
* @param {Number} [scale[1]=1] vertical axis scale <ko>수직축 배율</ko>
* @param {Number} [thresholdAngle=45] The threshold value that determines whether user action is horizontal or vertical (0~90) <ko>사용자의 동작이 가로 방향인지 세로 방향인지 판단하는 기준 각도(0~90)</ko>
* @param {Number} [threshold=0] Minimal pan distance required before recognizing <ko>사용자의 Pan 동작을 인식하기 위해산 최소한의 거리</ko>
* @param {Boolean} [preventClickOnDrag=false] Whether to cancel the {@link https://developer.mozilla.org/en/docs/Web/API/Element/click_event click} event when the user finishes dragging more than 1 pixel <ko>사용자가 1픽셀 이상 드래그를 마쳤을 때 {@link https://developer.mozilla.org/ko/docs/Web/API/Element/click_event click} 이벤트 취소 여부/ko>
* @param {Number} [iOSEdgeSwipeThreshold=30] Area (px) that can go to the next page when swiping the right edge in iOS safari <ko>iOS Safari에서 오른쪽 엣지를 스와이프 하는 경우 다음 페이지로 넘어갈 수 있는 영역(px)</ko>
* @param {String} [touchAction=null] Value that overrides the element's "touch-action" css property. If set to null, it is automatically set to prevent scrolling in the direction of the connected axis. <ko>엘리먼트의 "touch-action" CSS 속성을 덮어쓰는 값. 만약 null로 설정된 경우, 연결된 축 방향으로의 스크롤을 방지하게끔 자동으로 설정된다.</ko>
**/
Expand Down Expand Up @@ -115,6 +117,7 @@ export class PanInput implements InputType {
private _originalCssProps: { [key: string]: string };
private _atRightEdge = false;
private _rightEdgeTimer = 0;
private _dragged = false;

/**
*
Expand All @@ -127,6 +130,7 @@ export class PanInput implements InputType {
scale: [1, 1],
thresholdAngle: 45,
threshold: 0,
preventClickOnDrag: false,
iOSEdgeSwipeThreshold: IOS_EDGE_THRESHOLD,
releaseOnScroll: false,
touchAction: null,
Expand Down Expand Up @@ -227,6 +231,7 @@ export class PanInput implements InputType {
if (panEvent.srcEvent.cancelable !== false) {
const edgeThreshold = this.options.iOSEdgeSwipeThreshold;

this._dragged = false;
this._observer.hold(this, panEvent);
this._atRightEdge =
IS_IOS_SAFARI && panEvent.center.x > window.innerWidth - edgeThreshold;
Expand Down Expand Up @@ -291,6 +296,7 @@ export class PanInput implements InputType {
}
panEvent.preventSystemEvent = prevent;
if (prevent) {
this._dragged = true;
this._observer.change(this, panEvent, toAxis(this.axes, offset));
}
activeEvent.prevEvent = panEvent;
Expand Down Expand Up @@ -347,32 +353,51 @@ export class PanInput implements InputType {

private _attachElementEvent(observer: InputTypeObserver) {
const activeEvent = convertInputType(this.options.inputType);
const element = this.element;
if (!activeEvent) {
return;
}
if (!element) {
throw new Error("Element to connect input does not exist.");
}
this._observer = observer;
this._enabled = true;
this._activeEvent = activeEvent;
if (this.options.preventClickOnDrag) {
element.addEventListener("click", this._preventClickWhenDragged, true);
}
activeEvent.start.forEach((event) => {
this.element?.addEventListener(event, this._onPanstart);
element.addEventListener(event, this._onPanstart);
});
// adding event listener to element prevents invalid behavior in iOS Safari
activeEvent.move.forEach((event) => {
this.element?.addEventListener(event, this._voidFunction);
element.addEventListener(event, this._voidFunction);
});
}

private _detachElementEvent() {
const activeEvent = this._activeEvent;
const element = this.element;
if (this.options.preventClickOnDrag) {
element?.removeEventListener("click", this._preventClickWhenDragged, true);
}
activeEvent?.start.forEach((event) => {
this.element?.removeEventListener(event, this._onPanstart);
element?.removeEventListener(event, this._onPanstart);
});
activeEvent?.move.forEach((event) => {
this.element?.removeEventListener(event, this._voidFunction);
element?.removeEventListener(event, this._voidFunction);
});
this._enabled = false;
this._observer = null;
}

private _preventClickWhenDragged = (e: PointerEvent | MouseEvent) => {
if (this._dragged) {
e.preventDefault();
e.stopPropagation();
}
this._dragged = false;
};

private _voidFunction = () => {};
}
19 changes: 12 additions & 7 deletions packages/axes/src/inputType/PinchInput.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export interface PinchInputOption {
* @example
* ```js
* const pinch = new eg.Axes.PinchInput("#area", {
* scale: 1
* scale: 1
* });
*
* // Connect 'something' axis when two pointers are moving toward (zoom-in) or away from each other (zoom-out).
Expand Down Expand Up @@ -191,33 +191,38 @@ export class PinchInput implements InputType {

private _attachEvent(observer: InputTypeObserver) {
const activeEvent = convertInputType(this.options.inputType);
const element = this.element;
if (!activeEvent) {
return;
}
if (!element) {
throw new Error("Element to connect input does not exist.");
}
this._observer = observer;
this._enabled = true;
this._activeEvent = activeEvent;
activeEvent.start.forEach((event) => {
this.element.addEventListener(event, this._onPinchStart, false);
element.addEventListener(event, this._onPinchStart, false);
});
activeEvent.move.forEach((event) => {
this.element.addEventListener(event, this._onPinchMove, false);
element.addEventListener(event, this._onPinchMove, false);
});
activeEvent.end.forEach((event) => {
this.element.addEventListener(event, this._onPinchEnd, false);
element.addEventListener(event, this._onPinchEnd, false);
});
}

private _detachEvent() {
const activeEvent = this._activeEvent;
const element = this.element;
activeEvent?.start.forEach((event) => {
this.element.removeEventListener(event, this._onPinchStart, false);
element?.removeEventListener(event, this._onPinchStart, false);
});
activeEvent?.move.forEach((event) => {
this.element.removeEventListener(event, this._onPinchMove, false);
element?.removeEventListener(event, this._onPinchMove, false);
});
activeEvent?.end.forEach((event) => {
this.element.removeEventListener(event, this._onPinchEnd, false);
element?.removeEventListener(event, this._onPinchEnd, false);
});
this._enabled = false;
this._observer = null;
Expand Down
6 changes: 5 additions & 1 deletion packages/axes/src/inputType/WheelInput.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,12 @@ export class WheelInput implements InputType {
}

private _attachEvent(observer: InputTypeObserver) {
const element = this.element;
if (!element) {
throw new Error("Element to connect input does not exist.");
}
this._observer = observer;
this.element.addEventListener("wheel", this._onWheel);
element.addEventListener("wheel", this._onWheel);
this._enabled = true;
}

Expand Down
61 changes: 61 additions & 0 deletions packages/axes/test/unit/inputType/PanInput.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,67 @@ describe("PanInput", () => {
cleanup();
});

describe("preventClickOnDrag", () => {
it("should prevent click event when preventClickOnDrag is true", (done) => {
// Given
const click = sinon.spy();
el.addEventListener("click", click);
input = new PanInput(el, {
inputType: ["touch", "mouse"],
preventClickOnDrag: true,
});
inst.connect(["x", "y"], input);

// When
Simulator.gestures.pan(
el,
{
pos: [0, 0],
deltaX: 50,
deltaY: 50,
duration: 200,
easing: "linear",
},
() => {
el.click();
// Then
expect(click.called).to.be.false;
done();
}
);
});

it("shouldn't bother click event when input is disabled", (done) => {
// Given
const click = sinon.spy();
el.addEventListener("click", click);
input = new PanInput(el, {
inputType: ["touch", "mouse"],
preventClickOnDrag: true,
});
inst.connect(["x", "y"], input);
input.disable();

// When
Simulator.gestures.pan(
el,
{
pos: [0, 0],
deltaX: 50,
deltaY: 50,
duration: 200,
easing: "linear",
},
() => {
el.click();
// Then
expect(click.called).to.be.true;
done();
}
);
});
});

describe("inputButton", () => {
["left", "middle", "right"].forEach((button) => {
it("should check only the button set in inputButton is available", (done) => {
Expand Down