Skip to content

Commit 46fd0cb

Browse files
waleedlatif1waleedlatif1
committed
fix(ui): align organization and workspace sidebar interactions
1 parent b3524ec commit 46fd0cb

5 files changed

Lines changed: 198 additions & 78 deletions

File tree

‎apps/sim/app/o/[organizationId]/components/organization-sidebar/organization-sidebar.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ export const OrganizationSidebar = memo(function OrganizationSidebar() {
205205
onExpandSidebar={toggleCollapsed}
206206
/>
207207
<div
208+
inert={isCollapsed}
208209
className={cn(
209210
'flex h-[30px] items-center gap-[1px] overflow-hidden',
210211
isCollapsed

‎apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/workspace-header.tsx‎

Lines changed: 33 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ import {
4848
} from '@/hooks/queries/workspace'
4949
import { usePermissionConfig } from '@/hooks/use-permission-config'
5050
import { useSettingsNavigation } from '@/hooks/use-settings-navigation'
51-
import { SIDEBAR_WIDTH } from '@/stores/constants'
5251

5352
const logger = createLogger('WorkspaceHeader')
5453

@@ -458,31 +457,32 @@ function WorkspaceHeaderImpl({
458457
return (
459458
<div className='min-w-0 flex-1'>
460459
{isMounted && isCollapsed ? (
461-
<button
462-
type='button'
460+
<Chip
463461
aria-label='Expand sidebar'
464462
onClick={onExpandSidebar}
465-
className={cn(chipVariants({ fullWidth: true }), SIDEBAR_RAIL_CHIP_CLASS)}
466-
>
467-
<div className='relative flex size-[16px] shrink-0 items-center justify-center'>
468-
{activeWorkspace ? (
469-
<>
470-
<IdentityTile
471-
initial={workspaceInitial}
472-
logoUrl={activeWorkspaceFull?.logoUrl}
473-
alt={activeWorkspaceFull?.name || 'Workspace logo'}
474-
className='group-hover:invisible'
475-
/>
476-
<PanelLeft
477-
aria-hidden
478-
className='pointer-events-none invisible absolute inset-0 m-auto size-[16px] rotate-180 text-[var(--text-icon)] group-hover:visible'
479-
/>
480-
</>
481-
) : (
482-
<Skeleton className='size-[16px] rounded-sm' />
483-
)}
484-
</div>
485-
</button>
463+
fullWidth
464+
className={SIDEBAR_RAIL_CHIP_CLASS}
465+
leftAdornment={
466+
<div className='relative flex size-[16px] shrink-0 items-center justify-center'>
467+
{activeWorkspace ? (
468+
<>
469+
<IdentityTile
470+
initial={workspaceInitial}
471+
logoUrl={activeWorkspaceFull?.logoUrl}
472+
alt={activeWorkspaceFull?.name || 'Workspace logo'}
473+
className='group-hover:invisible'
474+
/>
475+
<PanelLeft
476+
aria-hidden
477+
className='pointer-events-none invisible absolute inset-0 m-auto size-[16px] rotate-180 text-[var(--text-icon)] group-hover:visible'
478+
/>
479+
</>
480+
) : (
481+
<Skeleton className='size-[16px] rounded-sm' />
482+
)}
483+
</div>
484+
}
485+
/>
486486
) : isMounted && isWorkspaceReady ? (
487487
<DropdownMenu
488488
open={isWorkspaceMenuOpen}
@@ -508,36 +508,25 @@ function WorkspaceHeaderImpl({
508508
}}
509509
>
510510
<DropdownMenuTrigger asChild>
511-
<button
512-
type='button'
511+
<Chip
513512
aria-label='Switch workspace'
514-
className={cn(chipVariants(), 'min-w-0 max-w-full')}
513+
className='min-w-0 max-w-full'
515514
onContextMenu={(e) => {
516515
if (activeWorkspaceFull) {
517516
handleContextMenu(e, activeWorkspaceFull)
518517
}
519518
}}
520-
>
521-
{activeWorkspaceFull ? (
519+
leftAdornment={
522520
<IdentityTile
523521
initial={workspaceInitial}
524522
logoUrl={activeWorkspaceFull.logoUrl}
525523
alt={activeWorkspaceFull.name || 'Workspace logo'}
526524
/>
527-
) : (
528-
<Skeleton className='size-[16px] shrink-0 rounded-sm' />
529-
)}
530-
{!isCollapsed && activeWorkspace?.name && (
531-
<>
532-
<OverflowText
533-
label={activeWorkspace.name}
534-
className={cn('flex-1', chipContentLabelClass)}
535-
focusTarget='nearest-interactive'
536-
/>
537-
<ChipChevronDown />
538-
</>
539-
)}
540-
</button>
525+
}
526+
rightAdornment={activeWorkspace?.name ? <ChipChevronDown /> : undefined}
527+
>
528+
{activeWorkspace?.name}
529+
</Chip>
541530
</DropdownMenuTrigger>
542531
<DropdownMenuContent
543532
align='start'
@@ -547,12 +536,7 @@ function WorkspaceHeaderImpl({
547536
still bounded by the space Radix measured — at six rows the menu is tall
548537
enough that a short viewport would otherwise push the footer actions off
549538
screen with nothing able to scroll to them. */
550-
className='flex max-h-[var(--radix-dropdown-menu-content-available-height,400px)] flex-col overflow-y-auto'
551-
style={{
552-
width: `${SIDEBAR_WIDTH.DEFAULT}px`,
553-
maxWidth: 'calc(100vw - 24px)',
554-
}}
555-
onCloseAutoFocus={(e) => e.preventDefault()}
539+
className='flex max-h-[var(--radix-dropdown-menu-content-available-height,400px)] w-64 max-w-[calc(100vw-24px)] flex-col overflow-y-auto'
556540
>
557541
<OrganizationMenuItems onNavigate={() => setIsWorkspaceMenuOpen(false)} />
558542
{isWorkspacesLoading ? (

‎apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1349,6 +1349,7 @@ export const Sidebar = memo(function Sidebar() {
13491349
* flex-1`, so an `auto` width would crush the workspace chip instead.
13501350
*/}
13511351
<div
1352+
inert={isCollapsed}
13521353
className={cn(
13531354
'flex h-[30px] items-center gap-[1px] overflow-hidden',
13541355
isCollapsed

‎apps/sim/components/settings/settings-sidebar.test.tsx‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { act, type ComponentProps } from 'react'
5+
import { Users } from '@sim/emcn/icons'
6+
import { createRoot, type Root } from 'react-dom/client'
7+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
8+
9+
const { mockPush, mockReplace, mockNavigate } = vi.hoisted(() => ({
10+
mockPush: vi.fn(),
11+
mockReplace: vi.fn(),
12+
mockNavigate: vi.fn(),
13+
}))
14+
15+
vi.mock('next/navigation', () => ({
16+
usePathname: () => '/o/org-a/settings/members',
17+
useRouter: () => ({ push: mockPush, replace: mockReplace }),
18+
}))
19+
vi.mock('@/app/(landing)/components/navbar/components', () => ({ SimWordmark: () => null }))
20+
vi.mock('@/components/settings/settings-intent-link', () => ({
21+
SettingsIntentLink: ({
22+
onNavigate,
23+
replace: _replace,
24+
scroll: _scroll,
25+
...props
26+
}: ComponentProps<'a'> & {
27+
replace?: boolean
28+
scroll?: boolean
29+
onNavigate?: (event: { preventDefault: () => void }) => void
30+
}) => (
31+
<a
32+
{...props}
33+
href={props.href}
34+
onClick={(event) => {
35+
event.preventDefault()
36+
let prevented = false
37+
onNavigate?.({
38+
preventDefault: () => {
39+
prevented = true
40+
},
41+
})
42+
if (!prevented) mockNavigate(props.href)
43+
}}
44+
/>
45+
),
46+
}))
47+
48+
import { SettingsSidebar } from '@/components/settings/settings-sidebar'
49+
import { useSettingsDirtyStore } from '@/stores/settings/dirty/store'
50+
51+
let root: Root
52+
let container: HTMLDivElement
53+
54+
beforeEach(() => {
55+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
56+
vi.clearAllMocks()
57+
useSettingsDirtyStore.getState().reset()
58+
container = document.createElement('div')
59+
document.body.appendChild(container)
60+
root = createRoot(container)
61+
})
62+
63+
afterEach(() => {
64+
act(() => root.unmount())
65+
container.remove()
66+
useSettingsDirtyStore.getState().reset()
67+
})
68+
69+
function renderSidebar(isCollapsed = false) {
70+
act(() =>
71+
root.render(
72+
<SettingsSidebar
73+
plane='organization'
74+
activeSection='members'
75+
groups={[{ key: 'organization', title: 'Organization' }]}
76+
items={[
77+
{ id: 'members', label: 'Members', group: 'organization', icon: Users },
78+
{ id: 'search-mcp', label: 'Search MCP', group: 'organization', icon: Users },
79+
]}
80+
hrefForSection={(section) => `/o/org-a/settings/${section}`}
81+
backHref='/o/org-a/home'
82+
isCollapsed={isCollapsed}
83+
/>
84+
)
85+
)
86+
}
87+
88+
function button(label: string): HTMLButtonElement {
89+
const element = [...document.querySelectorAll('button')].find(
90+
(candidate) => candidate.textContent?.trim() === label
91+
)
92+
if (!element) throw new Error(`Missing button: ${label}`)
93+
return element
94+
}
95+
96+
describe('SettingsSidebar interactions', () => {
97+
it('keeps destinations available in the icon rail after collapsing a section', () => {
98+
renderSidebar()
99+
act(() => button('Organization').click())
100+
expect(container.querySelector('a')).toBeNull()
101+
102+
renderSidebar(true)
103+
expect(container.querySelectorAll('a')).toHaveLength(2)
104+
105+
renderSidebar()
106+
expect(button('Organization')).toHaveAttribute('aria-expanded', 'false')
107+
act(() => button('Organization').click())
108+
expect(container.querySelectorAll('a')).toHaveLength(2)
109+
})
110+
111+
it('preserves dirty settings when Back is cancelled, then leaves only after confirmation', () => {
112+
renderSidebar()
113+
act(() => useSettingsDirtyStore.getState().setDirty(true))
114+
act(() => button('Back').click())
115+
expect(mockPush).not.toHaveBeenCalled()
116+
act(() => button('Keep editing').click())
117+
expect(useSettingsDirtyStore.getState().isDirty).toBe(true)
118+
expect(mockPush).not.toHaveBeenCalled()
119+
120+
act(() => button('Back').click())
121+
act(() => button('Discard changes').click())
122+
expect(mockPush).toHaveBeenCalledWith('/o/org-a/home')
123+
})
124+
125+
it('blocks section navigation during a save even when the draft is already clean', () => {
126+
renderSidebar()
127+
act(() => useSettingsDirtyStore.getState().setNavigationBlocked(true))
128+
act(() => container.querySelector<HTMLAnchorElement>('a[href$="search-mcp"]')?.click())
129+
expect(mockNavigate).not.toHaveBeenCalled()
130+
expect(mockReplace).not.toHaveBeenCalled()
131+
expect(useSettingsDirtyStore.getState().pendingLeave).toBeNull()
132+
133+
act(() => useSettingsDirtyStore.getState().setNavigationBlocked(false))
134+
act(() => container.querySelector<HTMLAnchorElement>('a[href$="search-mcp"]')?.click())
135+
expect(mockNavigate).toHaveBeenCalledWith('/o/org-a/settings/search-mcp')
136+
})
137+
})

0 commit comments

Comments
 (0)