Skip to content

Commit 5a1ffbd

Browse files
j15zj15z
committed
fix(tables): keep menu actions open
1 parent 9fb56ae commit 5a1ffbd

5 files changed

Lines changed: 221 additions & 59 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/components/resource/components/resource-options/resource-options.test.tsx‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,4 +67,35 @@ describe('SortDropdown', () => {
6767
expect(item?.querySelector('[data-testid="column-icon"]')).not.toBeNull()
6868
expect(item?.querySelectorAll('svg')).toHaveLength(2)
6969
})
70+
71+
it('keeps the popup open while changing or clearing the sort', () => {
72+
const onOpenChange = vi.fn()
73+
const onSort = vi.fn()
74+
const onClear = vi.fn()
75+
act(() => {
76+
root.render(
77+
<SortDropdown
78+
open
79+
onOpenChange={onOpenChange}
80+
config={{
81+
options: [{ id: 'name', label: 'Name', icon: ColumnIcon }],
82+
active: { column: 'name', direction: 'asc' },
83+
onSort,
84+
onClear,
85+
}}
86+
/>
87+
)
88+
})
89+
90+
const items = document.body.querySelectorAll<HTMLElement>('[role="menuitem"]')
91+
expect(items).toHaveLength(2)
92+
93+
act(() => items[1]?.click())
94+
expect(onSort).toHaveBeenCalledWith('name', 'desc')
95+
96+
act(() => items[0]?.click())
97+
expect(onClear).toHaveBeenCalledOnce()
98+
expect(onOpenChange).not.toHaveBeenCalledWith(false)
99+
expect(document.body.querySelectorAll('[role="menuitem"]')).toHaveLength(2)
100+
})
70101
})

