Skip to content

Commit 9a20d7f

Browse files
myk0la-bkshitijk4poor
authored andcommitted
perf(desktop): keep spinner frames out of React commits
Advance the existing animated status glyph through its DOM text node instead of React state, and pause its timer for hidden panes or inactive windows. Cover frame advancement, zero update-phase commits, and timer suspension with behavior tests.
1 parent 8f52040 commit 9a20d7f

2 files changed

Lines changed: 144 additions & 8 deletions

File tree

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
import { act, render, screen } from '@testing-library/react'
2+
import { Profiler, type ProfilerOnRenderCallback } from 'react'
3+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
4+
5+
import { PaneVisibleContext } from '@/components/pane-shell/pane-visibility'
6+
7+
import { GlyphSpinner } from './glyph-spinner'
8+
9+
describe('GlyphSpinner', () => {
10+
beforeEach(() => {
11+
vi.useFakeTimers()
12+
vi.spyOn(globalThis.document, 'hasFocus').mockReturnValue(true)
13+
})
14+
15+
afterEach(() => {
16+
vi.clearAllTimers()
17+
vi.restoreAllMocks()
18+
vi.useRealTimers()
19+
})
20+
21+
it('advances its glyph without an update-phase React commit', () => {
22+
let updateCommits = 0
23+
24+
const onRender: ProfilerOnRenderCallback = (_id, phase) => {
25+
if (phase !== 'mount') {
26+
updateCommits += 1
27+
}
28+
}
29+
30+
render(
31+
<Profiler id="glyph-spinner" onRender={onRender}>
32+
<GlyphSpinner spinner="braille" />
33+
</Profiler>
34+
)
35+
36+
const status = screen.getByRole('status', { name: 'Loading' })
37+
expect(status.textContent).toBe('⠋')
38+
39+
act(() => vi.advanceTimersByTime(80))
40+
41+
expect(status.textContent).toBe('⠙')
42+
expect(updateCommits).toBe(0)
43+
})
44+
45+
it('does not tick while its kept-alive pane is hidden', () => {
46+
const { rerender } = render(
47+
<PaneVisibleContext.Provider value={false}>
48+
<GlyphSpinner spinner="braille" />
49+
</PaneVisibleContext.Provider>
50+
)
51+
52+
const status = screen.getByRole('status', { name: 'Loading' })
53+
54+
expect(status.textContent).toBe('⠋')
55+
expect(vi.getTimerCount()).toBe(0)
56+
57+
rerender(
58+
<PaneVisibleContext.Provider value>
59+
<GlyphSpinner spinner="braille" />
60+
</PaneVisibleContext.Provider>
61+
)
62+
expect(vi.getTimerCount()).toBe(1)
63+
64+
act(() => vi.advanceTimersByTime(80))
65+
expect(status.textContent).toBe('⠙')
66+
67+
rerender(
68+
<PaneVisibleContext.Provider value={false}>
69+
<GlyphSpinner spinner="braille" />
70+
</PaneVisibleContext.Provider>
71+
)
72+
expect(vi.getTimerCount()).toBe(0)
73+
74+
const frozen = status.textContent
75+
act(() => vi.advanceTimersByTime(800))
76+
expect(status.textContent).toBe(frozen)
77+
})
78+
79+
it('suspends animation while the Desktop window is inactive', () => {
80+
render(<GlyphSpinner spinner="braille" />)
81+
82+
const status = screen.getByRole('status', { name: 'Loading' })
83+
expect(vi.getTimerCount()).toBe(1)
84+
85+
act(() => window.dispatchEvent(new Event('blur')))
86+
expect(vi.getTimerCount()).toBe(0)
87+
88+
const frozen = status.textContent
89+
act(() => vi.advanceTimersByTime(800))
90+
expect(status.textContent).toBe(frozen)
91+
92+
act(() => window.dispatchEvent(new Event('focus')))
93+
expect(vi.getTimerCount()).toBe(1)
94+
95+
act(() => vi.advanceTimersByTime(80))
96+
expect(status.textContent).not.toBe(frozen)
97+
})
98+
})

apps/desktop/src/components/ui/glyph-spinner.tsx

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
import { useEffect, useState } from 'react'
1+
import { useEffect, useRef } from 'react'
22
import spinners, { type BrailleSpinnerName as SpinnerName } from 'unicode-animations'
33

44
import { usePaneVisible } from '@/components/pane-shell/pane-visibility'
5+
import { createRendererLoopPauseController } from '@/lib/renderer-loop-pause'
56
import { cn } from '@/lib/utils'
67

78
export type { SpinnerName }
@@ -43,29 +44,66 @@ interface GlyphSpinnerProps {
4344
*/
4445
export function GlyphSpinner({ ariaLabel = 'Loading', className, spinner = 'braille' }: GlyphSpinnerProps) {
4546
const spin = FRAMES_BY_NAME[spinner] ?? FRAMES_BY_NAME.braille!
46-
const [frame, setFrame] = useState(0)
47+
const glyphRef = useRef<HTMLSpanElement>(null)
4748
// Pause when this surface is a hidden (kept-alive) tab: N mounted tabs each
48-
// ticking a setInterval + setState burn CPU for pixels nobody can see.
49+
// ticking a setInterval burns CPU for pixels nobody can see.
4950
const visible = usePaneVisible()
5051

5152
useEffect(() => {
52-
if (!visible) {
53+
const glyph = glyphRef.current
54+
55+
if (!visible || !glyph) {
5356
return
5457
}
5558

56-
setFrame(0)
57-
const id = window.setInterval(() => setFrame(f => (f + 1) % spin.frames.length), spin.interval)
59+
let frame = 0
60+
let timer: number | undefined
61+
let pauseController: ReturnType<typeof createRendererLoopPauseController> | undefined
62+
glyph.textContent = spin.frames[frame]
63+
64+
const stopAnimation = () => {
65+
if (timer === undefined) {
66+
return
67+
}
68+
69+
window.clearInterval(timer)
70+
timer = undefined
71+
}
72+
73+
const syncAnimation = () => {
74+
if (pauseController?.isPaused()) {
75+
stopAnimation()
76+
77+
return
78+
}
5879

59-
return () => window.clearInterval(id)
80+
if (timer !== undefined) {
81+
return
82+
}
83+
84+
timer = window.setInterval(() => {
85+
frame = (frame + 1) % spin.frames.length
86+
glyph.textContent = spin.frames[frame]
87+
}, spin.interval)
88+
}
89+
90+
pauseController = createRendererLoopPauseController(syncAnimation)
91+
syncAnimation()
92+
93+
return () => {
94+
pauseController.dispose()
95+
stopAnimation()
96+
}
6097
}, [spin, visible])
6198

6299
return (
63100
<span
64101
aria-label={ariaLabel}
65102
className={cn('inline-flex items-center justify-center font-mono leading-none tabular-nums', className)}
103+
ref={glyphRef}
66104
role="status"
67105
>
68-
{spin.frames[frame]}
106+
{spin.frames[0]}
69107
</span>
70108
)
71109
}

0 commit comments

Comments
 (0)