From c6a5507daf640e5dc0412a668f4d851d724c1f27 Mon Sep 17 00:00:00 2001 From: Brian Krabach Date: Sat, 28 Mar 2026 16:38:54 -0700 Subject: [PATCH] =?UTF-8?q?revert:=20remove=20JS=20touch=20scroll=20handle?= =?UTF-8?q?rs=20=E2=80=94=20back=20to=20mouse-only=20baseline?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removing terminal WheelEvent dispatch and sidebar scrollTop handlers. CSS fixes retained (touch-action:pan-y, overscroll-behavior:contain, flex-shrink:0). Testing mouse-only baseline across all devices before deciding on the right mobile scroll approach. --- muxplex/frontend/app.js | 27 ------------- muxplex/frontend/terminal.js | 49 ------------------------ muxplex/frontend/tests/test_app.mjs | 10 ----- muxplex/frontend/tests/test_terminal.mjs | 37 ------------------ 4 files changed, 123 deletions(-) diff --git a/muxplex/frontend/app.js b/muxplex/frontend/app.js index c36b116..4999411 100644 --- a/muxplex/frontend/app.js +++ b/muxplex/frontend/app.js @@ -1051,33 +1051,6 @@ document.addEventListener('DOMContentLoaded', () => { document.addEventListener('click', trackInteraction); document.addEventListener('touchstart', trackInteraction); - // --------------------------------------------------------------------------- - // Sidebar touch scroll — manual scrollTop manipulation bypasses the CSS - // touch-scroll path that fails when position:fixed + overflow:hidden is on - // the parent (.session-sidebar) in mobile overlay mode. - // --------------------------------------------------------------------------- - ;(function initSidebarTouchScroll() { - var list = document.querySelector('.sidebar-list'); - if (!list || typeof list.addEventListener !== 'function') return; - - var _lastY = 0; - - list.addEventListener('touchstart', function (e) { - _lastY = e.touches[0].clientY; - }, { passive: true }); - - list.addEventListener('touchmove', function (e) { - e.preventDefault(); // block page scroll - var y = e.touches[0].clientY; - list.scrollTop += _lastY - y; // direct scrollTop — always works - _lastY = y; - }, { passive: false }); // passive:false required for preventDefault - - list.addEventListener('touchend', function () { - _lastY = 0; - }, { passive: true }); - })(); - restoreState() .then(() => { startPolling(); diff --git a/muxplex/frontend/terminal.js b/muxplex/frontend/terminal.js index 0a766c8..867d5ee 100644 --- a/muxplex/frontend/terminal.js +++ b/muxplex/frontend/terminal.js @@ -247,53 +247,4 @@ function closeTerminal() { window._openTerminal = openTerminal; window._closeTerminal = closeTerminal; -// --------------------------------------------------------------------------- -// Touch scroll — accumulates delta and dispatches fixed-size WheelEvents. -// Fixed deltaY=120 per SCROLL_PX pixels ensures consistent scroll steps -// regardless of how Android Chrome batches touchmove events. Each WheelEvent -// travels through xterm.js mouse reporting to tmux (the "inside" path). -// Mouse wheel (wheel event) and iOS/desktop are unaffected. -// --------------------------------------------------------------------------- -;(function initTerminalTouchScroll() { - var container = document.getElementById('terminal-container'); - if (!container || typeof container.addEventListener !== 'function') return; - var _touchLastY = 0; - var _accumulated = 0; // running pixel debt between scroll events - var SCROLL_PX = 20; // pixels of touch movement = one scroll click - - container.addEventListener('touchstart', function (e) { - _touchLastY = e.touches[0].clientY; - _accumulated = 0; // reset on new touch - }, { passive: true }); - - container.addEventListener('touchmove', function (e) { - if (!_term) return; - e.preventDefault(); - - var y = e.touches[0].clientY; - _accumulated += _touchLastY - y; // accumulate — positive = swipe up - _touchLastY = y; - - var viewport = container.querySelector('.xterm-viewport'); - if (!viewport) return; - - // Fire one standard-sized click (deltaY=120) per SCROLL_PX pixels. - // Fixed deltaY makes every step identical regardless of Android event batching. - while (Math.abs(_accumulated) >= SCROLL_PX) { - var dir = _accumulated > 0 ? 1 : -1; - viewport.dispatchEvent(new WheelEvent('wheel', { - deltaY: dir * 120, // 120 = one standard wheel click, consistent - deltaMode: WheelEvent.DOM_DELTA_PIXEL, - bubbles: true, - cancelable: true, - })); - _accumulated -= dir * SCROLL_PX; - } - }, { passive: false }); // passive: false required to allow e.preventDefault() - - container.addEventListener('touchend', function () { - _touchLastY = 0; - _accumulated = 0; // discard sub-threshold remainder on lift - }, { passive: true }); -})(); diff --git a/muxplex/frontend/tests/test_app.mjs b/muxplex/frontend/tests/test_app.mjs index b34e203..dc200d1 100644 --- a/muxplex/frontend/tests/test_app.mjs +++ b/muxplex/frontend/tests/test_app.mjs @@ -40,7 +40,6 @@ Object.defineProperty(globalThis, 'navigator', { configurable: true, }); -import fs from 'node:fs'; import { createRequire } from 'node:module'; import { test } from 'node:test'; import assert from 'node:assert/strict'; @@ -2026,13 +2025,4 @@ test('bindSidebarClickAway registers click listener on terminal-container', () = globalThis.document.getElementById = origGetById; }); -// --- Sidebar touch scroll --- -test('app.js has sidebar touch scroll handler that uses scrollTop', () => { - const source = fs.readFileSync( - new URL('../app.js', import.meta.url), 'utf8' - ); - assert.ok(source.includes('sidebar-list'), 'must target .sidebar-list'); - assert.ok(source.includes('scrollTop'), 'must use scrollTop (not scrollLines or WheelEvent)'); - assert.ok(source.includes('passive: false'), 'touchmove must be non-passive to allow preventDefault'); -}); diff --git a/muxplex/frontend/tests/test_terminal.mjs b/muxplex/frontend/tests/test_terminal.mjs index b227fb5..3e73efc 100644 --- a/muxplex/frontend/tests/test_terminal.mjs +++ b/muxplex/frontend/tests/test_terminal.mjs @@ -5,7 +5,6 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import { fileURLToPath } from 'node:url'; import { dirname, join } from 'node:path'; -import fs from 'node:fs'; const __filename = fileURLToPath(import.meta.url); const __dirname = dirname(__filename); @@ -719,40 +718,4 @@ test('terminal is auto-focused when WebSocket opens', () => { '_term.focus() should be called exactly once when the WebSocket open event fires'); }); -// --- Touch scroll source-inspection tests --- -const terminalSrc = fs.readFileSync( - new URL('../terminal.js', import.meta.url), 'utf8' -); - -test('terminal.js touchmove dispatches WheelEvent to xterm viewport', () => { - assert.ok(terminalSrc.includes('touchmove'), 'touchmove listener must be present'); - assert.ok(terminalSrc.includes('e.preventDefault'), 'must call preventDefault to prevent page scroll'); - assert.ok(terminalSrc.includes('WheelEvent'), 'must dispatch WheelEvent (not call scrollLines)'); - assert.ok(terminalSrc.includes('.xterm-viewport'), 'must target xterm viewport element'); - assert.ok(!terminalSrc.includes('scrollLines'), 'must NOT call scrollLines — that only moves local scrollback, not the PTY'); - assert.ok(terminalSrc.includes('passive: false'), 'touchmove must be non-passive to allow preventDefault'); -}); - -test('terminal.js touchstart and touchend are passive', () => { - // touchstart and touchend should be passive: true for performance - // (they don't need preventDefault) - const touchstartPassive = terminalSrc.match(/touchstart[\s\S]*?passive:\s*(true|false)/); - const touchendPassive = terminalSrc.match(/touchend[\s\S]*?passive:\s*(true|false)/); - assert.ok(touchstartPassive, 'touchstart listener must declare passive'); - assert.ok(touchendPassive, 'touchend listener must declare passive'); -}); - -test('terminal.js touchmove uses accumulation for consistent scroll steps', () => { - const source = fs.readFileSync( - new URL('../terminal.js', import.meta.url), 'utf8' - ); - assert.ok(source.includes('touchmove'), 'touchmove listener must be present'); - assert.ok(source.includes('e.preventDefault'), 'must call preventDefault'); - assert.ok(source.includes('WheelEvent'), 'must dispatch WheelEvent'); - assert.ok(source.includes('.xterm-viewport'), 'must target xterm viewport'); - assert.ok(source.includes('_accumulated'), 'must accumulate delta between events'); - assert.ok(source.includes('SCROLL_PX'), 'must use a threshold constant'); - assert.ok(!source.includes('deltaY * 3'), 'must NOT use variable deltaY scaling (causes jumpy scroll)'); - assert.ok(source.includes('passive: false'), 'touchmove must be non-passive'); -});