From defd63d20b98f6a3a45fa4cff8ae92a3867aeac5 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Tue, 2 Apr 2024 11:11:32 +0200 Subject: [PATCH] FIX: allows modals to disable swipe to close (#26460) Prior to this fix the `swipe` modifier could not be disabled and we were not using the `this.dimissable` property to apply/not apply it. This commit adds a new `enabled` param to the `swipe` modifier, which is used in modals with the value of `this.dismissable`. Note this commit also adds tests for this modifier. --- .../discourse/app/components/d-modal.gjs | 4 +- .../discourse/app/modifiers/swipe.js | 25 ++++-- .../discourse/tests/modifiers/swipe-test.gjs | 85 +++++++++++++++++++ 3 files changed, 104 insertions(+), 10 deletions(-) create mode 100644 app/assets/javascripts/discourse/tests/modifiers/swipe-test.gjs diff --git a/app/assets/javascripts/discourse/app/components/d-modal.gjs b/app/assets/javascripts/discourse/app/components/d-modal.gjs index dfd5cd408e2..7546b8190d0 100644 --- a/app/assets/javascripts/discourse/app/components/d-modal.gjs +++ b/app/assets/javascripts/discourse/app/components/d-modal.gjs @@ -302,9 +302,9 @@ export default class DModal extends Component {
{{yield to="headerAboveTitle"}} @@ -396,9 +396,9 @@ export default class DModal extends Component {
diff --git a/app/assets/javascripts/discourse/app/modifiers/swipe.js b/app/assets/javascripts/discourse/app/modifiers/swipe.js index 73eb00097aa..7ce242ed5b0 100644 --- a/app/assets/javascripts/discourse/app/modifiers/swipe.js +++ b/app/assets/javascripts/discourse/app/modifiers/swipe.js @@ -26,10 +26,11 @@ export default class SwipeModifier extends Modifier { * @type {Element} */ element; + enabled = true; constructor(owner, args) { super(owner, args); - registerDestructor(this, (instance) => instance.cleanup(instance.element)); + registerDestructor(this, (instance) => instance.cleanup()); } /** @@ -41,8 +42,14 @@ export default class SwipeModifier extends Modifier { * @param {Function} options.didStartSwipe Callback to be executed when a swipe starts. * @param {Function} options.didSwipe Callback to be executed when a swipe moves. * @param {Function} options.didEndSwipe Callback to be executed when a swipe ends. + * @param {Boolean} options.enabled Enable or disable the swipe modifier. */ - modify(element, _, { didStartSwipe, didSwipe, didEndSwipe }) { + modify(element, _, { didStartSwipe, didSwipe, didEndSwipe, enabled }) { + if (enabled === false) { + this.enabled = enabled; + return; + } + this.element = element; this.didSwipeCallback = didSwipe; this.didStartSwipeCallback = didStartSwipe; @@ -121,12 +128,14 @@ export default class SwipeModifier extends Modifier { /** * Cleans up the modifier by removing event listeners from the element. - * - * @param {Element} element The DOM element from which to remove event listeners. */ - cleanup(element) { - element.removeEventListener("touchstart", this.handleTouchStart); - element.removeEventListener("touchmove", this.handleTouchMove); - element.removeEventListener("touchend", this.handleTouchEnd); + cleanup() { + if (!this.enabled) { + return; + } + + this.element?.removeEventListener("touchstart", this.handleTouchStart); + this.element?.removeEventListener("touchmove", this.handleTouchMove); + this.element?.removeEventListener("touchend", this.handleTouchEnd); } } diff --git a/app/assets/javascripts/discourse/tests/modifiers/swipe-test.gjs b/app/assets/javascripts/discourse/tests/modifiers/swipe-test.gjs new file mode 100644 index 00000000000..e512af801c5 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/modifiers/swipe-test.gjs @@ -0,0 +1,85 @@ +import { render, triggerEvent } from "@ember/test-helpers"; +import { setupRenderingTest } from "ember-qunit"; +import hbs from "htmlbars-inline-precompile"; +import { module, test } from "qunit"; + +module("Integration | Modifier | swipe", function (hooks) { + setupRenderingTest(hooks); + + test("it calls didStartSwipe on touchstart", async function (assert) { + this.didStartSwipe = (state) => { + assert.ok(state, "didStartSwipe called with state"); + }; + + await render(hbs`
`); + + await triggerEvent("div", "touchstart", { + touches: [{ clientX: 0, clientY: 0 }], + }); + }); + + test("it calls didSwipe on touchmove", async function (assert) { + this.didSwipe = (state) => { + assert.ok(state, "didSwipe called with state"); + }; + + await render(hbs`
`); + + await triggerEvent("div", "touchstart", { + touches: [{ clientX: 0, clientY: 0 }], + changedTouches: [{ clientX: 0, clientY: 0 }], + }); + + await triggerEvent("div", "touchmove", { + touches: [{ clientX: 5, clientY: 5 }], + }); + }); + + test("it calls didEndSwipe on touchend", async function (assert) { + this.didEndSwipe = (state) => { + assert.ok(state, "didEndSwipe called with state"); + }; + + await render(hbs`
`); + + await triggerEvent("div", "touchstart", { + touches: [{ clientX: 0, clientY: 0 }], + changedTouches: [{ clientX: 0, clientY: 0 }], + }); + + await triggerEvent("div", "touchmove", { + touches: [{ clientX: 10, clientY: 0 }], + changedTouches: [{ clientX: 10, clientY: 0 }], + }); + + await triggerEvent("div", "touchend", { + changedTouches: [{ clientX: 10, clientY: 0 }], + }); + }); + + test("it does not trigger when disabled", async function (assert) { + let calls = 0; + + this.didStartSwipe = () => { + calls++; + }; + + this.set("isEnabled", false); + + await render( + hbs`
` + ); + + await triggerEvent("div", "touchstart", { + touches: [{ clientX: 0, clientY: 0 }], + }); + + this.set("isEnabled", true); + + await triggerEvent("div", "touchstart", { + touches: [{ clientX: 0, clientY: 0 }], + }); + + assert.deepEqual(calls, 1, "didStartSwipe should be called once"); + }); +});