Skip to content

Commit 2413ec3

Browse files
committed
test(desktop): count the lane class itself as a viewport claim
Cursor caught the audit failing to watch exactly the files this PR converted. Detection keyed on `min-h-screen`/`h-screen`/`fixed inset-0`, but converting a surface to `.desktop-title-bar-page` removes those tokens — the class supplies `min-height: 100vh` itself. So `/oauth-error`, the public-file view and `AuthShell` dropped out of the check entirely, and a nested class-only reservation could ship green. That also means the doubled-reservation check had never actually fired. The mutation I used to "verify" it removed the lane component as well, so the pairing check caught it and the doubled check was never exercised. It now fires on Cursor's exact scenario: a nested class-only reservation, correctly paired, inside a lane-aware shell. Pulling those files back in exposed a second-order bug: a shell's own definition file sits under the layout that renders it, so ancestor resolution called `AuthShell` nested inside itself. Shell definitions are excluded from inheritance. One limit stays, documented rather than papered over: a root is in scope because of how it claims the viewport, so deleting the reservation outright drops the file from the check. That regression is loud, not silent — the surface stops being full height. Closing it properly means treating every route entry point as a window root, which pulls in seven account/organization/selfhost pages needing individual assessment. Worth doing separately; allowlisting them on assumptions is the mistake that produced the `LogoShell` hole.
1 parent dabcc2c commit 2413ec3

1 file changed

Lines changed: 25 additions & 4 deletions

File tree

apps/sim/app/_shell/desktop-title-bar-surfaces.test.ts

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,14 @@ describe('desktop title-bar surface audit', () => {
172172
*
173173
* So the check enumerates instead of listing what to look at: any new full-viewport root
174174
* fails here until it either composes `.desktop-title-bar-page` or is added below with a
175-
* reason. Reaching for this allowlist should feel like a claim you have to defend — the
175+
* reason.
176+
*
177+
* Known limit: a root is in scope because of how it claims the viewport, so deleting the
178+
* reservation outright — class gone, nothing put back — also drops the file from the check.
179+
* That regression is loud rather than silent (the surface stops being full height, which is
180+
* plainly visible), and closing it properly means treating every route entry point as a
181+
* window root, which pulls in seven account/organization/selfhost pages that each need
182+
* their own assessment. Worth doing; not worth guessing at here. Reaching for this allowlist should feel like a claim you have to defend — the
176183
* entry that read "marketing chrome, not reachable in the desktop shell" was false, and
177184
* hid four live surfaces behind one unverified sentence.
178185
*
@@ -200,10 +207,14 @@ const LANE_EXEMPT: Record<string, string> = {
200207
* covered. Both mistakes were in the first draft of this check and made it unfailable.
201208
*/
202209
/**
203-
* How a root claims the whole window. `fixed inset-0` counts: it covers the lights just as
204-
* completely as `h-screen`, and was invisible to the first version of this check.
210+
* How a root claims the whole window.
211+
*
212+
* `fixed inset-0` counts: it covers the lights as completely as `h-screen`. So does
213+
* `desktop-title-bar-page` itself, which sets `min-height: 100vh` — without that arm, every
214+
* surface this PR converted lost its `min-h-screen` token and dropped out of the check
215+
* entirely, leaving the files most likely to regress unwatched.
205216
*/
206-
const FILLS_VIEWPORT = /\b(min-h-screen|h-screen)\b|fixed inset-0/
217+
const FILLS_VIEWPORT = /\b(min-h-screen|h-screen)\b|fixed inset-0|desktop-title-bar-page/
207218

208219
const LANE_AWARE_SHELL_USAGE = /<(AuthShell|LogoShell|WorkspaceChrome|InterfacesShell)\b/
209220

@@ -255,6 +266,15 @@ function ancestorLayouts(file: string): string[] {
255266
return layouts
256267
}
257268

269+
/**
270+
* A file that DEFINES a lane-aware shell, rather than wearing one.
271+
*
272+
* Such a file sits under the very layout that renders it, so ancestor resolution would
273+
* call it nested inside itself and report a double reservation. The shell is the reason
274+
* that layout is lane-aware in the first place.
275+
*/
276+
const DEFINES_LANE_SHELL = /export function (AuthShell|LogoShell|InterfacesShell|WorkspaceChrome)\b/
277+
258278
const reservesLane = (source: string) =>
259279
source.includes('desktop-title-bar-page') || LANE_AWARE_SHELL_USAGE.test(source)
260280

@@ -272,6 +292,7 @@ function viewportRoots() {
272292
self: reservesLane(source),
273293
inherited:
274294
!SELF_RESERVE_REQUIRED.has(file) &&
295+
!DEFINES_LANE_SHELL.test(source) &&
275296
ancestorLayouts(file).some((l) => reservesLane(sourceOf(l))),
276297
}
277298
})

0 commit comments

Comments
 (0)