fix(web-ui): stop the session nav flashing on delete - #1923
Merged
bobleer merged 1 commit intoJul 31, 2026
Conversation
Deleting a session made the nav list flash before the next session moved up. Four things stacked up: - Every row swapped between a bare div and a Tooltip wrapper whenever a row menu opened or closed, changing each row's element type and remounting the whole list. Rows now stay wrapped and the tooltip is suppressed with Tooltip's own `disabled`. - The collapsed view loaded exactly the rows it renders, so the row that takes the freed slot only arrived after a metadata round trip. It now keeps a small off-screen buffer and promotes a loaded row in the same commit. - The post-delete reconcile ran as a foreground load, flipping the expand toggle to a spinner and bouncing its remaining count. Background loads no longer touch the loading/error state, and no longer claim the request id that a user-driven load depends on. - Rows jumped into their new slots in a single frame. They now slide up from where they were, the backfilled row fades in, and the list closes its own height in step so the sections below follow instead of leading. Honors prefers-reduced-motion, and measures with offsetTop so a second delete mid-animation still lands on the real layout.
bobleer
force-pushed
the
bob/bitfun-session-list-flicker-59067e
branch
from
July 31, 2026 05:53
2ae3c47 to
9c3bcc8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Deleting a session from the left nav made the list flash before the next session moved up. This fixes the four causes behind that.
Type and Areas
Type: bug fix (UI/UX)
Areas: web UI (
NavPanelsession list)Motivation / Impact
Deleting a session was visibly janky: the list blinked, the expand toggle flickered between a spinner and two different "show more N" counts, and the replacement row popped in a moment later. Four independent causes:
The whole list remounted. Each row was rendered as
isEditing || openMenuSessionId !== null ? row : <Tooltip>{row}</Tooltip>, so opening or closing a row menu changed every row's element type and forced React to tear down and rebuild the list — and clicking Delete closes the menu. Rows now stay wrapped inTooltipand use its existingdisabledprop (which already exists for exactly this case: "parent opens a menu/popover that covers the trigger").Tooltiprenders viacloneElement, so no extra DOM.The row that moves up needed a round trip. The collapsed view loaded exactly the 5 rows it renders, so after a delete it showed 4 until a metadata page came back. It now keeps a small off-screen buffer (3 rows, refilled quietly in the background) and promotes an already-loaded row in the same commit. The backend paginates by top-level session and excludes archived/hidden ones, so the prefetch limit maps 1:1 to rows.
Background upkeep surfaced as loading state. The post-delete reconcile ran as a normal load: it flipped the expand toggle to a spinner and made its count bounce 7 → 6 → 7 → 6. Background loads now leave
isLoading/loadErroralone and commit counts atomically on success. They also no longer claim the request id, which previously let a background load strand a user-driven load's spinner forever.Rows teleported. Remaining rows now slide up from where they were (180 ms), the backfilled row fades in, and the list animates its own height in step so the workspace sections below follow the rows instead of jumping ahead of them.
No user-facing strings or behavior changes beyond the motion;
prefers-reduced-motionis honored.Verification
New tests (16):
sessionNavExpand.test.ts— buffer prefetch sizing: fill, refill-after-delete, buffer-full, clamped to workspace total, level 1, and the skip cases.sessionRowShift.test.ts— the pure before/after diff: which rows shift and by how much, which row is entering, no motion on insert-only renders, sub-pixel noise ignored.sessionRowShiftTransition.test.tsx(jsdom) — the hook itself: rows start at their old offset then release, the backfilled row fades in, the list height closes up, a row landing mid-animation retargets the collapse instead of getting clipped, and pure inserts stay still.Testing level: fully tested at the unit level; the motion itself was not verified by hand in a running desktop build.
Reviewer Notes
offsetTop, which ignores transforms, so a second delete during an animation still measures the settled layout.totalTopLevelSessionCountis the filtered count, so the buffer prefetch computes to 0 and stays idle — the delete animation still runs, there is just no prefetched row to promote. That path had no buffer before either, so it is not a regression.