Skip to content

Commit b03f9e4

Browse files
committed
fix(desktop): stop the lane controller clobbering a mode another owner set
A regression I introduced last round. `DesktopTitleBarController` seeded `inset` unconditionally on mount, before its own `getState()` resolved. That was harmless while only `AuthShell` mounted it — but giving the file viewer a lane put a controller inside workspace for the first time, exactly where this PR's own comments say `WorkspaceChrome` owns the mode. Opening a file during native fullscreen therefore snapped the traffic-light lane back on and jumped the content, and left it wrong permanently if `getState()` rejected, since the rejection is swallowed. It now seeds only when no owner has established a mode. The pre-paint script sets the marker before first paint and `WorkspaceChrome` maintains it, so the unconditional write was never the thing making the lane correct — it was only ever able to make it wrong. Adds a controller test covering the case directly: mount during `fullscreen` with a `getState` that never settles, and the mode survives. Verified it fails against the previous behaviour.
1 parent 5ca7c6f commit b03f9e4

2 files changed

Lines changed: 102 additions & 3 deletions

File tree

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { act } from 'react'
5+
import { createRoot, type Root } from 'react-dom/client'
6+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
7+
8+
const { mockGetDesktopBridge } = vi.hoisted(() => ({ mockGetDesktopBridge: vi.fn() }))
9+
10+
vi.mock('@/lib/desktop', () => ({ getDesktopBridge: mockGetDesktopBridge }))
11+
12+
import {
13+
DESKTOP_TITLE_BAR_ATTRIBUTE,
14+
DesktopTitleBarController,
15+
} from '@/app/_shell/desktop-title-bar'
16+
17+
let container: HTMLDivElement
18+
let root: Root
19+
20+
/**
21+
* A bridge whose `getState` never settles, which is the window this guards: the gap
22+
* between mount and the async state arriving is exactly when the old code clobbered
23+
* whatever mode another owner had already established.
24+
*/
25+
function pendingBridge() {
26+
return {
27+
windowState: {
28+
onStateChange: vi.fn(() => vi.fn()),
29+
getState: vi.fn(() => new Promise<never>(() => {})),
30+
},
31+
}
32+
}
33+
34+
function mount() {
35+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
36+
container = document.createElement('div')
37+
document.body.appendChild(container)
38+
root = createRoot(container)
39+
act(() => root.render(<DesktopTitleBarController />))
40+
}
41+
42+
beforeEach(() => {
43+
vi.clearAllMocks()
44+
document.documentElement.removeAttribute(DESKTOP_TITLE_BAR_ATTRIBUTE)
45+
Object.defineProperty(navigator, 'userAgent', {
46+
value: 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)',
47+
configurable: true,
48+
})
49+
})
50+
51+
afterEach(() => {
52+
act(() => root.unmount())
53+
container.remove()
54+
document.documentElement.removeAttribute(DESKTOP_TITLE_BAR_ATTRIBUTE)
55+
})
56+
57+
describe('DesktopTitleBarController', () => {
58+
it('leaves an established mode alone while its own state is still pending', () => {
59+
mockGetDesktopBridge.mockReturnValue(pendingBridge())
60+
// Native fullscreen, set by WorkspaceChrome. A lane-aware overlay mounting here used
61+
// to seed `inset` first, snapping the traffic-light lane back on and jumping the
62+
// content until the async state corrected it — or permanently, if `getState` rejected.
63+
document.documentElement.setAttribute(DESKTOP_TITLE_BAR_ATTRIBUTE, 'fullscreen')
64+
65+
mount()
66+
67+
expect(document.documentElement.getAttribute(DESKTOP_TITLE_BAR_ATTRIBUTE)).toBe('fullscreen')
68+
})
69+
70+
it('seeds inset when no owner has set a mode yet', () => {
71+
mockGetDesktopBridge.mockReturnValue(pendingBridge())
72+
73+
mount()
74+
75+
expect(document.documentElement.getAttribute(DESKTOP_TITLE_BAR_ATTRIBUTE)).toBe('inset')
76+
})
77+
78+
it('clears the marker off the desktop shell', () => {
79+
mockGetDesktopBridge.mockReturnValue(null)
80+
document.documentElement.setAttribute(DESKTOP_TITLE_BAR_ATTRIBUTE, 'inset')
81+
82+
mount()
83+
84+
expect(document.documentElement.hasAttribute(DESKTOP_TITLE_BAR_ATTRIBUTE)).toBe(false)
85+
})
86+
})

apps/sim/app/_shell/desktop-title-bar.tsx

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ import { getDesktopBridge } from '@/lib/desktop'
55

66
export type DesktopTitleBarMode = 'fullscreen' | 'inset' | null
77

8+
/** The document marker every lane owner reads and writes. */
9+
export const DESKTOP_TITLE_BAR_ATTRIBUTE = 'data-sim-desktop-title-bar'
10+
811
/**
912
* Whether this surface reserves the macOS traffic-light lane itself.
1013
*
@@ -25,10 +28,10 @@ export function applyDesktopTitleBarMode(
2528
mode: DesktopTitleBarMode
2629
): void {
2730
if (mode === null) {
28-
root.removeAttribute('data-sim-desktop-title-bar')
31+
root.removeAttribute(DESKTOP_TITLE_BAR_ATTRIBUTE)
2932
return
3033
}
31-
root.setAttribute('data-sim-desktop-title-bar', mode)
34+
root.setAttribute(DESKTOP_TITLE_BAR_ATTRIBUTE, mode)
3235
}
3336

3437
/**
@@ -64,7 +67,17 @@ export function DesktopTitleBarController() {
6467
}
6568

6669
const windowState = bridge?.windowState
67-
applyDesktopTitleBarMode(root, 'inset')
70+
/**
71+
* Seed `inset` only when nobody has established a mode yet.
72+
*
73+
* The pre-paint script sets this before first paint and `WorkspaceChrome` maintains it
74+
* for workspace routes, so writing unconditionally clobbered whatever they had:
75+
* mounting a lane-aware overlay during native fullscreen forced the traffic-light lane
76+
* back on and jumped the content, and left it wrong entirely if `getState()` rejected.
77+
*/
78+
if (!root.hasAttribute(DESKTOP_TITLE_BAR_ATTRIBUTE)) {
79+
applyDesktopTitleBarMode(root, 'inset')
80+
}
6881
if (!windowState) return
6982

7083
let disposed = false

0 commit comments

Comments
 (0)