‎apps/sim/app/workspace/[workspaceId]/components/resource/components/resource-options/resource-options.tsx‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,12 @@ export const SortDropdown = memo(function SortDropdown({
290290
>
291291
{active && onClear && (
292292
<>
293-
<DropdownMenuItem onSelect={onClear}>
293+
<DropdownMenuItem
294+
onSelect={(event) => {
295+
event.preventDefault()
296+
onClear()
297+
}}
298+
>
294299
<X />
295300
Clear sort
296301
</DropdownMenuItem>
@@ -305,7 +310,8 @@ export const SortDropdown = memo(function SortDropdown({
305310
return (
306311
<DropdownMenuItem
307312
key={option.id}
308-
onSelect={() => {
313+
onSelect={(event) => {
314+
event.preventDefault()
309315
if (isActive) {
310316
onSort(option.id, active.direction === 'asc' ? 'desc' : 'asc')
311317
} else {

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/columns-menu/columns-menu.test.tsx‎

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,34 @@
11
/**
22
* @vitest-environment jsdom
33
*/
4-
import { act } from 'react'
4+
import { act, useState } from 'react'
55
import { createRoot, type Root } from 'react-dom/client'
66
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
77
import { ColumnsMenu } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/columns-menu/columns-menu'
88

99
let container: HTMLDivElement
1010
let root: Root
1111

12+
function ColumnsMenuHarness({ onChange }: { onChange: (hiddenColumns: string[]) => void }) {
13+
const [hiddenColumns, setHiddenColumns] = useState<string[]>([])
14+
15+
return (
16+
<ColumnsMenu
17+
columns={[
18+
{ id: 'col-name', name: 'Name', type: 'string' },
19+
{ id: 'col-email', name: 'Email', type: 'string' },
20+
{ id: 'col-company', name: 'Company', type: 'string' },
21+
]}
22+
workflowGroups={[]}
23+
hiddenColumns={hiddenColumns}
24+
onChange={(nextHiddenColumns) => {
25+
setHiddenColumns(nextHiddenColumns)
26+
onChange(nextHiddenColumns)
27+
}}
28+
/>
29+
)
30+
}
31+
1232
beforeEach(() => {
1333
globalThis.IS_REACT_ACT_ENVIRONMENT = true
1434
container = document.createElement('div')
@@ -22,34 +42,28 @@ afterEach(() => {
2242
})
2343

2444
describe('ColumnsMenu', () => {
25-
it('uses the app menu typography and icon sizing shared by Sort', () => {
45+
it('uses the app menu styling and stays open across column changes', () => {
2646
const onChange = vi.fn()
2747
act(() => {
28-
root.render(
29-
<ColumnsMenu
30-
columns={[
31-
{ id: 'col-name', name: 'Name', type: 'string' },
32-
{ id: 'col-email', name: 'Email', type: 'string' },
33-
]}
34-
workflowGroups={[]}
35-
hiddenColumns={[]}
36-
onChange={onChange}
37-
/>
38-
)
48+
root.render(<ColumnsMenuHarness onChange={onChange} />)
3949
})
4050
act(() => {
4151
container
4252
.querySelector<HTMLButtonElement>('button')
4353
?.dispatchEvent(new MouseEvent('pointerdown', { bubbles: true, button: 0 }))
4454
})
4555

46-
const item = document.body.querySelector<HTMLElement>('[role="menuitem"]')
47-
expect(item).not.toBeNull()
48-
expect(item).toHaveClass('text-small')
49-
expect(item?.querySelector('svg')).toHaveClass('size-[14px]')
56+
const items = document.body.querySelectorAll<HTMLElement>('[role="menuitem"]')
57+
expect(items).toHaveLength(3)
58+
expect(items[0]).toHaveClass('text-small')
59+
expect(items[0]?.querySelector('svg')).toHaveClass('size-[14px]')
5060

51-
act(() => item?.click())
61+
act(() => items[0]?.click())
5262
expect(onChange).toHaveBeenCalledWith(['col-name'])
53-
expect(document.body.querySelector('[role="menuitem"]')).not.toBeNull()
63+
64+
const remainingItems = document.body.querySelectorAll<HTMLElement>('[role="menuitem"]')
65+
expect(remainingItems).toHaveLength(3)
66+
act(() => remainingItems[1]?.click())
67+
expect(onChange).toHaveBeenLastCalledWith(['col-name', 'col-email'])
5468
})
5569
})

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/views-menu/views-menu.test.tsx‎

Lines changed: 107 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import { act } from 'react'
55
import { createRoot } from 'react-dom/client'
66
import { renderToStaticMarkup } from 'react-dom/server'
7-
import { describe, expect, it, vi } from 'vitest'
7+
import { afterEach, describe, expect, it, vi } from 'vitest'
88
import type { TableViewWire } from '@/lib/api/contracts/tables'
99
import { ViewsMenu } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/views-menu/views-menu'
1010

@@ -26,6 +26,15 @@ const SECOND_VIEW: TableViewWire = {
2626
isDefault: false,
2727
}
2828

29+
const PRIMARY_VIEW: TableViewWire = {
30+
...DEFAULT_VIEW,
31+
name: 'Primary view',
32+
}
33+
34+
afterEach(() => {
35+
vi.useRealTimers()
36+
})
37+
2938
function renderMenu(views: TableViewWire[], activeViewId: string | null): string {
3039
return renderToStaticMarkup(
3140
<ViewsMenu
@@ -56,7 +65,7 @@ describe('ViewsMenu', () => {
5665
expect(markup).not.toContain('>View<')
5766
})
5867

59-
it('offers a set-default action only for non-default views', () => {
68+
it('shows filled and outline pins without a Default badge and keeps the menu open', () => {
6069
const container = document.createElement('div')
6170
document.body.appendChild(container)
6271
const root = createRoot(container)
@@ -65,8 +74,8 @@ describe('ViewsMenu', () => {
6574
act(() => {
6675
root.render(
6776
<ViewsMenu
68-
views={[DEFAULT_VIEW, SECOND_VIEW]}
69-
activeViewId={DEFAULT_VIEW.id}
77+
views={[PRIMARY_VIEW, SECOND_VIEW]}
78+
activeViewId={PRIMARY_VIEW.id}
7079
onSelect={vi.fn()}
7180
onRename={vi.fn()}
7281
onSetDefault={onSetDefault}
@@ -78,12 +87,103 @@ describe('ViewsMenu', () => {
7887
})
7988
act(() => container.querySelector<HTMLButtonElement>('button[aria-label="Views"]')?.click())
8089

81-
const actions = document.body.querySelectorAll<HTMLButtonElement>(
90+
const defaultPin = document.body.querySelector<HTMLButtonElement>(
91+
'button[aria-label="Current default view"]'
92+
)
93+
const setDefaultPin = document.body.querySelector<HTMLButtonElement>(
8294
'button[aria-label="Set as default"]'
8395
)
84-
expect(actions).toHaveLength(1)
85-
act(() => actions[0]?.click())
96+
97+
expect(defaultPin?.querySelector('svg')).toHaveClass('fill-current')
98+
expect(setDefaultPin?.querySelector('svg')).not.toHaveClass('fill-current')
99+
expect(document.body).not.toHaveTextContent('Default')
100+
101+
act(() => setDefaultPin?.click())
86102
expect(onSetDefault).toHaveBeenCalledWith(SECOND_VIEW.id)
103+
expect(document.body).toHaveTextContent('New view')
104+
105+
act(() => root.unmount())
106+
container.remove()
107+
})
108+
109+
it('keeps the menu open when keyboard focus moves from the trigger to the default pin', () => {
110+
vi.useFakeTimers()
111+
const container = document.createElement('div')
112+
document.body.appendChild(container)
113+
const root = createRoot(container)
114+
115+
act(() => {
116+
root.render(
117+
<ViewsMenu
118+
views={[PRIMARY_VIEW, SECOND_VIEW]}
119+
activeViewId={PRIMARY_VIEW.id}
120+
onSelect={vi.fn()}
121+
onRename={vi.fn()}
122+
onSetDefault={vi.fn()}
123+
onDelete={vi.fn()}
124+
onNewView={vi.fn()}
125+
canEdit
126+
/>
127+
)
128+
})
129+
130+
const trigger = container.querySelector<HTMLButtonElement>('button[aria-label="Views"]')
131+
act(() => trigger?.focus())
132+
133+
const setDefaultPin = document.body.querySelector<HTMLButtonElement>(
134+
'button[aria-label="Set as default"]'
135+
)
136+
expect(setDefaultPin).not.toBeNull()
137+
act(() => {
138+
setDefaultPin?.focus()
139+
vi.advanceTimersByTime(121)
140+
})
141+
142+
expect(document.activeElement).toBe(setDefaultPin)
143+
expect(document.body).toHaveTextContent('New view')
144+
expect(document.body.querySelector('[data-native-surface-overlay]')).not.toBeNull()
145+
146+
act(() => root.unmount())
147+
container.remove()
148+
})
149+
150+
it('shows disabled pins without closing the menu for read-only members', () => {
151+
const container = document.createElement('div')
152+
document.body.appendChild(container)
153+
const root = createRoot(container)
154+
const onSetDefault = vi.fn()
155+
156+
act(() => {
157+
root.render(
158+
<ViewsMenu
159+
views={[PRIMARY_VIEW, SECOND_VIEW]}
160+
activeViewId={PRIMARY_VIEW.id}
161+
onSelect={vi.fn()}
162+
onRename={vi.fn()}
163+
onSetDefault={onSetDefault}
164+
onDelete={vi.fn()}
165+
onNewView={vi.fn()}
166+
canEdit={false}
167+
/>
168+
)
169+
})
170+
act(() => container.querySelector<HTMLButtonElement>('button[aria-label="Views"]')?.click())
171+
172+
const defaultPin = document.body.querySelector<HTMLButtonElement>(
173+
'button[aria-label="Current default view"]'
174+
)
175+
const setDefaultPin = document.body.querySelector<HTMLButtonElement>(
176+
'button[aria-label="Set as default"]'
177+
)
178+
179+
expect(defaultPin?.querySelector('svg')).toHaveClass('fill-current')
180+
expect(setDefaultPin?.querySelector('svg')).not.toHaveClass('fill-current')
181+
expect(setDefaultPin).toBeDisabled()
182+
183+
act(() => setDefaultPin?.click())
184+
185+
expect(onSetDefault).not.toHaveBeenCalled()
186+
expect(document.body.querySelector('[data-native-surface-overlay]')).not.toBeNull()
87187

88188
act(() => root.unmount())
89189
container.remove()

0 commit comments

Comments
 (0)