test(desktop): extract ProfileRail focus/visibilitychange wiring into a tested hook
Addresses review feedback from the hermes-sweeper (salvageability=high, keep_open): "The new focus/visibility listener behavior lacks a runtime UI regression test... no ProfileRail test." Rendering the full ProfileRail component for this would drag in drag-and-drop, dialogs, hotkeys, and i18n unrelated to what needs testing. Instead, extracted the focus/visibilitychange wiring into its own use-profile-rail-refresh-on-active hook, matching this exact directory's own established convention (use-profile-prewarm.ts is the same shape: a small side-effect hook pulled out of ProfileRail specifically so it's unit-testable in isolation). Added 6 tests covering exactly what the review asked for: refresh on mount, refresh on window focus, refresh on visibilitychange while visible, NO refresh on visibilitychange while hidden, listener cleanup on unmount, and no listener accumulation across repeated mount/unmount cycles. Verified the tests have real teeth: simulated the exact bug this PR originally fixed (dropped the cleanup return, leaving listeners attached after unmount) and confirmed 4 of 6 tests correctly fail against it -- including "no accumulate listeners" showing 7 calls instead of 1, the exact leaked-listener signature. Restored the real fix and all 6 pass. ProfileRail itself is otherwise unchanged in behavior -- this is a pure extraction (same effect, same dependencies, same cleanup), not a behavior change. Full sidebar test suite: 93 passed across 12 files (up from 87 across 11), 0 regressions. Python side unaffected: 158 passed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -69,6 +69,7 @@ import { RenameProfileDialog } from '../../profiles/rename-profile-dialog'
|
||||
import { PROFILES_ROUTE, SETTINGS_ROUTE } from '../../routes'
|
||||
|
||||
import { useProfilePrewarm } from './use-profile-prewarm'
|
||||
import { useProfileRailRefreshOnActive } from './use-profile-rail-refresh-on-active'
|
||||
|
||||
const RAIL_GAP = 4 // px — matches gap-1 between squares.
|
||||
|
||||
@@ -207,30 +208,10 @@ export function ProfileRail() {
|
||||
// Re-pull the running profile + list on mount, and again whenever the window
|
||||
// regains focus/visibility -- a profile created, deleted, or renamed by
|
||||
// another surface (Manage Profiles, another window, the CLI) leaves this
|
||||
// rail's cached $profiles stale until something re-fetches it. Without this,
|
||||
// a deleted profile's square lingered in the rail until the user happened to
|
||||
// open Manage Profiles (whose own refresh() call was the only other reader).
|
||||
// Cheap and best-effort, matching the focus/visibilitychange refresh pattern
|
||||
// used elsewhere in the sidebar (see refreshProjects/refreshProjectTree).
|
||||
useEffect(() => {
|
||||
void refreshActiveProfile()
|
||||
|
||||
const onActive = () => {
|
||||
if (document.visibilityState === 'hidden') {
|
||||
return
|
||||
}
|
||||
|
||||
void refreshActiveProfile()
|
||||
}
|
||||
|
||||
window.addEventListener('focus', onActive)
|
||||
document.addEventListener('visibilitychange', onActive)
|
||||
|
||||
return () => {
|
||||
window.removeEventListener('focus', onActive)
|
||||
document.removeEventListener('visibilitychange', onActive)
|
||||
}
|
||||
}, [])
|
||||
// rail's cached $profiles stale until something re-fetches it. See
|
||||
// use-profile-rail-refresh-on-active.ts for the extracted (and tested)
|
||||
// wiring.
|
||||
useProfileRailRefreshOnActive()
|
||||
|
||||
// Open the create dialog when the `profile.create` hotkey fires (the dialog
|
||||
// state lives here, so the global keybind bumps a request atom we watch).
|
||||
|
||||
@@ -0,0 +1,91 @@
|
||||
import { act, renderHook } from '@testing-library/react'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { refreshActiveProfile } = vi.hoisted(() => ({
|
||||
refreshActiveProfile: vi.fn().mockResolvedValue(undefined)
|
||||
}))
|
||||
|
||||
vi.mock('@/store/profile', () => ({ refreshActiveProfile }))
|
||||
|
||||
import { useProfileRailRefreshOnActive } from './use-profile-rail-refresh-on-active'
|
||||
|
||||
describe('useProfileRailRefreshOnActive', () => {
|
||||
afterEach(() => {
|
||||
refreshActiveProfile.mockClear()
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
it('refreshes once on mount', () => {
|
||||
renderHook(() => useProfileRailRefreshOnActive())
|
||||
|
||||
expect(refreshActiveProfile).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('refreshes again when the window regains focus', async () => {
|
||||
renderHook(() => useProfileRailRefreshOnActive())
|
||||
refreshActiveProfile.mockClear()
|
||||
|
||||
await act(async () => {
|
||||
window.dispatchEvent(new Event('focus'))
|
||||
})
|
||||
|
||||
expect(refreshActiveProfile).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('refreshes when visibilitychange fires while the document is visible', async () => {
|
||||
vi.spyOn(document, 'visibilityState', 'get').mockReturnValue('visible')
|
||||
renderHook(() => useProfileRailRefreshOnActive())
|
||||
refreshActiveProfile.mockClear()
|
||||
|
||||
await act(async () => {
|
||||
document.dispatchEvent(new Event('visibilitychange'))
|
||||
})
|
||||
|
||||
expect(refreshActiveProfile).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('does NOT refresh when visibilitychange fires while the document is hidden', async () => {
|
||||
// Backgrounding/tab-switching away must not trigger a redundant refresh
|
||||
// -- only becoming visible/focused again should.
|
||||
vi.spyOn(document, 'visibilityState', 'get').mockReturnValue('hidden')
|
||||
renderHook(() => useProfileRailRefreshOnActive())
|
||||
refreshActiveProfile.mockClear()
|
||||
|
||||
await act(async () => {
|
||||
document.dispatchEvent(new Event('visibilitychange'))
|
||||
})
|
||||
|
||||
expect(refreshActiveProfile).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('removes both listeners on unmount, so a later focus/visibilitychange event does not refresh', async () => {
|
||||
vi.spyOn(document, 'visibilityState', 'get').mockReturnValue('visible')
|
||||
const { unmount } = renderHook(() => useProfileRailRefreshOnActive())
|
||||
refreshActiveProfile.mockClear()
|
||||
|
||||
unmount()
|
||||
|
||||
await act(async () => {
|
||||
window.dispatchEvent(new Event('focus'))
|
||||
document.dispatchEvent(new Event('visibilitychange'))
|
||||
})
|
||||
|
||||
expect(refreshActiveProfile).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not accumulate listeners across repeated mount/unmount cycles', async () => {
|
||||
// A leaked listener from a prior mount would double- (or N-times-)
|
||||
// refresh on a single focus event after remounting.
|
||||
const first = renderHook(() => useProfileRailRefreshOnActive())
|
||||
first.unmount()
|
||||
|
||||
renderHook(() => useProfileRailRefreshOnActive())
|
||||
refreshActiveProfile.mockClear()
|
||||
|
||||
await act(async () => {
|
||||
window.dispatchEvent(new Event('focus'))
|
||||
})
|
||||
|
||||
expect(refreshActiveProfile).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,38 @@
|
||||
import { useEffect } from 'react'
|
||||
|
||||
import { refreshActiveProfile } from '@/store/profile'
|
||||
|
||||
/**
|
||||
* Re-pull the running profile + list on mount, and again whenever the window
|
||||
* regains focus/visibility -- a profile created, deleted, or renamed by
|
||||
* another surface (Manage Profiles, another window, the CLI) leaves the
|
||||
* rail's cached $profiles stale until something re-fetches it. Without this,
|
||||
* a deleted profile's square lingers in the rail until the user happens to
|
||||
* open Manage Profiles (whose own refresh() call was the only other reader).
|
||||
*
|
||||
* Cheap and best-effort, matching the focus/visibilitychange refresh pattern
|
||||
* used elsewhere in the sidebar (see refreshProjects/refreshProjectTree).
|
||||
* Extracted into its own hook (rather than left inline in ProfileRail) so the
|
||||
* focus/visibility wiring is unit-testable without rendering the whole rail.
|
||||
*/
|
||||
export function useProfileRailRefreshOnActive(): void {
|
||||
useEffect(() => {
|
||||
void refreshActiveProfile()
|
||||
|
||||
const onActive = () => {
|
||||
if (document.visibilityState === 'hidden') {
|
||||
return
|
||||
}
|
||||
|
||||
void refreshActiveProfile()
|
||||
}
|
||||
|
||||
window.addEventListener('focus', onActive)
|
||||
document.addEventListener('visibilitychange', onActive)
|
||||
|
||||
return () => {
|
||||
window.removeEventListener('focus', onActive)
|
||||
document.removeEventListener('visibilitychange', onActive)
|
||||
}
|
||||
}, [])
|
||||
}
|
||||
Reference in New Issue
Block a user