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.

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

Ensure that events are always sent on blur, but this time correctly #3097

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 26 additions & 9 deletions assets/js/phoenix_live_view/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,23 @@ let DOM = {

default:
let timeout = parseInt(value)
let trigger = () => throttle ? this.deletePrivate(el, THROTTLED) : callback()
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 currentCycle = this.incCycle(el, DEBOUNCE_TRIGGER, trigger)
if(isNaN(timeout)){ return logError(`invalid throttle/debounce value: ${value}`) }
if(throttle){
Expand All @@ -236,10 +252,14 @@ let DOM = {
return false
} else {
callback()
this.putPrivate(el, THROTTLED, true)
setTimeout(() => {
// 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)
this.putPrivate(el, THROTTLED, t)
}
} else {
setTimeout(() => {
Expand All @@ -258,20 +278,17 @@ let DOM = {
})
}
if(this.once(el, "bind-debounce")){
el.addEventListener("blur", () => {
// always trigger callback on blur
callback()
})
el.addEventListener("blur", () => this.triggerCycle(el, DEBOUNCE_TRIGGER, null, [true]))
}
}
},

triggerCycle(el, key, currentCycle){
triggerCycle(el, key, currentCycle, params=[]){
let [cycle, trigger] = this.private(el, key)
if(!currentCycle){ currentCycle = cycle }
if(currentCycle === cycle){
this.incCycle(el, key)
trigger()
trigger(...params)
}
},

Expand Down
34 changes: 21 additions & 13 deletions assets/test/debounce_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,16 +169,21 @@ describe("throttle", function (){
DOM.dispatchEvent(el, "click")
expect(calls).toBe(1)
expect(el.innerText).toBe("now:1")
after(250, () => {
after(100, () => {
expect(calls).toBe(1)
expect(el.innerText).toBe("now:1")
DOM.dispatchEvent(el, "click")
DOM.dispatchEvent(el, "click")
DOM.dispatchEvent(el, "click")
after(250, () => {
// now wait another 150ms (after 100ms, so 200 total we expect 2 events)
after(150, () => {
expect(calls).toBe(2)
expect(el.innerText).toBe("now:2")
done()
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()
})
})
})
})
Expand Down Expand Up @@ -255,13 +260,16 @@ describe("throttle keydown", function (){
el.dispatchEvent(pressA)

expect(keyPresses["a"]).toBe(1)
after(250, () => {
after(100, () => {
expect(keyPresses["a"]).toBe(1)
el.dispatchEvent(pressA)
el.dispatchEvent(pressA)
el.dispatchEvent(pressA)
expect(keyPresses["a"]).toBe(2)
done()
after(150, () => {
expect(keyPresses["a"]).toBe(2)
el.dispatchEvent(pressA)
el.dispatchEvent(pressA)
el.dispatchEvent(pressA)
expect(keyPresses["a"]).toBe(3)
done()
})
})
})

Expand Down
Loading