Fix touch listeners stacking on canvas when horizontal scrolling is toggled

[Abhishek-Sonje]

Description

_setupBlocksContainerEvents() in js/activity.js attaches touchstart, touchmove, touchend, and wheel listeners to myCanvas. This function reruns every time the user toggles "Enable/Disable horizontal scrolling" in the Advanced mode toolbar (via setScroller, see toolbar-ui.js:1373-1378), not just once at startup.

The wheel listener already guards against this, it stores a handler reference and removes the old one before adding a new one on each rerun. The touchstart, touchmove, and touchend listeners didn't have that guard. They were attached with raw myCanvas.addEventListener(...) calls with no stored reference and no removal, so every toggle added a fresh set of touch handlers on top of the old ones. The old ones never got removed and kept firing alongside the new ones, each one holding the whole Activity instance alive through its closure.

This PR applies the same store-reference-and-remove-before-add pattern the wheel handler already uses to the three touch handlers, and routes all of them through the class's managed this.addEventListener/this.removeEventListener so they also get swept up automatically by cleanupEventListeners().

Related Issue

This PR fixes #7993

PR Category

• Bug Fix — Fixes a bug or incorrect behavior

Changes Made

• Wrapped the touchstart handler in a named function (__touchStartHandler), stored on this._touchStartHandler, with a remove-before-add guard, matching the existing wheel handler pattern.
• Did the same for touchmove (__touchMoveHandler / this._touchMoveHandler) and touchend (__touchEndHandler / this._touchEndHandler).
• Switched all three from raw myCanvas.addEventListener(...) to the class's managed this.addEventListener(...), so they're tracked in this._listeners and cleaned up by cleanupEventListeners() like the rest of the managed listeners.
• No behavioral logic inside the handlers themselves was changed, pinch-zoom, two-finger scroll, and pinch-distance tracking work exactly as before.

Testing Performed

• Added a temporary console.count("_setupBlocksContainerEvents") at the top of the function and confirmed it climbs on each horizontal-scroll toggle, both before and after this fix (confirming the function itself is still expected to rerun, that part isn't the bug).
• Before the fix: checked getEventListeners(document.getElementById("myCanvas")) in Chrome DevTools after several toggles and confirmed touchstart/touchmove/touchend entries grew by one per toggle while wheel stayed at one.
• After the fix: repeated the same steps and confirmed all four (touchstart, touchmove, touchend, wheel) stay at exactly one listener entry regardless of how many times horizontal scrolling is toggled.
• Manually tested pinch-zoom and two-finger scroll on a touch device after multiple toggles to confirm no duplicated/doubled behavior and no regressions.
• Verified node --check js/activity.js passes.

Checklist

• I have tested these changes locally and they work as expected.
• I have added/updated tests that prove the effectiveness of these changes.
• I have updated the documentation to reflect these changes, if applicable.
• I have followed the project's coding style guidelines.
• I have run npm run lint and npx prettier --check . with no errors.
• I have addressed the code review feedback from the previous submission, if applicable.
• I have enabled "Allow edits from maintainers" (required for auto-rebase; this only affects the PR branch, not your fork).

Additional Notes for Reviewers

The fix mirrors the exact pattern already present in this same function for the wheel listener (see the lines right below the touch handlers), so it's intentionally not introducing a new convention, just applying the existing one consistently to the three handlers that were missing it. Happy to add a regression test around _setupBlocksContainerEvents if there's an existing pattern in the test suite for asserting listener counts on canvas elements, pointers welcome.

[Copilot]

Pull request overview

Fixes a bug in Activity._setupBlocksContainerEvents() where re-running the setup (triggered by toggling horizontal scrolling) could stack multiple canvas touch listeners, causing duplicated gesture handling and a memory leak.

Changes:

• Convert touchstart, touchmove, and touchend canvas handlers to stored instance references so prior handlers can be removed before re-adding.
• Route touch handlers through Activity’s managed addEventListener/removeEventListener so they’re tracked and cleaned up via cleanupEventListeners().
• Keep the existing wheel handler’s remove-before-add pattern intact while aligning touch handlers with it.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.