fix(ui,workbench,desktop,webview): swap the thread-title view transition for a header enter animation - #354
Merged
Merged
Conversation
…ion for a header enter animation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62b00f3af7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
# Conflicts: # apps/webview/src/shell/web-workbench-shell.tsx
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.
Closes CODE-511. Follow-up to CODE-457 / CODE-469.
Problem
Switching threads animated two titles at once, in opposite directions — the outgoing title flew from the header down into the sidebar while the incoming one flew from the sidebar up to the header. It read as the two labels trading places rather than as one title being promoted.
Root cause
thread-title-${sessionId}was published symmetrically: the sidebar row carried the boundary only while it was not active, and the header carried it keyed by the open session. On a switch A → B that makes two valid pairs in the same transition:thread-title-Athread-title-Benter="none" exit="none"only silences unpaired enter/exit, so both pairs took the defaultsharemorph. The direction cannot be suppressed on one side either:commitEnterViewTransitions/commitExitViewTransitionseach readprops.sharefrom their own boundary, soshare="none"on the row makes it skip applying the name to the old snapshot,pairednever becomes true, and the wanted direction dies with it.Same scheme, three more defects:
truncated bitmap stretched into the ≤420px header slot across afont-normal→font-semiboldchange — blurry and double-imaged.active.title ?? tk(active.kind), the row issession.title ?? "<agent> in <repo>".selectVisibleSessionspreview truncation) the pair never forms,exit="none"applies, and the old title vanishes instantly — the same interaction animating differently depending on sidebar state.Fix
Drop matched geometry. The sidebar row goes back to a plain
<span>; each header title remounts onsessionIdand plays its own enter animation —animate-title-enter, a fade plus a 4px rise on--motion-fast, declared as--animate-title-enterwith a nested@keyframesin@linkcode/ui's@theme. A rename inside the open thread keeps the key and stays put.No
<ViewTransition>and nodocument.startViewTransitionsurvive, so the dead::view-transition-*rules and the fourtypes: ["react/canary"]entries come out with it (useEffectEventis in stableindex.d.ts; canary was only forViewTransition).Verification
Runtime, in the webview mock host (
vite --mode mock— 5 seeded threads, no daemon):document.startViewTransitioncalled 0 times.MutationObserveron the header title slot catches the remount each switch; the new node carriesanimate-title-enterandgetAnimations()reports{name: "title-enter", state: "running", duration: 150}.view-transition-nameany more (only the UA'srooton<html>), and noview-transitionrule survives in the compiled stylesheet..reduce-motioncollapses the duration0.15s → 1e-05s.Desktop's
MainChromeTitlewas not driven in Electron (that needs a built daemon plus a seeded two-session DB). It is covered by typecheck and by its own renderer build output, which emits--animate-title-enter,.animate-title-enter { animation: var(--animate-title-enter) }and@keyframes title-enterwith zeroview-transitionrules —apps/desktop/src/renderer/src/index.cssdoes not importtw-animate-css, so that was the one real compile risk.pnpm check:cigreen.pnpm test: 2151 passed; 2 failures inpackages/host/engine/tests/integration/worktree-service.test.tsreproduce with this branch fully stashed, so they predate it and are tracked separately.