aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorChristophe Besson <cbesson@gmail.com>2026-09-16 16:42:00 +0200
committerChristophe Besson <cbesson@gmail.com>2026-09-16 16:42:00 +0200
commite2fedc92a789db4c1fe5d6575108cc43265a10a4 (patch)
treef3b41d258efa609faec0187e7c77832ef601f4b8
parent17ddd4de9087a98c87bec28a6b773572b1c58b50 (diff)
downloadmeshbay-e2fedc92a789db4c1fe5d6575108cc43265a10a4.tar.gz
menu: give the panel the room that is actually below it
`max-height: calc(100vh - 16px)` says how tall the menu may be and nothing about where its bottom lands, so one opened partway down the window ran past it and its last rows scrolled out of reach. Measure from where the panel was placed, and re-measure when a submenu opens. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
-rw-r--r--docs/playlists.md19
-rw-r--r--packages/meshbay-hub/src/meshbay_hub/static/menu.js67
-rw-r--r--packages/meshbay-hub/src/meshbay_hub/static/style.css8
-rwxr-xr-xpackages/meshbay-hub/tests/harness/menu_scroll_probe.py76
-rw-r--r--packages/meshbay-hub/tests/test_menu_scroll.py68
5 files changed, 215 insertions, 23 deletions
diff --git a/docs/playlists.md b/docs/playlists.md
index c450815..badbc99 100644
--- a/docs/playlists.md
+++ b/docs/playlists.md
@@ -819,8 +819,23 @@ The codebase has no context menu and no dropdown outside the account menu in
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.
+- **measures where it can go, and is given exactly that much room.** It opens at
+ the pointer when the list fits below it, flips above when it does not and there
+ is more room there, and in either case its `max-height` is the distance from
+ where it was just placed to the bottom of the window. The stylesheet's
+ `max-height: calc(100vh - 16px)` is only a floor: it says how *tall* the panel
+ may be and nothing about where its *bottom* lands, so a panel opened 300 px
+ down the window ran 300 px past the bottom of it. It scrolled — and its last
+ rows scrolled into a part of itself that is off the screen, which no further
+ scrolling brings back. That was the "the last track cannot be reached" report,
+ and it is the second of two defects with that one symptom; the first, above,
+ hid it. Both are measured by `menu_scroll_probe.py`.
+- **re-measures when a submenu opens.** The height changes inside `MenuItems`'
+ own state, and `loadItems` arrives later still, so `MenuItems` reports upward
+ rather than `Menu` watching for it: a `ResizeObserver` on the panel would be an
+ observer on the very box the callback resizes, and the browser breaks that loop
+ by raising an error. Without the report a two-row menu opened low keeps the
+ 192 px it was measured for and threads sixty tracks through that porthole.
- submenus **expand in place**, downward, at every width. This is the
correction: the proposal wanted a flyout with a sheet on a coarse pointer, and
the account menu's language list (`app.js`) already shows the simpler answer.
diff --git a/packages/meshbay-hub/src/meshbay_hub/static/menu.js b/packages/meshbay-hub/src/meshbay_hub/static/menu.js
index 16aecd8..12ca584 100644
--- a/packages/meshbay-hub/src/meshbay_hub/static/menu.js
+++ b/packages/meshbay-hub/src/meshbay_hub/static/menu.js
@@ -58,11 +58,20 @@ function useMenu() {
return { menu, openAt, close };
}
-function MenuItems({ items, depth, onClose }) {
+function MenuItems({ items, depth, onClose, onResize }) {
const [expanded, setExpanded] = useState(null);
// index -> the items that came back, or 'loading'.
const [loaded, setLoaded] = useState({});
+ // A submenu opening, and a lazily-read one arriving, are the only two things
+ // that change how tall the panel is — and `Menu` can see neither, because
+ // both are state in here. So they are reported rather than watched for: an
+ // observer on the panel would be an observer on the very box the callback
+ // resizes, which is a loop the browser breaks by raising an error.
+ useEffect(() => {
+ if (onResize) onResize();
+ }, [expanded, loaded, onResize]);
+
const toggle = useCallback((i, it) => {
if (expanded === i) { setExpanded(null); return; }
setExpanded(i);
@@ -95,7 +104,8 @@ function MenuItems({ items, depth, onClose }) {
? html`<div class="ctx-menu-empty"
style=${`padding-left: ${28 + depth * 14}px`}><span class="spinner"></span></div>`
: (rows && rows.length
- ? html`<${MenuItems} items=${rows} depth=${depth + 1} onClose=${onClose} />`
+ ? html`<${MenuItems} items=${rows} depth=${depth + 1}
+ onClose=${onClose} onResize=${onResize} />`
: html`<div class="ctx-menu-empty"
style=${`padding-left: ${28 + depth * 14}px`}>${it.empty || ''}</div>`))}
`;
@@ -123,26 +133,46 @@ function MenuItems({ items, depth, onClose }) {
function Menu({ x, y, items, onClose }) {
const ref = useRef(null);
const [place, setPlace] = useState(null);
+ // Bumped by the tree below when it changes height; a dependency of the
+ // measurement, and nothing else reads it.
+ const [grew, setGrew] = useState(0);
+ const remeasure = useCallback(() => setGrew((n) => n + 1), []);
- // Measure, then place. A menu opened near the right or bottom edge has to
- // flip rather than be clipped, and how tall it is depends on its own items —
- // so the number cannot be written down, it has to be read.
+ // Measure, then place. A menu opened near an edge has to flip rather than be
+ // clipped, and how tall it is depends on its own items — so the numbers
+ // cannot be written down, they have to be read.
//
- // This is not the mutate-then-measure loop CLAUDE.md records: the effect
- // writes `place`, which moves the panel, and `place` is not in its
- // dependencies. Position does not change the size being measured, so it
- // cannot wake itself.
+ // This is not the mutate-then-measure loop CLAUDE.md records. The height read
+ // is `scrollHeight`, the content's, which the `max-height` written back does
+ // not move; and `place` is not one of the dependencies. So the effect cannot
+ // wake itself.
useEffect(() => {
const el = ref.current;
if (!el) return;
- const r = el.getBoundingClientRect();
- const left = Math.max(MARGIN, Math.min(x, window.innerWidth - r.width - MARGIN));
- const top = (y + r.height > window.innerHeight - MARGIN)
- ? Math.max(MARGIN, y - r.height)
- : y;
- setPlace({ left, top });
+ // Content plus the panel's own borders — `box-sizing` is `border-box`, so
+ // those count against the `max-height` about to be set.
+ const h = el.scrollHeight + (el.offsetHeight - el.clientHeight);
+ const left = Math.max(MARGIN,
+ Math.min(x, window.innerWidth - el.offsetWidth - MARGIN));
+ const below = window.innerHeight - MARGIN - y;
+ const above = y - MARGIN;
+ // Fits below, or below is simply the roomier side: stay at the pointer and
+ // scroll. Otherwise flip above it.
+ const top = (h <= below || below >= above) ? y : Math.max(MARGIN, y - h);
+ // And the panel may take exactly the room left below where it was just
+ // put. The stylesheet's `max-height` is a floor: it says how tall the panel
+ // may be and nothing about where its bottom lands, so a panel opened 300px
+ // down the window ran 300px past the bottom of it — it scrolled, but its
+ // last rows scrolled into a part of itself that is off the screen, which no
+ // further scrolling brings back. That is "the last track cannot be reached".
+ const maxHeight = Math.max(0, window.innerHeight - MARGIN - top);
+ setPlace((prev) => ((prev && prev.left === left && prev.top === top
+ && prev.maxHeight === maxHeight) ? prev : { left, top, maxHeight }));
+ // `grew` is a submenu reporting that it opened. Without it the panel keeps
+ // the placement measured for its collapsed self, and a two-row menu opened
+ // low threads sixty tracks through the room that was free beneath it.
// eslint-disable-next-line
- }, [x, y, items]);
+ }, [x, y, items, grew]);
useEffect(() => {
const onKey = (e) => { if (e.key === 'Escape') onClose(); };
@@ -183,11 +213,12 @@ function Menu({ x, y, items, onClose }) {
return html`
<div class="ctx-menu" ref=${ref} role="menu"
style=${place
- ? `left: ${place.left}px; top: ${place.top}px`
+ ? `left: ${place.left}px; top: ${place.top}px; max-height: ${place.maxHeight}px`
// Drawn where asked but not yet shown, for the one frame it takes to
// find out how big it is — otherwise it visibly jumps into place.
: `left: ${x}px; top: ${y}px; visibility: hidden`}>
- <${MenuItems} items=${items} depth=${0} onClose=${onClose} />
+ <${MenuItems} items=${items} depth=${0} onClose=${onClose}
+ onResize=${remeasure} />
</div>
`;
}
diff --git a/packages/meshbay-hub/src/meshbay_hub/static/style.css b/packages/meshbay-hub/src/meshbay_hub/static/style.css
index 5ef04b0..2d07fd1 100644
--- a/packages/meshbay-hub/src/meshbay_hub/static/style.css
+++ b/packages/meshbay-hub/src/meshbay_hub/static/style.css
@@ -5040,7 +5040,13 @@ h2 .gn-owner, h3 .gn-owner { font-size: 0.55em; }
max-width: calc(100vw - 16px);
/* A playlist's tracklist expands inside this panel rather than flying out
sideways, so it can be far taller than the window. Same answer the
- account menu's language list already has. */
+ account menu's language list already has.
+
+ A floor, not the real value: this says how tall the panel may be and
+ nothing about where its bottom lands, so a panel opened partway down the
+ window overruns it and its last rows end up in a region of itself that is
+ off-screen. `menu.js` measures where it put the panel and overrides this
+ with the room actually left below that point. */
max-height: calc(100vh - 16px);
overflow-x: hidden;
overflow-y: auto;
diff --git a/packages/meshbay-hub/tests/harness/menu_scroll_probe.py b/packages/meshbay-hub/tests/harness/menu_scroll_probe.py
index 809d1b1..5fb9579 100755
--- a/packages/meshbay-hub/tests/harness/menu_scroll_probe.py
+++ b/packages/meshbay-hub/tests/harness/menu_scroll_probe.py
@@ -119,7 +119,81 @@ let closed = false;
}
cases.push({ case: 'pressed outside the menu', closed });
- // 4. And so does a scroll of the page. Not optional: the menu is fixed, so
+ // 4. Opened partway down the window, the panel must end *inside* it.
+ // Scrolling a panel whose own bottom is off-screen moves its last rows
+ // into a region of itself that no scroll can bring back, which is what
+ // "the last track cannot be reached" was.
+ closed = false;
+ render(html`<${Menu} x=${300} y=${300} items=${ITEMS} onClose=${onClose} />`,
+ document.getElementById('root'));
+ await frame();
+ const low = document.querySelector('.ctx-menu');
+ low.scrollTop = low.scrollHeight;
+ await frame();
+ const lowRows = low.querySelectorAll('.ctx-menu-item');
+ const lowLast = lowRows[lowRows.length - 1];
+ cases.push({ case: 'a long menu opened partway down the window',
+ openedAt: 300, viewport: window.innerHeight,
+ panelTop: Math.round(low.getBoundingClientRect().top),
+ panelBottom: Math.round(low.getBoundingClientRect().bottom),
+ lastRowBottom: Math.round(lowLast.getBoundingClientRect().bottom),
+ lastRowLabel: lowLast.textContent.trim(),
+ overflows: low.scrollHeight > low.clientHeight + 1 });
+
+ // 5. And it must still end inside it after a submenu has expanded, which
+ // is a size change happening in `MenuItems`' own state — the placement
+ // effect's dependencies cannot see it.
+ //
+ // Opened low on purpose. A two-row menu 600px down has 192px of room
+ // beneath it, and that is the height the panel is given; expanding a
+ // sixty-track list into it must re-place the panel rather than thread
+ // the list through the porthole the collapsed one was measured for.
+ const NESTED = [
+ { key: 'load', label: 'Load a playlist', items: ITEMS.slice(0, 3) },
+ { key: 'remove', label: 'Remove a track', items: ITEMS },
+ ];
+ render(html`<${Menu} x=${300} y=${600} items=${NESTED} onClose=${onClose} />`,
+ document.getElementById('root'));
+ await frame();
+ const nested = document.querySelector('.ctx-menu');
+ const collapsedBottom = Math.round(nested.getBoundingClientRect().bottom);
+ const toggle = Array.from(nested.querySelectorAll('.ctx-menu-item'))
+ .find((b) => b.textContent.indexOf('Remove a track') >= 0);
+ if (!toggle) return fail('no submenu row to expand');
+ toggle.click();
+ await frame();
+ await sleep(60);
+ await frame();
+ const open = document.querySelector('.ctx-menu');
+ open.scrollTop = open.scrollHeight;
+ await frame();
+ const openRows = open.querySelectorAll('.ctx-menu-item');
+ const openLast = openRows[openRows.length - 1];
+ cases.push({ case: 'a submenu expanded inside it',
+ openedAt: 600, viewport: window.innerHeight,
+ collapsedBottom,
+ panelHeight: Math.round(open.getBoundingClientRect().height),
+ panelTop: Math.round(open.getBoundingClientRect().top),
+ panelBottom: Math.round(open.getBoundingClientRect().bottom),
+ lastRowBottom: Math.round(openLast.getBoundingClientRect().bottom),
+ lastRowLabel: openLast.textContent.trim(),
+ overflows: open.scrollHeight > open.clientHeight + 1 });
+
+ // 6. A short menu opened at the very bottom still flips above the pointer
+ // rather than being pinned there and scrolled — the condition deciding
+ // that was rewritten, so it is measured rather than assumed.
+ const SHORT = ITEMS.slice(0, 4);
+ render(html`<${Menu} x=${300} y=${780} items=${SHORT} onClose=${onClose} />`,
+ document.getElementById('root'));
+ await frame();
+ const short = document.querySelector('.ctx-menu');
+ cases.push({ case: 'a short menu opened at the bottom edge',
+ openedAt: 780, viewport: window.innerHeight,
+ panelTop: Math.round(short.getBoundingClientRect().top),
+ panelBottom: Math.round(short.getBoundingClientRect().bottom),
+ overflows: short.scrollHeight > short.clientHeight + 1 });
+
+ // 7. 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'));
diff --git a/packages/meshbay-hub/tests/test_menu_scroll.py b/packages/meshbay-hub/tests/test_menu_scroll.py
index 5b680a4..b56a053 100644
--- a/packages/meshbay-hub/tests/test_menu_scroll.py
+++ b/packages/meshbay-hub/tests/test_menu_scroll.py
@@ -1,5 +1,8 @@
"""
-A pop-up menu taller than the window can be scrolled without dismissing itself.
+A pop-up menu taller than the window can be scrolled, and scrolled all the way.
+
+Two defects, one symptom — "the last track cannot be reached" — and they had to
+be fixed in that order, because the first hides the second.
`.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,
@@ -16,6 +19,14 @@ 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.
+
+The second defect is where the panel ends. `max-height: calc(100vh - 16px)`
+says how tall it may be and nothing about where its bottom lands, so a panel
+opened 300px down the window ran 300px past the bottom of it. It scrolled — but
+its last rows scrolled into a part of itself that is off the screen, which no
+further scrolling brings back. `menu.js` measures the room left below where it
+put the panel. The flip case is measured too, because the condition deciding it
+was rewritten in the same breath.
"""
import json
@@ -74,3 +85,58 @@ def test_scrolling_the_page_still_closes_it(cases):
# 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"]
+
+
+def test_a_long_menu_ends_inside_the_window(cases):
+ c = cases["a long menu opened partway down the window"]
+ assert c["overflows"], f"the fixture measures nothing: {c}"
+ assert c["panelBottom"] <= c["viewport"], (
+ f"the panel runs {c['panelBottom'] - c['viewport']}px past the bottom "
+ f"of the window: {c}")
+
+
+def test_the_last_row_of_a_long_menu_can_be_reached(cases):
+ # Scrolled to the end, the last row has to be *visible*. This is the
+ # user-facing sentence, and the one that fails while the panel overruns the
+ # window: the rows are there, scrolled into a region nothing can show.
+ c = cases["a long menu opened partway down the window"]
+ assert c["lastRowLabel"].startswith("Track 60"), c
+ assert c["lastRowBottom"] <= c["viewport"], (
+ f"the last row sits {c['lastRowBottom'] - c['viewport']}px below the "
+ f"window after scrolling to the end: {c}")
+
+
+def test_it_still_opens_at_the_pointer_when_the_room_is_below(cases):
+ # The cure must not be a menu that jumps somewhere else: a list too tall
+ # for any placement stays where it was asked for and scrolls.
+ c = cases["a long menu opened partway down the window"]
+ assert c["panelTop"] == c["openedAt"], c
+
+
+def test_an_expanded_submenu_stays_inside_the_window(cases):
+ c = cases["a submenu expanded inside it"]
+ assert c["panelBottom"] > c["collapsedBottom"], (
+ f"the submenu did not actually expand: {c}")
+ assert c["panelBottom"] <= c["viewport"], c
+ assert c["lastRowLabel"].startswith("Track 60"), c
+ assert c["lastRowBottom"] <= c["viewport"], c
+
+
+def test_an_expanded_submenu_is_re_placed_rather_than_left_in_its_slot(cases):
+ # The size change happens in `MenuItems`' own state, which the placement
+ # effect cannot see on its own — this is what the report upward is for.
+ # Without it the panel keeps the height measured for its collapsed self:
+ # the menu stays inside the window, and a sixty-track list is threaded
+ # through the 192px that were free below where it was opened.
+ c = cases["a submenu expanded inside it"]
+ assert c["panelHeight"] > c["viewport"] - c["openedAt"], (
+ f"the panel is still the size the collapsed menu was given: {c}")
+
+
+def test_a_short_menu_at_the_bottom_edge_still_flips(cases):
+ # No regression on the placement that already worked: a menu that fits
+ # above the pointer is drawn there whole, not pinned to the edge and
+ # squeezed into the few pixels left below it.
+ c = cases["a short menu opened at the bottom edge"]
+ assert c["panelBottom"] <= c["openedAt"], c
+ assert not c["overflows"], f"a four-item menu should not need scrolling: {c}"