Skip to content

Commit b1a2886

Browse files
author
Waleed Latif
committed
fix(desktop): cover fullscreen routes, spare embedded panels
Review round 2 found the two arrangements the collapsed-sidebar selector alone gets wrong. A fullscreen route (/upgrade) slides the sidebar to zero width without collapsing it, so `data-sidebar-collapsed` is absent and the lane stayed zero while the pane was in fact sitting under the traffic lights. The pane owns the lane whenever the sidebar is not there to own it, so the selector now matches a new `data-content-fullscreen` as well. The mirror error: the variable is inherited, and the mothership panel is the right half of the pane — never under the lights — yet it embeds whole pages (KnowledgeBase and friends) whose header bars reserve the lane. Those bars were gaining ~38px inside the panel. The panel now zeroes the variable for its subtree. I had checked for exactly this nesting and concluded it did not exist, having looked only at the settings pages that import `Resource` types without rendering `Resource.Header`. `resource-content.tsx` renders the knowledge page itself, which does. An assertion each, both verified to fail when the fix is reverted.
1 parent b7f76b5 commit b1a2886

4 files changed

Lines changed: 29 additions & 4 deletions

File tree

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

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ const pageHeaderBar = read('../../components/page-header-bar.ts')
1818
const resourceHeader = read(
1919
'../workspace/[workspaceId]/components/resource/components/resource-header/resource-header.tsx'
2020
)
21+
const mothershipView = read(
22+
'../workspace/[workspaceId]/home/components/mothership-view/mothership-view.tsx'
23+
)
2124

2225
describe('desktop title-bar surface audit', () => {
2326
it('applies the safe-area shell only when the auth route is login', () => {
@@ -76,9 +79,21 @@ describe('desktop title-bar surface audit', () => {
7679
// /organization/[id], /selfhost; the landing tables preview). An undefined var()
7780
// inside calc() is invalid at computed-value time and drops padding-top entirely.
7881
expect(globalStyles).toMatch(/:root\s*\{[^}]*--workspace-content-title-bar-inset:\s*0px/s)
79-
expect(globalStyles).toContain(
80-
'.workspace-content-shell[data-sidebar-collapsed] {\n --workspace-content-title-bar-inset: var(--desktop-title-bar-height);'
81-
)
82+
// The pane owns the lane in both arrangements where the sidebar is not there to
83+
// own it: collapsed to zero width, and slid away for a fullscreen route (which
84+
// leaves the sidebar expanded in the store, so the collapsed selector alone misses
85+
// it and /upgrade's back chip lands under the traffic lights).
86+
expect(globalStyles).toContain('.workspace-content-shell[data-sidebar-collapsed],')
87+
expect(globalStyles).toContain('.workspace-content-shell[data-content-fullscreen] {')
88+
expect(workspaceChrome).toContain('data-content-fullscreen={isFullscreen || undefined}')
89+
})
90+
91+
it('clears the lane for panels that embed pages away from the lights', () => {
92+
// The mothership panel is the right half of the pane and embeds whole pages
93+
// (KnowledgeBase et al) whose header bars reserve the lane. It inherits the
94+
// variable, so without this reset those bars gain the inset while sitting nowhere
95+
// near the traffic lights.
96+
expect(mothershipView).toContain('[--workspace-content-title-bar-inset:0px]')
8297
})
8398

8499
it('reserves that lane in every top-of-pane header bar', () => {

apps/sim/app/_styles/globals.css

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,10 @@ html[data-sim-desktop-title-bar="inset"]
123123
letter-spacing: 0.02em;
124124
}
125125

126-
.workspace-content-shell[data-sidebar-collapsed] {
126+
/* The pane owns the lane whenever the sidebar is not there to own it: collapsed to
127+
zero width, or slid away for a fullscreen route. */
128+
.workspace-content-shell[data-sidebar-collapsed],
129+
.workspace-content-shell[data-content-fullscreen] {
127130
--workspace-content-title-bar-inset: var(--desktop-title-bar-height);
128131
}
129132

apps/sim/app/workspace/[workspaceId]/components/workspace-chrome/workspace-chrome.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,9 @@ export function WorkspaceChrome({
295295
isCollapsed && '[[data-sim-desktop-title-bar=inset]_&]:p-0'
296296
)}
297297
data-sidebar-collapsed={isCollapsed || undefined}
298+
/* A fullscreen route slides the sidebar away without collapsing it, so the pane
299+
inherits the traffic-light lane the same way a collapsed sidebar does. */
300+
data-content-fullscreen={isFullscreen || undefined}
298301
>
299302
<div className='flex-1 overflow-hidden rounded-[8px] border border-[var(--border)] bg-[var(--bg)]'>
300303
{children}

apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/mothership-view.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,10 @@ export const MothershipView = memo(
137137
className={cn(
138138
'relative z-10 flex h-full flex-col overflow-hidden border-[var(--border)] bg-[var(--bg)] transition-[width,min-width,border-width] duration-200 ease-[cubic-bezier(0.25,0.1,0.25,1)]',
139139
isCollapsed ? 'w-0 min-w-0 border-l-0' : 'w-1/2 border-l',
140+
/* This panel is the right half of the pane, never under the traffic lights,
141+
yet it embeds whole pages whose header bars reserve that lane. Zeroing the
142+
inherited variable here keeps their top bars flush inside the panel. */
143+
'[--workspace-content-title-bar-inset:0px]',
140144
className
141145
)}
142146
>

0 commit comments

Comments
 (0)