-
Notifications
You must be signed in to change notification settings - Fork 2
feat(workbench,ui): matched-geometry view transition on session switch #299
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
AprilNEA
wants to merge
1
commit into
master
Choose a base branch
from
xuan/code-457
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
95 changes: 95 additions & 0 deletions
95
packages/client/workbench/src/surface/__tests__/session-switch-transition.test.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| // @vitest-environment jsdom | ||
|
|
||
| import type { SessionId } from '@linkcode/schema'; | ||
| import { wait } from 'foxts/wait'; | ||
| import { afterEach, describe, expect, it, vi } from 'vitest'; | ||
| import { applySessionSwitchTransition } from '../session-switch-transition'; | ||
|
|
||
| // The real store transitively imports the whole @linkcode/ui barrel (dnd-kit needs | ||
| // ResizeObserver); the transition only reads `reduceMotion`, so stub exactly that. | ||
| const prefs = vi.hoisted(() => ({ reduceMotion: false })); | ||
| vi.mock('../../settings/appearance-store', () => ({ | ||
| useAppearancePrefsStore: { getState: () => prefs }, | ||
| })); | ||
|
|
||
| const SESSION = 'session-1' as SessionId; | ||
|
|
||
| function installVt(impl: (update: () => void) => { finished: Promise<void> }): void { | ||
| document.startViewTransition = impl as unknown as typeof document.startViewTransition; | ||
| } | ||
|
|
||
| function mountPair(): { row: HTMLElement; header: HTMLElement } { | ||
| const row = document.createElement('span'); | ||
| row.dataset.threadTitle = SESSION; | ||
| const header = document.createElement('div'); | ||
| header.dataset.conversationTitle = ''; | ||
| document.body.append(row, header); | ||
| return { row, header }; | ||
| } | ||
|
|
||
| afterEach(() => { | ||
| document.body.innerHTML = ''; | ||
| Reflect.deleteProperty(document, 'startViewTransition'); | ||
| prefs.reduceMotion = false; | ||
| }); | ||
|
|
||
| describe('applySessionSwitchTransition', () => { | ||
| it('applies plainly when the API is missing', () => { | ||
| mountPair(); | ||
| const apply = vi.fn(); | ||
| applySessionSwitchTransition(SESSION, apply); | ||
| expect(apply).toHaveBeenCalledOnce(); | ||
| }); | ||
|
|
||
| it('applies plainly under reduce-motion even with the API present', () => { | ||
| mountPair(); | ||
| const startViewTransition = vi.fn(); | ||
| installVt(startViewTransition); | ||
| prefs.reduceMotion = true; | ||
| const apply = vi.fn(); | ||
| applySessionSwitchTransition(SESSION, apply); | ||
| expect(apply).toHaveBeenCalledOnce(); | ||
| expect(startViewTransition).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('applies plainly when the clicked row is not in the DOM', () => { | ||
| const startViewTransition = vi.fn(); | ||
| installVt(startViewTransition); | ||
| const apply = vi.fn(); | ||
| applySessionSwitchTransition(SESSION, apply); | ||
| expect(apply).toHaveBeenCalledOnce(); | ||
| expect(startViewTransition).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('pairs the row and header names around the switch, then clears them', async () => { | ||
| const { row, header } = mountPair(); | ||
| let rowNameDuringCapture = ''; | ||
| let headerNameAfterUpdate = ''; | ||
| installVt((update) => { | ||
| rowNameDuringCapture = row.style.getPropertyValue('view-transition-name'); | ||
| update(); | ||
| headerNameAfterUpdate = header.style.getPropertyValue('view-transition-name'); | ||
| return { finished: Promise.resolve() }; | ||
| }); | ||
| const apply = vi.fn(); | ||
| applySessionSwitchTransition(SESSION, apply); | ||
| expect(apply).toHaveBeenCalledOnce(); | ||
| expect(rowNameDuringCapture).toBe('thread-title'); | ||
| expect(headerNameAfterUpdate).toBe('thread-title'); | ||
| expect(row.style.getPropertyValue('view-transition-name')).toBe(''); | ||
| // The cleanup sits behind finished → catch → finally; a macrotask flushes all of them. | ||
| await wait(0); | ||
| expect(header.style.getPropertyValue('view-transition-name')).toBe(''); | ||
| }); | ||
|
|
||
| it('clears a stale header name even when the transition is interrupted', async () => { | ||
| const { header } = mountPair(); | ||
| installVt((update) => { | ||
| update(); | ||
| return { finished: Promise.reject(new Error('skipped')) }; | ||
| }); | ||
| applySessionSwitchTransition(SESSION, vi.fn()); | ||
| await wait(0); | ||
| expect(header.style.getPropertyValue('view-transition-name')).toBe(''); | ||
| }); | ||
| }); |
43 changes: 43 additions & 0 deletions
43
packages/client/workbench/src/surface/session-switch-transition.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| import type { SessionId } from '@linkcode/schema'; | ||
| import { noop } from 'foxts/noop'; | ||
| import { flushSync } from 'react-dom'; | ||
| import { useAppearancePrefsStore } from '../settings/appearance-store'; | ||
|
|
||
| /** The matched-geometry pair for a session switch: the clicked thread row's title | ||
| * (`data-thread-title`) travels to the conversation header title (`data-conversation-title`). */ | ||
| const PAIR_NAME = 'thread-title'; | ||
|
|
||
| function headerTitle(): HTMLElement | null { | ||
| return document.querySelector<HTMLElement>('[data-conversation-title]'); | ||
| } | ||
|
|
||
| /** | ||
| * Wrap a session switch in a View Transition. Falls back to a plain apply without the API, | ||
| * under reduce-motion, or when the clicked row is not in the DOM. Only the switching pair may | ||
| * carry the transition name: a duplicate name in either snapshot makes the browser skip the | ||
| * whole transition, so the header's name is cleared on entry and after every run. | ||
| */ | ||
| export function applySessionSwitchTransition(id: SessionId, apply: () => void): void { | ||
| // Session ids are daemon-generated identifiers (no quotes/backslashes) — safe to interpolate. | ||
| const source = document.querySelector<HTMLElement>(`[data-thread-title="${id}"]`); | ||
| if ( | ||
| !source || | ||
| typeof document.startViewTransition !== 'function' || | ||
| useAppearancePrefsStore.getState().reduceMotion | ||
| ) { | ||
| apply(); | ||
| return; | ||
| } | ||
| headerTitle()?.style.removeProperty('view-transition-name'); | ||
| source.style.setProperty('view-transition-name', PAIR_NAME); | ||
| const transition = document.startViewTransition(() => { | ||
| // eslint-disable-next-line @eslint-react/dom-no-flush-sync -- the browser captures the new snapshot when this callback returns, so the React commit must land synchronously | ||
| flushSync(apply); | ||
| source.style.removeProperty('view-transition-name'); | ||
| headerTitle()?.style.setProperty('view-transition-name', PAIR_NAME); | ||
| }); | ||
| // `finished` rejects when a newer transition interrupts this one; cleanup runs either way. | ||
| transition.finished | ||
| .catch(noop) | ||
| .finally(() => headerTitle()?.style.removeProperty('view-transition-name')); | ||
| } | ||
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
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When two session selections occur before the first View Transition update callback runs, both calls retain the same rendered
currentLocation, leave multiple source titles temporarily namedthread-title, and queue separateapplycallbacks. This can make Chromium skip the matched transition because the old snapshot has duplicate names, while navigation history records the stale origin twice so Back skips the intermediate thread. Rapid consecutive clicks or shortcut/palette selections can trigger this; cancel or serialize the pending switch and ensure history is recorded with the selection that actually applies.Useful? React with 👍 / 👎.