aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorChristophe Besson <cbesson@gmail.com>2026-09-16 16:26:16 +0200
committerChristophe Besson <cbesson@gmail.com>2026-09-16 16:26:16 +0200
commit17ddd4de9087a98c87bec28a6b773572b1c58b50 (patch)
tree8ea8ee7cb57ecd8b0b428b089395c449621c102b
parent2916aa376009283305a7acec4aafa3c96544499e (diff)
downloadmeshbay-17ddd4de9087a98c87bec28a6b773572b1c58b50.tar.gz
menu: do not close on the panel's own scrolling
The dismiss-on-scroll listener is on the capture phase, because `scroll` does not bubble — so it also heard the menu scrolling itself, and a long tracklist closed the moment it was wheeled. Filter on the event's origin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
-rw-r--r--docs/playlists.md15
-rw-r--r--packages/meshbay-hub/src/meshbay_hub/static/menu.js17
-rwxr-xr-xpackages/meshbay-hub/tests/harness/menu_scroll_probe.py216
-rw-r--r--packages/meshbay-hub/tests/test_menu_scroll.py76
4 files changed, 318 insertions, 6 deletions
diff --git a/docs/playlists.md b/docs/playlists.md
index 49b1585..c450815 100644
--- a/docs/playlists.md
+++ b/docs/playlists.md
@@ -808,10 +808,17 @@ The codebase has no context menu and no dropdown outside the account menu in
button** — `icon.js` already has `dots` (`icon.js:54`). Both affordances, both
platforms: the button is visible on hover on a fine pointer and always visible
under `@media (pointer: coarse)`.
-- closes on Escape, on an outside click, and **on scroll**. The last one is not
- optional here: Music's toolbar is a sticky band and the grid scrolls beneath
- it, so a menu that survives a scroll is a menu now anchored to a different
- album than the one it was opened on.
+- closes on Escape, on an outside click, and **on a scroll of the page**. The
+ last one is not optional here: Music's toolbar is a sticky band and the grid
+ scrolls beneath it, so a menu that survives a scroll is a menu now anchored to
+ a different album than the one it was opened on. **Never on a scroll of its
+ own panel** — the panel is `overflow-y: auto` and a tracklist is routinely
+ taller than the window, so the two must be told apart by where the event came
+ from. `scroll` does not bubble, so that listener is on the capture phase,
+ which is also what made it hear the panel's own scrolling: the menu closed the
+ instant it was wheeled or its scrollbar dragged, and no track below the fold
+ could be reached. Measured by `menu_scroll_probe.py`; `playlist_ui_probe.py`
+ cannot see it, because `.click()` scrolls nothing.
- flips its anchor when it would open past the viewport edge, which on a phone
is most of the time.
- submenus **expand in place**, downward, at every width. This is the
diff --git a/packages/meshbay-hub/src/meshbay_hub/static/menu.js b/packages/meshbay-hub/src/meshbay_hub/static/menu.js
index 1c6a96a..16aecd8 100644
--- a/packages/meshbay-hub/src/meshbay_hub/static/menu.js
+++ b/packages/meshbay-hub/src/meshbay_hub/static/menu.js
@@ -155,14 +155,27 @@ function Menu({ x, y, items, onClose }) {
// sticky bands and the grid scrolls underneath them, so a menu that
// survives a scroll is a menu still pointing at the album it was opened
// on while sitting over a completely different one.
+ //
+ // But the panel also scrolls *itself*: a playlist's tracklist expands
+ // inside it and is routinely taller than the window. `scroll` does not
+ // bubble, which is why this listener is on the capture phase — and capture
+ // is equally what makes it hear the panel's own scrolling, on the way
+ // down. So the menu closed the instant it was scrolled, by wheel or by
+ // dragging its own scrollbar, and the track being reached for could not be
+ // reached at all. Ask where the scroll came from, not merely that one
+ // happened.
+ const onScroll = (e) => {
+ if (ref.current && e.target instanceof Node && ref.current.contains(e.target)) return;
+ onClose();
+ };
document.addEventListener('keydown', onKey);
document.addEventListener('mousedown', onDown);
- window.addEventListener('scroll', onClose, true);
+ window.addEventListener('scroll', onScroll, true);
window.addEventListener('resize', onClose);
return () => {
document.removeEventListener('keydown', onKey);
document.removeEventListener('mousedown', onDown);
- window.removeEventListener('scroll', onClose, true);
+ window.removeEventListener('scroll', onScroll, true);
window.removeEventListener('resize', onClose);
};
}, [onClose]);
diff --git a/packages/meshbay-hub/tests/harness/menu_scroll_probe.py b/packages/meshbay-hub/tests/harness/menu_scroll_probe.py
new file mode 100755
index 0000000..809d1b1
--- /dev/null
+++ b/packages/meshbay-hub/tests/harness/menu_scroll_probe.py
@@ -0,0 +1,216 @@
+#!/usr/bin/env python3
+"""
+The pop-up menu, scrolled in a real browser.
+
+A playlist's tracklist expands inside `.ctx-menu`, which is `overflow-y: auto`
+with a `max-height` — so a menu taller than the window is the ordinary case,
+not an edge one, and scrolling it is the only way to reach the track being
+removed.
+
+`playlist_ui_probe.py` cannot see this: it reaches every row with `.click()`,
+which scrolls nothing. Neither can a source-reading test, because the question
+is which listener a scroll event reaches. So this mounts the shipped `Menu`,
+scrolls it the two ways a person can, and reads back whether it survived.
+
+The last case is the one that must keep failing to close: a scroll of the
+*page* has to dismiss the menu, or it ends up pointing at an album that has
+moved out from under it.
+
+ menu_scroll_probe.py
+
+Prints JSON: one entry per case.
+"""
+
+import http.server
+import json
+import socketserver
+import subprocess
+import sys
+import tempfile
+import threading
+import time
+from pathlib import Path
+
+STATIC = Path(__file__).resolve().parents[2] / "src" / "meshbay_hub" / "static"
+PORT = 8757
+RECORDS = []
+socketserver.TCPServer.allow_reuse_address = True
+
+FRAME = r"""<!doctype html><html><head><meta charset=utf-8>
+<link rel="stylesheet" href="/style.css"></head><body>
+<div id="root"></div>
+<!-- Something for the page itself to scroll, for the last case. -->
+<div style="height: 3000px"></div>
+<script type="module">
+import { html, render } from '/vendor/htm-preact.js';
+import { Menu } from '/menu.js';
+
+const LOGS = [];
+addEventListener('error', (e) => LOGS.push('error: ' + (e.message || e)));
+addEventListener('unhandledrejection',
+ (e) => LOGS.push('rejection: ' + ((e.reason && (e.reason.stack || e.reason.message)) || e.reason)));
+
+const sleep = (ms) => new Promise((r) => setTimeout(r, ms));
+const frame = () => new Promise((r) => requestAnimationFrame(() => requestAnimationFrame(r)));
+
+// Sixty rows, which is what a playlist of sixty tracks draws. Fewer than the
+// panel's max-height holds would measure nothing at all, so the count is
+// checked rather than assumed (`overflows` below).
+const ITEMS = [];
+for (let i = 1; i <= 60; i++) {
+ ITEMS.push({ key: 'k' + i, label: 'Track ' + i, hint: 'An Artist',
+ onSelect: () => {} });
+}
+
+const cases = [];
+let closed = false;
+
+(async () => {
+ const fail = (why) => parent.postMessage({ error: why, logs: LOGS.slice(0, 12) }, '*');
+ try {
+ const onClose = () => { closed = true; };
+ render(html`<${Menu} x=${300} y=${40} items=${ITEMS} onClose=${onClose} />`,
+ document.getElementById('root'));
+ await frame();
+
+ const panel = document.querySelector('.ctx-menu');
+ if (!panel) return fail('no menu rendered');
+
+ const overflows = panel.scrollHeight > panel.clientHeight + 1;
+ cases.push({ case: 'the panel is scrollable at all',
+ overflows, scrollHeight: panel.scrollHeight,
+ clientHeight: panel.clientHeight,
+ // Read back, not asserted from the stylesheet: the sidebar
+ // needed exactly this, for exactly this reason.
+ overscrollBehaviorY: getComputedStyle(panel).overscrollBehaviorY });
+ if (!overflows) return fail('the fixture does not overflow — it measures nothing');
+
+ // 1. The wheel. An untrusted WheelEvent performs no default scroll in
+ // Chrome, so what a wheel *does* is reproduced instead: the panel's
+ // scrollTop moves, which is what raises the event any listener sees.
+ panel.scrollTop = 200;
+ await frame();
+ cases.push({ case: 'scrolled inside the menu',
+ stillOpen: !closed && !!document.querySelector('.ctx-menu'),
+ scrollTop: document.querySelector('.ctx-menu')
+ ? document.querySelector('.ctx-menu').scrollTop : null });
+
+ // 2. Pressing the scrollbar. Chrome reports the scrolled element itself as
+ // the target of that mousedown — not a child, and not the document.
+ closed = false;
+ const live = document.querySelector('.ctx-menu');
+ if (live) {
+ live.dispatchEvent(new MouseEvent('mousedown', { bubbles: true, cancelable: true }));
+ await frame();
+ live.scrollTop = 400;
+ await frame();
+ }
+ cases.push({ case: 'pressed the menu scrollbar and dragged',
+ stillOpen: !closed && !!document.querySelector('.ctx-menu'),
+ scrollTop: document.querySelector('.ctx-menu')
+ ? document.querySelector('.ctx-menu').scrollTop : null });
+
+ // 3. A press outside still closes it.
+ closed = false;
+ if (document.querySelector('.ctx-menu')) {
+ document.body.dispatchEvent(
+ new MouseEvent('mousedown', { bubbles: true, cancelable: true }));
+ await frame();
+ }
+ cases.push({ case: 'pressed outside the menu', closed });
+
+ // 4. And so does a scroll of the page. Not optional: the menu is fixed, so
+ // a grid scrolling under it leaves it pointing at the wrong album.
+ render(html`<${Menu} x=${300} y=${40} items=${ITEMS} onClose=${onClose} />`,
+ document.getElementById('root'));
+ await frame();
+ closed = false;
+ window.scrollTo(0, 500);
+ await sleep(80);
+ cases.push({ case: 'scrolled the page underneath', closed });
+
+ parent.postMessage({ cases, logs: LOGS.slice(0, 8) }, '*');
+ } catch (err) {
+ fail(String((err && err.stack) || err));
+ }
+})();
+</script></body></html>"""
+
+PAGE = r"""<!doctype html><html><head><meta charset=utf-8></head>
+<body style="margin:0"><div id="frames"></div><script>
+addEventListener('message', (e) => {
+ fetch('/log', { method: 'POST', body: JSON.stringify(e.data) });
+});
+const f = document.createElement('iframe');
+f.src = '/case';
+f.style.cssText = 'width:1100px;height:800px;border:0;display:block';
+document.getElementById('frames').appendChild(f);
+</script></body></html>"""
+
+
+class H(http.server.BaseHTTPRequestHandler):
+ def log_message(self, *a):
+ pass
+
+ def do_POST(self):
+ length = int(self.headers.get("Content-Length") or 0)
+ if self.path == "/log":
+ RECORDS.append(json.loads(self.rfile.read(length).decode()))
+ else:
+ self.rfile.read(length)
+ self.send_response(204)
+ self.end_headers()
+
+ def _send(self, body: bytes, ctype: str) -> None:
+ self.send_response(200)
+ self.send_header("Content-Type", ctype)
+ self.send_header("Content-Length", str(len(body)))
+ self.end_headers()
+ self.wfile.write(body)
+
+ def do_GET(self):
+ path = self.path.split("?")[0]
+ if path == "/":
+ self._send(PAGE.encode(), "text/html; charset=utf-8")
+ elif path == "/case":
+ self._send(FRAME.encode(), "text/html; charset=utf-8")
+ else:
+ asset = (STATIC / path.lstrip("/")).resolve()
+ if not str(asset).startswith(str(STATIC)) or not asset.is_file():
+ self.send_response(404)
+ self.end_headers()
+ return
+ self._send(asset.read_bytes(),
+ "text/css" if asset.suffix == ".css"
+ else "text/javascript" if asset.suffix == ".js"
+ else "application/octet-stream")
+
+
+def main() -> int:
+ with socketserver.TCPServer(("127.0.0.1", PORT), H) as srv:
+ threading.Thread(target=srv.serve_forever, daemon=True).start()
+ with tempfile.TemporaryDirectory(ignore_cleanup_errors=True) as profile:
+ proc = subprocess.Popen(
+ ["google-chrome", "--headless=new", "--disable-gpu", "--no-sandbox",
+ f"--user-data-dir={profile}", "--window-size=1100,900",
+ f"http://127.0.0.1:{PORT}/"],
+ stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
+ for _ in range(300):
+ if RECORDS:
+ break
+ time.sleep(0.1)
+ proc.terminate()
+ try:
+ proc.wait(timeout=10)
+ except subprocess.TimeoutExpired:
+ proc.kill()
+ proc.wait()
+ if not RECORDS:
+ print(json.dumps({"error": "no measurement"}), file=sys.stderr)
+ return 1
+ print(json.dumps(RECORDS[0], indent=1))
+ return 0
+
+
+if __name__ == "__main__":
+ raise SystemExit(main())
diff --git a/packages/meshbay-hub/tests/test_menu_scroll.py b/packages/meshbay-hub/tests/test_menu_scroll.py
new file mode 100644
index 0000000..5b680a4
--- /dev/null
+++ b/packages/meshbay-hub/tests/test_menu_scroll.py
@@ -0,0 +1,76 @@
+"""
+A pop-up menu taller than the window can be scrolled without dismissing itself.
+
+`.ctx-menu` is `overflow-y: auto` under a `max-height`, and a playlist's
+tracklist expands *inside* it — sixty tracks is three times the panel's height,
+so scrolling is the only way to reach most of them. Both ways of scrolling it
+closed it instead: the wheel, and dragging its own scrollbar. Reported against
+"remove a track", where the menu vanished before a track could be clicked.
+
+`scroll` does not bubble, so the listener that dismisses the menu on a page
+scroll is on the capture phase — and capture is equally what made it hear the
+panel's own scrolling on the way down.
+
+Measured, not read. `playlist_ui_probe.py` drives the same menu and cannot see
+this: it reaches every row with `.click()`, which scrolls nothing at all.
+
+The last two cases are the ones that must keep closing the menu, and they are
+why the fix is a filter on the event's origin rather than a removed listener.
+"""
+
+import json
+import shutil
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+
+HARNESS = Path(__file__).parent / "harness" / "menu_scroll_probe.py"
+
+
+@pytest.fixture(scope="module")
+def cases():
+ if shutil.which("google-chrome") is None:
+ pytest.skip("Chrome is not available")
+ proc = subprocess.run([sys.executable, str(HARNESS)],
+ capture_output=True, text=True, timeout=90)
+ data = json.loads(proc.stdout)
+ assert "error" not in data, f"probe failed: {proc.stdout}{proc.stderr}"
+ return {c["case"]: c for c in data["cases"]}
+
+
+def test_the_fixture_actually_overflows(cases):
+ # A menu that fits measures nothing, and would let every assertion below
+ # pass with the fix removed.
+ c = cases["the panel is scrollable at all"]
+ assert c["overflows"], f"60 rows did not overflow the panel: {c}"
+
+
+def test_the_wheel_scrolls_the_menu_instead_of_closing_it(cases):
+ c = cases["scrolled inside the menu"]
+ assert c["stillOpen"], "the menu closed when it was scrolled"
+ assert c["scrollTop"] == 200, "the menu did not scroll"
+
+
+def test_dragging_the_menus_own_scrollbar_does_not_close_it(cases):
+ c = cases["pressed the menu scrollbar and dragged"]
+ assert c["stillOpen"], "pressing the scrollbar closed the menu"
+ assert c["scrollTop"] == 400, "the drag did not scroll"
+
+
+def test_reaching_the_end_does_not_scroll_the_page_behind(cases):
+ # Chained to the page, that overscroll *is* a page scroll, and a page
+ # scroll closes the menu on purpose — the same symptom by another door.
+ assert cases["the panel is scrollable at all"]["overscrollBehaviorY"] == "contain"
+
+
+def test_a_press_outside_still_closes_it(cases):
+ assert cases["pressed outside the menu"]["closed"]
+
+
+def test_scrolling_the_page_still_closes_it(cases):
+ # Not optional: the menu is `position: fixed` and the media grids scroll
+ # under a sticky toolbar, so a menu that survives leaves itself pointing at
+ # an album that has moved.
+ assert cases["scrolled the page underneath"]["closed"]