Skip to content

Commit 8310dc9

Browse files
tenphitenphi
authored
fix: popover sync (#765)
1 parent 534737c commit 8310dc9

File tree

12 files changed

+172
-135
lines changed

12 files changed

+172
-135
lines changed

‎.changeset/tiny-ducks-accept.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@cube-dev/ui-kit": patch
3+
---
4+
5+
Improve popover state sync.

‎src/components/actions/Menu/Menu.stories.tsx

Lines changed: 57 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import {
4444
} from '../../../index';
4545
import { baseProps } from '../../../stories/lists/baseProps';
4646
import { ComboBox } from '../../fields/ComboBox';
47+
import { FilterPicker } from '../../fields/FilterPicker';
4748
import { Select } from '../../fields/Select';
4849

4950
export default {
@@ -1849,140 +1850,85 @@ export const ComprehensivePopoverSynchronization = () => {
18491850
{ id: 'item4', name: 'Item 4' },
18501851
];
18511852

1853+
const filterPickerOptions = [
1854+
{ id: 'filter1', name: 'Filter 1' },
1855+
{ id: 'filter2', name: 'Filter 2' },
1856+
{ id: 'filter3', name: 'Filter 3' },
1857+
{ id: 'filter4', name: 'Filter 4' },
1858+
];
1859+
18521860
return (
18531861
<Flow gap="4x" placeContent="start" padding="4x">
18541862
<Title level={3} margin="0 0 2x 0">
1855-
Complete Popover Synchronization Demo
1863+
Popover Synchronization
18561864
</Title>
18571865
<Paragraph preset="t4" color="#dark-03" margin="0 0 4x 0">
1858-
Only one popover can be open at a time across all components: Menu,
1859-
Select, and ComboBox. Opening any popover automatically closes others.
1866+
Only one popover can be open at a time. Opening any trigger
1867+
automatically closes others.
18601868
</Paragraph>
18611869

1862-
<Flow gap="3x" flexDirection="row" flexWrap="wrap">
1863-
{/* Regular MenuTrigger */}
1864-
<Card border padding="3x" margin="0 0 3x 0" minWidth="200px">
1865-
<Paragraph preset="t3m" color="#dark" margin="0 0 2x 0">
1870+
<Flex gap="2x" placeItems="start">
1871+
{/* MenuTrigger */}
1872+
<MenuTrigger>
1873+
<Button size="small" icon={<MoreIcon />}>
18661874
MenuTrigger
1867-
</Paragraph>
1868-
<MenuTrigger>
1869-
<Button size="small" icon={<MoreIcon />} aria-label="Open Menu">
1870-
Click me
1871-
</Button>
1872-
<Menu width="220px">
1873-
<Menu.Item key="sync-edit" hotkeys="Ctrl+E">
1874-
Edit (Menu)
1875-
</Menu.Item>
1876-
<Menu.Item key="sync-duplicate" hotkeys="Ctrl+D">
1877-
Duplicate (Menu)
1878-
</Menu.Item>
1879-
<Menu.Item key="sync-delete" hotkeys="Del">
1880-
Delete (Menu)
1881-
</Menu.Item>
1882-
</Menu>
1883-
</MenuTrigger>
1884-
<Paragraph preset="t5" color="#dark-03" margin="2x 0 0 0">
1885-
Standard menu trigger
1886-
</Paragraph>
1887-
</Card>
1875+
</Button>
1876+
<Menu width="220px">
1877+
<Menu.Item key="sync-edit" hotkeys="Ctrl+E">
1878+
Edit (Menu)
1879+
</Menu.Item>
1880+
<Menu.Item key="sync-duplicate" hotkeys="Ctrl+D">
1881+
Duplicate (Menu)
1882+
</Menu.Item>
1883+
<Menu.Item key="sync-delete" hotkeys="Del">
1884+
Delete (Menu)
1885+
</Menu.Item>
1886+
</Menu>
1887+
</MenuTrigger>
18881888

18891889
{/* Anchored Menu */}
1890-
<Card ref={anchorRef1} border padding="3x" minWidth="200px">
1890+
<div ref={anchorRef1}>
18911891
{rendered1}
1892-
<Paragraph preset="t3m" color="#dark" margin="0 0 2x 0">
1893-
Anchored Menu
1894-
</Paragraph>
18951892
<Button
18961893
size="small"
1897-
theme={isOpen1 ? 'accent' : 'secondary'}
18981894
onPress={() => open1({ onAction: handleAction })}
18991895
>
1900-
Edit Menu {isOpen1 ? '(Open)' : ''}
1896+
Anchored Menu
19011897
</Button>
1902-
<Paragraph preset="t5" color="#dark-03" margin="2x 0 0 0">
1903-
useAnchoredMenu hook
1904-
</Paragraph>
1905-
</Card>
1898+
</div>
19061899

19071900
{/* Context Menu */}
1908-
<Card
1909-
ref={targetRef2}
1910-
border
1911-
padding="3x"
1912-
minWidth="200px"
1913-
background={isOpen2 ? '#purple-10' : undefined}
1914-
>
1901+
<div ref={targetRef2}>
19151902
{rendered2}
1916-
<Paragraph preset="t3m" color="#dark" margin="0 0 2x 0">
1917-
Context Menu
1918-
</Paragraph>
1919-
<Paragraph preset="t5" color="#dark-03" margin="0 0 2x 0">
1920-
Right-click here
1921-
</Paragraph>
1922-
<Paragraph preset="t5" color="#dark-03" margin="2x 0 0 0">
1923-
useContextMenu hook
1924-
</Paragraph>
1925-
</Card>
1903+
<Button size="small">Context Menu (Right-click)</Button>
1904+
</div>
19261905

19271906
{/* Select */}
1928-
<Card border padding="3x" minWidth="200px">
1929-
<Paragraph preset="t3m" color="#dark" margin="0 0 2x 0">
1930-
Select
1931-
</Paragraph>
1932-
<Select
1933-
label="Choose option"
1934-
placeholder="Select an option"
1935-
defaultSelectedKey="option1"
1936-
>
1937-
{selectOptions.map((option) => (
1938-
<Select.Item key={option.value}>{option.label}</Select.Item>
1939-
))}
1940-
</Select>
1941-
<Paragraph preset="t5" color="#dark-03" margin="2x 0 0 0">
1942-
Select component
1943-
</Paragraph>
1944-
</Card>
1907+
<Select placeholder="Select" defaultSelectedKey="option1" size="small">
1908+
{selectOptions.map((option) => (
1909+
<Select.Item key={option.value}>{option.label}</Select.Item>
1910+
))}
1911+
</Select>
19451912

1946-
{/* ComboBox */}
1947-
<Card border padding="3x" minWidth="200px">
1948-
<Paragraph preset="t3m" color="#dark" margin="0 0 2x 0">
1949-
ComboBox
1950-
</Paragraph>
1951-
<ComboBox
1952-
label="Search items"
1953-
placeholder="Type to search..."
1954-
items={comboBoxItems}
1955-
>
1956-
{(item) => <ComboBox.Item key={item.id}>{item.name}</ComboBox.Item>}
1957-
</ComboBox>
1958-
<Paragraph preset="t5" color="#dark-03" margin="2x 0 0 0">
1959-
ComboBox component
1960-
</Paragraph>
1961-
</Card>
1962-
</Flow>
1913+
{/* Simple Button */}
1914+
<Button size="small">Simple Button</Button>
19631915

1964-
<Alert type="info" title="Test the synchronization">
1965-
<Flow gap="1x">
1966-
<Text>
1967-
1. Click on any menu trigger, select dropdown, or combobox
1968-
</Text>
1969-
<Text>
1970-
2. Then click on another component - the first popover will
1971-
automatically close
1972-
</Text>
1973-
<Text>
1974-
3. Right-click on the context menu card for a context menu
1975-
</Text>
1976-
<Text>
1977-
4. Notice how only one popover can be open at any time across all
1978-
components
1979-
</Text>
1980-
<Text>
1981-
5. This ensures a clean UI where multiple overlays don't compete for
1982-
attention
1983-
</Text>
1984-
</Flow>
1985-
</Alert>
1916+
{/* ComboBox */}
1917+
<ComboBox placeholder="ComboBox" items={comboBoxItems} size="small">
1918+
{(item) => <ComboBox.Item key={item.id}>{item.name}</ComboBox.Item>}
1919+
</ComboBox>
1920+
1921+
{/* FilterPicker */}
1922+
<FilterPicker
1923+
placeholder="FilterPicker"
1924+
items={filterPickerOptions}
1925+
size="small"
1926+
>
1927+
{(item) => (
1928+
<FilterPicker.Item key={item.id}>{item.name}</FilterPicker.Item>
1929+
)}
1930+
</FilterPicker>
1931+
</Flex>
19861932
</Flow>
19871933
);
19881934
};

‎src/components/actions/Menu/Menu.test.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1736,8 +1736,8 @@ describe('Menu synchronization with event bus', () => {
17361736
// - ComboBox
17371737
//
17381738
// Implementation details:
1739-
// 1. Each component generates a unique ID and listens for 'menu:open' events
1740-
// 2. When a component opens, it emits a 'menu:open' event with its ID
1739+
// 1. Each component generates a unique ID and listens for 'popover:open' events
1740+
// 2. When a component opens, it emits a 'popover:open' event with its ID
17411741
// 3. Other open components receive the event and close themselves
17421742
// 4. The event bus uses setTimeout(0) to ensure async emission after render cycles
17431743
// 5. This prevents race conditions where components close themselves immediately after opening

‎src/components/actions/Menu/MenuTrigger.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ function MenuTrigger(props: CubeMenuTriggerProps, ref: DOMRef<HTMLElement>) {
7171

7272
// Listen for other menus opening and close this one if needed
7373
useEffect(() => {
74-
const unsubscribe = on('menu:open', (data: { menuId: string }) => {
74+
const unsubscribe = on('popover:open', (data: { menuId: string }) => {
7575
// If another menu is opening and this menu is open, close this one
7676
if (data.menuId !== menuId && state.isOpen && !isDummy) {
7777
state.close();
@@ -84,7 +84,7 @@ function MenuTrigger(props: CubeMenuTriggerProps, ref: DOMRef<HTMLElement>) {
8484
// Emit event when this menu opens
8585
useEffect(() => {
8686
if (state.isOpen && !isDummy) {
87-
emit('menu:open', { menuId });
87+
emit('popover:open', { menuId });
8888
}
8989
}, [state.isOpen, emit, menuId, isDummy]);
9090

‎src/components/actions/Menu/SubMenuTrigger.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ function InternalSubMenuTrigger(props: InternalSubMenuTriggerProps) {
185185

186186
// Listen for other menus opening and close this submenu if needed
187187
useEffect(() => {
188-
const unsubscribe = on('menu:open', (data: { menuId: string }) => {
188+
const unsubscribe = on('popover:open', (data: { menuId: string }) => {
189189
// If another menu is opening and this submenu is open, close this one
190190
if (data.menuId !== submenuId && state.isOpen) {
191191
state.close();
@@ -198,7 +198,7 @@ function InternalSubMenuTrigger(props: InternalSubMenuTriggerProps) {
198198
// Emit event when this submenu opens
199199
useEffect(() => {
200200
if (state.isOpen) {
201-
emit('menu:open', { menuId: submenuId });
201+
emit('popover:open', { menuId: submenuId });
202202
}
203203
}, [state.isOpen, emit, submenuId]);
204204

‎src/components/actions/use-anchored-menu.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,9 @@ export function useAnchoredMenu<P, T = ComponentProps<typeof MenuTrigger>>(
7373
useEffect(() => {
7474
const el = anchorRef.current;
7575
if (el) {
76-
el.dataset.menuTrigger = '';
76+
el.dataset.popoverTrigger = '';
7777
return () => {
78-
delete el.dataset.menuTrigger;
78+
delete el.dataset.popoverTrigger;
7979
};
8080
}
8181
}, []);
@@ -88,7 +88,7 @@ export function useAnchoredMenu<P, T = ComponentProps<typeof MenuTrigger>>(
8888

8989
// Listen for other menus opening and close this one if needed
9090
useEffect(() => {
91-
const unsubscribe = on('menu:open', (data: { menuId: string }) => {
91+
const unsubscribe = on('popover:open', (data: { menuId: string }) => {
9292
// If another menu is opening and this menu is open, close this one
9393
if (data.menuId !== menuId && isOpen) {
9494
setIsOpen(false);
@@ -101,7 +101,7 @@ export function useAnchoredMenu<P, T = ComponentProps<typeof MenuTrigger>>(
101101
// Emit event when this menu opens
102102
useEffect(() => {
103103
if (isOpen) {
104-
emit('menu:open', { menuId });
104+
emit('popover:open', { menuId });
105105
}
106106
}, [isOpen, emit, menuId]);
107107

‎src/components/actions/use-context-menu.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ export function useContextMenu<
104104

105105
// Listen for other menus opening and close this one if needed
106106
useEffect(() => {
107-
const unsubscribe = on('menu:open', (data: { menuId: string }) => {
107+
const unsubscribe = on('popover:open', (data: { menuId: string }) => {
108108
// If another menu is opening and this menu is open, close this one
109109
if (data.menuId !== menuId && isOpen) {
110110
setIsOpen(false);
@@ -118,7 +118,7 @@ export function useContextMenu<
118118
// Emit event when this menu opens
119119
useEffect(() => {
120120
if (isOpen) {
121-
emit('menu:open', { menuId });
121+
emit('popover:open', { menuId });
122122
}
123123
}, [isOpen, emit, menuId]);
124124

‎src/components/fields/ComboBox/ComboBox.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ export const ComboBox = forwardRef(function ComboBox<T extends object>(
230230

231231
// Listen for other menus opening and close this one if needed
232232
useEffect(() => {
233-
const unsubscribe = on('menu:open', (data: { menuId: string }) => {
233+
const unsubscribe = on('popover:open', (data: { menuId: string }) => {
234234
// If another menu is opening and this combobox is open, close this one
235235
if (data.menuId !== comboBoxId && state.isOpen) {
236236
state.close();
@@ -243,7 +243,7 @@ export const ComboBox = forwardRef(function ComboBox<T extends object>(
243243
// Emit event when this combobox opens
244244
useEffect(() => {
245245
if (state.isOpen) {
246-
emit('menu:open', { menuId: comboBoxId });
246+
emit('popover:open', { menuId: comboBoxId });
247247
}
248248
}, [state.isOpen, emit, comboBoxId]);
249249

‎src/components/fields/FilterPicker/FilterPicker.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -883,7 +883,7 @@ export const FilterPicker = forwardRef(function FilterPicker<T extends object>(
883883
const renderTrigger = (state) => {
884884
// Listen for other menus opening and close this one if needed
885885
useEffect(() => {
886-
const unsubscribe = on('menu:open', (data: { menuId: string }) => {
886+
const unsubscribe = on('popover:open', (data: { menuId: string }) => {
887887
// If another menu is opening and this FilterPicker is open, close this one
888888
if (data.menuId !== filterPickerId && state.isOpen) {
889889
state.close();
@@ -896,7 +896,7 @@ export const FilterPicker = forwardRef(function FilterPicker<T extends object>(
896896
// Emit event when this FilterPicker opens
897897
useEffect(() => {
898898
if (state.isOpen) {
899-
emit('menu:open', { menuId: filterPickerId });
899+
emit('popover:open', { menuId: filterPickerId });
900900
}
901901
}, [state.isOpen, emit, filterPickerId]);
902902

‎src/components/fields/Select/Select.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ function Select<T extends object>(
279279

280280
// Listen for other menus opening and close this one if needed
281281
useEffect(() => {
282-
const unsubscribe = on('menu:open', (data: { menuId: string }) => {
282+
const unsubscribe = on('popover:open', (data: { menuId: string }) => {
283283
// If another menu is opening and this select is open, close this one
284284
if (data.menuId !== selectId && state.isOpen) {
285285
state.close();
@@ -292,7 +292,7 @@ function Select<T extends object>(
292292
// Emit event when this select opens
293293
useEffect(() => {
294294
if (state.isOpen) {
295-
emit('menu:open', { menuId: selectId });
295+
emit('popover:open', { menuId: selectId });
296296
}
297297
}, [state.isOpen, emit, selectId]);
298298

0 commit comments

Comments
 (0)