Repository navigation
fix(app): slide between questions instead of flashing the question dock - #59085
Conversation
There was a problem hiding this comment.
The fix looks sound. On beta, the new frame-sampling component test fails with text visibility at 0 mid-switch; with this change all 15 question dock component tests pass, and they stay green when each runs three times. tsc -b in packages/app is clean.
Public API is unchanged: this only touches the app UI (new motion-tuning knobs and a story are dev-only).
No blocking concerns.
There was a problem hiding this comment.
The latest commit looks sound. My first review left no open threads.
All 16 question dock component tests pass, including the new rapid-switch test; I ran each three times and all 48 passed. tsc -b in packages/app is clean.
Public API is unchanged.
No new concerns.
There was a problem hiding this comment.
The default island-morph style works: its frame-sampling tests pass, as do the wipe and reduced-motion tests. tsc -b in packages/app is clean. My earlier reviews left no open threads.
One new test fails here. "keeps one outgoing copy and a visible question through rapid switches (slide)" failed 3 out of 3 runs on ArrowRight ArrowRight ArrowLeft ArrowLeft every 1 frames: question text visibility drops to about 0.37 against the test's 0.5 threshold. So with the blur slide, very fast key presses can still leave the card nearly blank, and the test likely depends on frame timing (see inline).
The commit also ships all three styles behind a tuning-panel knob, though users only ever get the default (see inline).
Public API is unchanged.
| const live = into ? arriving(into, v) : rest | ||
| const moving = into ? speed(into) : 0 | ||
| const back = ghost?.tab === tab ? ghost : undefined | ||
| const kept = !back && ghost && leaving(ghost, v).shown > live.shown ? ghost : undefined |
There was a problem hiding this comment.
With style B (slide), the new rapid-switch test fails here every time (3 out of 3). On ArrowRight ArrowRight ArrowLeft ArrowLeft with 1 frame between keys, the summed visibility of the question text drops to about 0.37 (Expected: >= 0.5). Morph and wipe pass the same sequence. It looks like a kept ghost hands over to a new into that starts at shown: 0, so both copies are faint at the same time. Either the handoff needs to keep at least one copy visible, or the test is sensitive to frame timing on slower machines. Your run reported it green.
| // How the old question gives way to the new one. A: the card reshapes as one surface while the old question blurs and | ||
| // shrinks away and the new one resolves out of a blur. B: they slide past each other, streaked along the way they | ||
| // move. C: a soft edge sweeps across the card, showing the new question behind it and the old one ahead of it. | ||
| const style = motionTuning.choice("question.style", { |
There was a problem hiding this comment.
The three styles are a tuning-panel choice, so users only ever see morph. The slide and wipe code paths, their per-style knobs and the two SVG streak filters rendered in every dock all ship to production unused. Once a style is picked, it would be simpler and lighter to keep only that one; the comparison could stay on a branch or in the story.
There was a problem hiding this comment.
The blur-slide rapid-switch failure is fixed: the fading copy now holds the card until the next question has caught up. All 21 question dock component tests pass. I ran each three times and all 63 passed. tsc -b in packages/app is clean.
Public API is unchanged.
No new concerns.
There was a problem hiding this comment.
Both earlier threads are addressed. Only the island morph style ships now, and the blur-slide fix from the previous commit still holds.
On dev with this PR's spec and story copied in, "keeps a question on screen while switching" fails with text visibility at 0. With the PR, all 20 question dock component tests pass; I ran each three times and all 60 passed. tsc is clean in packages/app and packages/ui.
Public API is unchanged (@opencode/ui/rolling-number is a new internal UI component).
One new point: the new RollingNumber duplicates the existing AnimatedNumber (inline).
| // A whole number whose changed digits roll: up when it grows, down when it shrinks. Each digit rolls inside its own | ||
| // box on a bounce-free spring, by transform and opacity only, and swaps at once under reduced motion. Digits are | ||
| // tabular, so the width holds while they roll. | ||
| export function RollingNumber(props: { value: number; spring?: DurationOptions; class?: string }) { |
There was a problem hiding this comment.
packages/ui already has AnimatedNumber (components/animated-number.tsx), which rolls changed digits and is used for the tool counts in session-ui. Adding RollingNumber next to it gives the app two components for the same job with different motion (a WAAPI or CSS digit strip versus a frame-loop spring). Could the question count use AnimatedNumber, or could AnimatedNumber move onto this spring version, so there is only one?
fb8a9ed to
1fe8c45
Compare
There was a problem hiding this comment.
This push is a rebase onto dev plus a fix to the tooltip check in "shows question navigation shortcuts". The RollingNumber / AnimatedNumber duplication thread is still open.
After the rebase, all 20 question dock component tests pass; I ran each three times and all 60 passed. tsc -b in packages/app is clean. On current dev, "keeps a question on screen while switching" still fails with text visibility at 0.
Public API is unchanged.
No new concerns.
There was a problem hiding this comment.
The footer animation works and tsc -b in packages/app is clean. The RollingNumber / AnimatedNumber thread is still open.
One new problem: "shows question navigation shortcuts in buttons and tooltips" now fails intermittently, 3 out of 10 runs here, at line 680: two tooltips are open when Submit is hovered (see inline). It passed every run at the previous head. The other 21 dock tests passed every run, including the new footer tests.
Public API is unchanged.
There was a problem hiding this comment.
The flaky tooltip test is fixed: Submit's tooltip is now looked up by name. The tooltip test and the new footer test passed 16 out of 16 runs. All 22 question dock component tests pass; I ran each three times and all 66 passed. tsc -b in packages/app is clean. The RollingNumber / AnimatedNumber thread is still open.
Public API is unchanged.
No new concerns.
Switching questions reshapes the card as one surface: the old question blurs, shrinks and fades as a still ghost while the new one sharpens in, on one spring. The count rolls, one bar slides along the progress dashes, and the footer's Back and Next/Submit move with the switch. Built on #59333: the answer highlight still jumps, shows only on hover or the arrow keys, and the leaving ghost carries no copy of it.
252edf0 to
1a48b22
Compare
There was a problem hiding this comment.
This push squashes the branch onto a newer dev and merges dev's hover/arrow-key-only answer highlight into the switch code. The old question's fading copy no longer carries its own highlight, and the new test covering that fails on dev. All 24 question dock component tests pass; I ran each three times and all 72 passed. tsc -b in packages/app is clean. The RollingNumber / AnimatedNumber thread is still open.
Public API is unchanged.
No new concerns.
Stepping between questions with ← and → no longer flashes the question dock.
Before, the new question swapped in at zero opacity and faded up, so for a few frames the card was nearly empty. Now the card reshapes as one surface.
❌ Before
✅ After
rolling-numberdoes this, with no new dependency.width. Back rides that edge. Both stay clickable mid-move, and a Back that is leaving can't be clicked.Built on #59333 and keeps all of it: the answer highlight still jumps with no glide, and shows only on hover or the arrow keys, never on a click. The leaving ghost drops its copy of the highlight, so only one answer is ever highlighted.
📊 Proof
Slow-motion clip (4×): → → ←, then a click on the last dash and one on the first: https://problems.kitlangton.com/media/HSgBu1T5xbUaWcHHs5g6Bg.mp4
Footer, before (
dev) | after at 4× slow (Back in, on to Submit and back, Back out): https://problems.kitlangton.com/media/0Svi_za5fBL3cKMjLCwN7A.mp4The first test fails on the old dock. 24 component tests, #59333's included, passed three runs in a row at the head.
👀 Read:
session-question-dock.tsx, the ghost and spring code. Heights still animateheightwhile they spring; everything else is transform, opacity and blur. Not checked on a real device. The rest is routine.Seen in real use · switch timings stay as tunable knobs under Timeline › Question · checked in headless Chromium, LTR and RTL
Checks run
bun typecheckinpackages/app,packages/uiandpackages/session-ui(after rebase ontodev): no errorscomponent-tests/session-question-dock.spec.tsat1a48b2273e: 24 passed, three runs in a rowbun run lint:changed: 0 problems