From f2749437e5007a76fdb9ea92b658cce4c52fc294 Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Wed, 14 Feb 2024 19:20:59 +0100 Subject: [PATCH] undo debounce/throttle changes (#3108) Relates to: #3097 Relates to: #3033 Relates to: #2318 Relates to: #3076 This reverts the changes made in #3097 and #3033. It adds one small change to the original behavior to clear the throttle timeout on blur. For cases where the user wants to get the latest value on blur when throttle is used, adding a `phx-blur={JS.dispatch("change")}` is the way to go: ``` ``` --- assets/js/phoenix_live_view/dom.js | 34 ++++------- assets/test/debounce_test.js | 95 +++++++++++++++++++++++------- 2 files changed, 84 insertions(+), 45 deletions(-) diff --git a/assets/js/phoenix_live_view/dom.js b/assets/js/phoenix_live_view/dom.js index 814724c578..f206bb61d3 100644 --- a/assets/js/phoenix_live_view/dom.js +++ b/assets/js/phoenix_live_view/dom.js @@ -221,23 +221,7 @@ let DOM = { default: let timeout = parseInt(value) - let trigger = (blur) => { - if(blur){ - // if the input is blurred, we need to cancel the next throttle timeout - // therefore we store the timer id in the THROTTLED private attribute - if(throttle && this.private(el, THROTTLED)){ - clearTimeout(this.private(el, THROTTLED)) - this.deletePrivate(el, THROTTLED) - } - // on debounce we just trigger the callback - return callback() - } - // no blur, remove the throttle attribute if we are in throttle mode - if(throttle) this.deletePrivate(el, THROTTLED) - // always call the callback to ensure that the latest event is processed, - // even when throttle is active - callback() - } + let trigger = () => throttle ? this.deletePrivate(el, THROTTLED) : callback() let currentCycle = this.incCycle(el, DEBOUNCE_TRIGGER, trigger) if(isNaN(timeout)){ return logError(`invalid throttle/debounce value: ${value}`) } if(throttle){ @@ -252,10 +236,6 @@ let DOM = { return false } else { callback() - // store the throttle timer id in the THROTTLED private attribute, - // so that we can cancel it if the input is blurred - // otherwise, when new events happen after blur, but before the old - // timeout is triggered, we would actually trigger the callback multiple times const t = setTimeout(() => { if(asyncFilter()){ this.triggerCycle(el, DEBOUNCE_TRIGGER) } }, timeout) @@ -278,17 +258,23 @@ let DOM = { }) } if(this.once(el, "bind-debounce")){ - el.addEventListener("blur", () => this.triggerCycle(el, DEBOUNCE_TRIGGER, null, [true])) + el.addEventListener("blur", () => { + // because we trigger the callback here, + // we also clear the throttle timeout to prevent the callback + // from being called again after the timeout fires + clearTimeout(this.private(el, THROTTLED)) + this.triggerCycle(el, DEBOUNCE_TRIGGER) + }) } } }, - triggerCycle(el, key, currentCycle, params=[]){ + triggerCycle(el, key, currentCycle){ let [cycle, trigger] = this.private(el, key) if(!currentCycle){ currentCycle = cycle } if(currentCycle === cycle){ this.incCycle(el, key) - trigger(...params) + trigger() } }, diff --git a/assets/test/debounce_test.js b/assets/test/debounce_test.js index d2758c251c..813a3e2b0e 100644 --- a/assets/test/debounce_test.js +++ b/assets/test/debounce_test.js @@ -20,6 +20,14 @@ let container = () => { +
` @@ -169,21 +177,16 @@ describe("throttle", function (){ DOM.dispatchEvent(el, "click") expect(calls).toBe(1) expect(el.innerText).toBe("now:1") - after(100, () => { + after(250, () => { expect(calls).toBe(1) - // now wait another 150ms (after 100ms, so 200 total we expect 2 events) - after(150, () => { + expect(el.innerText).toBe("now:1") + DOM.dispatchEvent(el, "click") + DOM.dispatchEvent(el, "click") + DOM.dispatchEvent(el, "click") + after(250, () => { expect(calls).toBe(2) expect(el.innerText).toBe("now:2") - DOM.dispatchEvent(el, "click") - DOM.dispatchEvent(el, "click") - DOM.dispatchEvent(el, "click") - // the first and last event are processed - after(250, () => { - expect(calls).toBe(4) - expect(el.innerText).toBe("now:4") - done() - }) + done() }) }) }) @@ -240,6 +243,59 @@ describe("throttle", function (){ done() }) }) + + test("triggers only once when there is only one event", done => { + let calls = 0 + let el = container().querySelector("#throttle-200") + + el.addEventListener("click", e => { + DOM.debounce(el, e, "phx-debounce", 100, "phx-throttle", 200, () => true, () => { + calls++ + el.innerText = `now:${calls}` + }) + }) + DOM.dispatchEvent(el, "click") + expect(calls).toBe(1) + expect(el.innerText).toBe("now:1") + after(250, () => { + expect(calls).toBe(1) + done() + }) + }) + + test("sends value on blur when phx-blur dispatches change", done => { + let calls = 0 + let el = container().querySelector("input[name=throttle-range-with-blur]") + + el.addEventListener("input", e => { + DOM.debounce(el, e, "phx-debounce", 100, "phx-throttle", 200, () => true, () => { + calls++ + el.innerText = `now:${calls}` + }) + }) + el.value = 500 + DOM.dispatchEvent(el, "input") + // these will be throttled + for(let i = 0; i < 100; i++){ + el.value = i + DOM.dispatchEvent(el, "input") + } + expect(calls).toBe(1) + expect(el.innerText).toBe("now:1") + // when using phx-blur={JS.dispatch("change")} we would trigger another + // input event immediately after the blur + // therefore starting a new throttle cycle + DOM.dispatchEvent(el, "blur") + // simulate phx-blur + DOM.dispatchEvent(el, "input") + expect(calls).toBe(2) + expect(el.innerText).toBe("now:2") + after(250, () => { + expect(calls).toBe(2) + expect(el.innerText).toBe("now:2") + done() + }) + }) }) @@ -260,16 +316,13 @@ describe("throttle keydown", function (){ el.dispatchEvent(pressA) expect(keyPresses["a"]).toBe(1) - after(100, () => { + after(250, () => { expect(keyPresses["a"]).toBe(1) - after(150, () => { - expect(keyPresses["a"]).toBe(2) - el.dispatchEvent(pressA) - el.dispatchEvent(pressA) - el.dispatchEvent(pressA) - expect(keyPresses["a"]).toBe(3) - done() - }) + el.dispatchEvent(pressA) + el.dispatchEvent(pressA) + el.dispatchEvent(pressA) + expect(keyPresses["a"]).toBe(2) + done() }) })