From bfd4f94f5f5e5fa0930808e7d2fe034e0d05f264 Mon Sep 17 00:00:00 2001 From: Vitalii Bondar Date: Mon, 8 Jun 2026 01:46:17 +0200 Subject: [PATCH 1/2] Fix comment-typing freeze: scope drag-strum keydown, stop listener leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The drag-and-strum controller adds a global keydown listener to play an instrument while dragging cards. Two issues made typing in the comment editor janky (reported on Safari, #2926): 1. Listener leak: connect added the handler via handleKeyDown.bind(this) and disconnect tried to remove it with a *different* .bind(this) reference, so the listener was never removed. Every Turbo reconnect added another permanent global keydown listener — they accumulate over a session and all fire on every keystroke. Bind once and reuse the reference so removeEventListener works. 2. Fires while typing: the handler ran on every document keydown, including inside the comment editor. Holding Shift (capital letters) repeatedly hit #preloadAudioFiles, constructing new Audio() in a loop on each keystroke. Ignore keydown originating from text inputs / contenteditable / the lexxy editor — the strum is only meaningful during a drag. Fixes #2926 Co-Authored-By: Claude Opus 4.8 --- .../controllers/drag_and_strum_controller.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/app/javascript/controllers/drag_and_strum_controller.js b/app/javascript/controllers/drag_and_strum_controller.js index 8946f360a5..e28b4d78db 100644 --- a/app/javascript/controllers/drag_and_strum_controller.js +++ b/app/javascript/controllers/drag_and_strum_controller.js @@ -49,14 +49,17 @@ export default class extends Controller { connect() { this.instrumentIndex = 0 this.preloadedAudioFiles = [] - document.addEventListener("keydown", this.handleKeyDown.bind(this)); + this.handleKeyDown = this.handleKeyDown.bind(this) + document.addEventListener("keydown", this.handleKeyDown); } disconnect() { - document.removeEventListener("keydown", this.handleKeyDown.bind(this)); + document.removeEventListener("keydown", this.handleKeyDown); } handleKeyDown(event) { + if (this.#isEditingText(event.target)) { return } + if (event.shiftKey) { this.instrumentIndex = this.#getInstrumentIndex(event) @@ -66,6 +69,13 @@ export default class extends Controller { } } + #isEditingText(element) { + if (!element) { return false } + if (element.isContentEditable) { return true } + + return element.closest?.("input, textarea, select, lexxy-editor") != null + } + dragEnter(event) { event.preventDefault() const container = this.#containerContaining(event.target) From 70b17a7b93d6ec30a25c0ca7d8dfeba5934605bc Mon Sep 17 00:00:00 2001 From: Vitalii Bondar Date: Mon, 8 Jun 2026 01:51:37 +0200 Subject: [PATCH 2/2] Narrow editing-detection to text inputs; fall back to activeElement Address review feedback on #isEditingText: - Only treat text-entry inputs as editing (skip checkbox/radio/range/file/ button/etc.) so the strum isn't suppressed when a non-text control has focus during a drag. - Fall back to document.activeElement when the event has no target. Co-Authored-By: Claude Opus 4.8 --- .../controllers/drag_and_strum_controller.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/app/javascript/controllers/drag_and_strum_controller.js b/app/javascript/controllers/drag_and_strum_controller.js index e28b4d78db..fcad6bf460 100644 --- a/app/javascript/controllers/drag_and_strum_controller.js +++ b/app/javascript/controllers/drag_and_strum_controller.js @@ -69,11 +69,17 @@ export default class extends Controller { } } - #isEditingText(element) { + #isEditingText(target) { + const element = target ?? document.activeElement if (!element) { return false } if (element.isContentEditable) { return true } + if (element.closest?.("textarea, select, lexxy-editor")) { return true } - return element.closest?.("input, textarea, select, lexxy-editor") != null + const input = element.closest?.("input") + if (!input) { return false } + + const NON_TEXT_INPUT_TYPES = [ "button", "submit", "reset", "checkbox", "radio", "file", "image", "range", "color" ] + return !NON_TEXT_INPUT_TYPES.includes((input.type || "text").toLowerCase()) } dragEnter(event) {