Skip to content

Commit 51f3566

Browse files
EYHNEYHN
committed
fix(core): fix menu bugs (#8074)
1 parent 01e6370 commit 51f3566

File tree

12 files changed

+188
-79
lines changed

12 files changed

+188
-79
lines changed

‎.eslintrc.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ const config = {
248248
'warn',
249249
{
250250
additionalHooks:
251-
'(useAsyncCallback|useCatchEventCallback|useDraggable|useDropTarget)',
251+
'(useAsyncCallback|useCatchEventCallback|useDraggable|useDropTarget|useRefEffect)',
252252
},
253253
],
254254
},

‎packages/frontend/component/src/hooks/focus-and-select.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ export const useAutoSelect = <T extends HTMLInputElement = HTMLInputElement>(
2525

2626
useLayoutEffect(() => {
2727
if (ref.current && autoSelect) {
28-
ref.current?.select();
28+
setTimeout(() => {
29+
ref.current?.select();
30+
}, 0);
2931
}
3032
}, [autoSelect, ref]);
3133

‎packages/frontend/component/src/hooks/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
export { useAutoFocus, useAutoSelect } from './focus-and-select';
2+
export { useRefEffect } from './use-ref-effect';

‎packages/frontend/component/src/hooks/use-ref-effect.ts‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
/**
2+
* modified version of useRefEffect from https://github.com/jantimon/react-use-ref-effect/blob/master/src/index.tsx
3+
*/
4+
import { useDebugValue, useEffect, useState } from 'react';
5+
6+
// internalRef is used as a reference and therefore save to be used inside an effect
7+
/* eslint-disable react-hooks/exhaustive-deps */
8+
9+
// the `process.env.NODE_ENV !== 'production'` condition is resolved by the build tool
10+
/* eslint-disable react-hooks/rules-of-hooks */
11+
12+
const noop: (...args: any[]) => any = () => {};
13+
14+
/**
15+
* `useRefEffect` returns a mutable ref object to be connected with a DOM Node.
16+
*
17+
* The returned object will persist for the full lifetime of the component.
18+
* Accepts a function that contains imperative, possibly effectful code.
19+
*
20+
* @param effect Imperative function that can return a cleanup function
21+
* @param deps If present, effect will only activate if the ref or the values in the list change.
22+
*/
23+
export const useRefEffect = <T>(
24+
effect: (element: T) => void | (() => void),
25+
dependencies: any[] = []
26+
): React.RefCallback<T> & React.MutableRefObject<T | null> => {
27+
// Use the initial state as mutable reference
28+
const internalRef = useState(() => {
29+
let currentValue = null as T | null;
30+
let cleanupPreviousEffect = noop as () => void;
31+
let currentDeps: any[] | undefined;
32+
/**
33+
* React.RefCallback
34+
*/
35+
const setRefValue = (newElement: T | null) => {
36+
// Only execute if dependencies or element changed:
37+
if (
38+
internalRef.dependencies_ !== currentDeps ||
39+
currentValue !== newElement
40+
) {
41+
currentValue = newElement;
42+
currentDeps = internalRef.dependencies_;
43+
internalRef.cleanup_();
44+
if (newElement) {
45+
cleanupPreviousEffect = internalRef.effect_(newElement) || noop;
46+
}
47+
}
48+
};
49+
return {
50+
/** Execute the effects cleanup function */
51+
cleanup_: () => {
52+
cleanupPreviousEffect();
53+
cleanupPreviousEffect = noop;
54+
},
55+
ref_: Object.defineProperty(setRefValue, 'current', {
56+
get: () => currentValue,
57+
set: setRefValue,
58+
}),
59+
} as {
60+
cleanup_: () => void;
61+
ref_: React.RefCallback<T> & React.MutableRefObject<T | null>;
62+
// Those two properties will be set immediately after initialisation
63+
effect_: typeof effect;
64+
dependencies_: typeof dependencies;
65+
};
66+
})[0];
67+
68+
// Show the current ref value in development
69+
// in react dev tools
70+
if (process.env.NODE_ENV !== 'production') {
71+
useDebugValue(internalRef.ref_.current);
72+
}
73+
74+
// Keep a ref to the latest callback
75+
internalRef.effect_ = effect;
76+
77+
useEffect(
78+
() => {
79+
// Run effect if dependencies change
80+
internalRef.ref_(internalRef.ref_.current);
81+
return () => {
82+
if (internalRef.dependencies_ === dependencies) {
83+
internalRef.cleanup_();
84+
internalRef.dependencies_ = [];
85+
}
86+
};
87+
}, // Keep a ref to the latest dependencies
88+
(internalRef.dependencies_ = dependencies)
89+
);
90+
91+
return internalRef.ref_;
92+
};

‎packages/frontend/component/src/ui/editable/inline-edit.stories.tsx‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ Basic.args = {
5757
editable: true,
5858
placeholder: 'Untitled',
5959
trigger: 'doubleClick',
60-
autoSelect: true,
6160
};
6261

6362
export const CustomizeText: StoryFn<typeof InlineEdit> =
@@ -104,7 +103,6 @@ export const TriggerEdit: StoryFn<typeof InlineEdit> = args => {
104103
TriggerEdit.args = {
105104
value: 'Trigger edit mode in parent component by `handleRef`',
106105
editable: true,
107-
autoSelect: true,
108106
};
109107

110108
export const UpdateValue: StoryFn<typeof InlineEdit> = args => {
@@ -137,5 +135,4 @@ export const UpdateValue: StoryFn<typeof InlineEdit> = args => {
137135
UpdateValue.args = {
138136
value: 'Update value in parent component by `value`',
139137
editable: true,
140-
autoSelect: true,
141138
};

‎packages/frontend/component/src/ui/editable/inline-edit.tsx‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,6 @@ export interface InlineEditProps
4646
*/
4747
trigger?: 'click' | 'doubleClick';
4848

49-
/**
50-
* whether to auto select all text when trigger edit
51-
*/
52-
autoSelect?: boolean;
53-
5449
/**
5550
* Placeholder when value is empty
5651
*/
@@ -79,7 +74,6 @@ export const InlineEdit = ({
7974
className,
8075
style,
8176
trigger = 'doubleClick',
82-
autoSelect,
8377

8478
onInput,
8579
onChange,
@@ -104,11 +98,7 @@ export const InlineEdit = ({
10498
const triggerEdit = useCallback(() => {
10599
if (!editable) return;
106100
setEditing(true);
107-
setTimeout(() => {
108-
inputRef.current?.focus();
109-
autoSelect && inputRef.current?.select();
110-
}, 0);
111-
}, [autoSelect, editable]);
101+
}, [editable]);
112102

113103
const onDoubleClick = useCallback(() => {
114104
if (trigger !== 'doubleClick') return;
@@ -208,7 +198,7 @@ export const InlineEdit = ({
208198
</div>
209199

210200
{/* actual input */}
211-
{
201+
{editing && (
212202
<Input
213203
ref={inputRef}
214204
className={styles.inlineEditInput}
@@ -220,9 +210,11 @@ export const InlineEdit = ({
220210
style={inputWrapperInheritsStyles}
221211
inputStyle={inputInheritsStyles}
222212
onBlur={onBlur}
213+
autoFocus
214+
autoSelect
223215
{...inputAttrs}
224216
/>
225-
}
217+
)}
226218
</div>
227219
);
228220
};

‎packages/frontend/component/src/ui/menu/desktop/controller.ts‎

Lines changed: 61 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,24 @@
1-
import { useCallback, useEffect, useRef, useState } from 'react';
1+
import { useCallback, useState } from 'react';
2+
3+
import { useRefEffect } from '../../../hooks';
24

35
export const useMenuContentController = ({
46
onOpenChange,
57
side,
68
defaultOpen,
79
sideOffset,
10+
open: controlledOpen,
811
}: {
912
defaultOpen?: boolean;
1013
side?: 'top' | 'bottom' | 'left' | 'right';
1114
onOpenChange?: (open: boolean) => void;
15+
open?: boolean;
1216
sideOffset?: number;
1317
} = {}) => {
1418
const [open, setOpen] = useState(defaultOpen ?? false);
19+
const actualOpen = controlledOpen ?? open;
1520
const contentSide = side ?? 'bottom';
1621
const [contentOffset, setContentOffset] = useState<number>(0);
17-
const contentRef = useRef<HTMLDivElement>(null);
1822

1923
const handleOpenChange = useCallback(
2024
(open: boolean) => {
@@ -23,65 +27,69 @@ export const useMenuContentController = ({
2327
},
2428
[onOpenChange]
2529
);
26-
useEffect(() => {
27-
if (!open || !contentRef.current) return;
30+
const contentRef = useRefEffect<HTMLDivElement>(
31+
contentElement => {
32+
if (!actualOpen) return;
2833

29-
const wrapperElement = contentRef.current.parentNode as HTMLDivElement;
34+
const wrapperElement = contentElement.parentNode as HTMLDivElement;
3035

31-
const updateContentOffset = () => {
32-
if (!contentRef.current) return;
33-
const contentRect = wrapperElement.getBoundingClientRect();
34-
if (contentSide === 'bottom') {
35-
setContentOffset(prev => {
36-
const viewportHeight = window.innerHeight;
37-
const newOffset = Math.min(
38-
viewportHeight - (contentRect.bottom - prev),
39-
0
40-
);
41-
return newOffset;
42-
});
43-
} else if (contentSide === 'top') {
44-
setContentOffset(prev => {
45-
const newOffset = Math.max(contentRect.top - prev, 0);
46-
return newOffset;
47-
});
48-
} else if (contentSide === 'left') {
49-
setContentOffset(prev => {
50-
const newOffset = Math.max(contentRect.left - prev, 0);
51-
return newOffset;
52-
});
53-
} else if (contentSide === 'right') {
54-
setContentOffset(prev => {
55-
const viewportWidth = window.innerWidth;
56-
const newOffset = Math.min(
57-
viewportWidth - (contentRect.right - prev),
58-
0
59-
);
60-
return newOffset;
61-
});
62-
}
63-
};
64-
let animationFrame: number = 0;
65-
const requestUpdateContentOffset = () => {
66-
cancelAnimationFrame(animationFrame);
67-
animationFrame = requestAnimationFrame(updateContentOffset);
68-
};
36+
const updateContentOffset = () => {
37+
if (!contentElement) return;
38+
const contentRect = wrapperElement.getBoundingClientRect();
39+
if (contentSide === 'bottom') {
40+
setContentOffset(prev => {
41+
const viewportHeight = window.innerHeight;
42+
const newOffset = Math.min(
43+
viewportHeight - (contentRect.bottom - prev),
44+
0
45+
);
46+
return newOffset;
47+
});
48+
} else if (contentSide === 'top') {
49+
setContentOffset(prev => {
50+
const newOffset = Math.min(contentRect.top + prev, 0);
51+
return newOffset;
52+
});
53+
} else if (contentSide === 'left') {
54+
setContentOffset(prev => {
55+
const newOffset = Math.min(contentRect.left + prev, 0);
56+
return newOffset;
57+
});
58+
} else if (contentSide === 'right') {
59+
setContentOffset(prev => {
60+
const viewportWidth = window.innerWidth;
61+
const newOffset = Math.min(
62+
viewportWidth - (contentRect.right - prev),
63+
0
64+
);
65+
return newOffset;
66+
});
67+
}
68+
};
69+
let animationFrame: number = 0;
70+
const requestUpdateContentOffset = () => {
71+
cancelAnimationFrame(animationFrame);
72+
animationFrame = requestAnimationFrame(updateContentOffset);
73+
};
6974

70-
const observer = new ResizeObserver(requestUpdateContentOffset);
71-
observer.observe(wrapperElement);
72-
window.addEventListener('resize', requestUpdateContentOffset);
73-
requestUpdateContentOffset();
74-
return () => {
75-
observer.disconnect();
76-
window.removeEventListener('resize', requestUpdateContentOffset);
77-
cancelAnimationFrame(animationFrame);
78-
};
79-
}, [contentSide, open]);
75+
const observer = new ResizeObserver(requestUpdateContentOffset);
76+
observer.observe(wrapperElement);
77+
window.addEventListener('resize', requestUpdateContentOffset);
78+
requestUpdateContentOffset();
79+
return () => {
80+
observer.disconnect();
81+
window.removeEventListener('resize', requestUpdateContentOffset);
82+
cancelAnimationFrame(animationFrame);
83+
};
84+
},
85+
[actualOpen, contentSide]
86+
);
8087

8188
return {
8289
handleOpenChange,
8390
contentSide,
8491
contentOffset: (sideOffset ?? 0) + contentOffset,
8592
contentRef,
93+
open: actualOpen,
8694
};
8795
};

‎packages/frontend/component/src/ui/menu/desktop/root.tsx‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,13 @@ export const DesktopMenu = ({
1010
children,
1111
items,
1212
portalOptions,
13-
rootOptions: { onOpenChange, defaultOpen, modal, ...rootOptions } = {},
13+
rootOptions: {
14+
onOpenChange,
15+
defaultOpen,
16+
modal,
17+
open: rootOpen,
18+
...rootOptions
19+
} = {},
1420
contentOptions: {
1521
className = '',
1622
style: contentStyle = {},
@@ -19,8 +25,9 @@ export const DesktopMenu = ({
1925
...otherContentOptions
2026
} = {},
2127
}: MenuProps) => {
22-
const { handleOpenChange, contentSide, contentOffset, contentRef } =
28+
const { handleOpenChange, contentSide, contentOffset, contentRef, open } =
2329
useMenuContentController({
30+
open: rootOpen,
2431
defaultOpen,
2532
onOpenChange,
2633
side,
@@ -31,6 +38,7 @@ export const DesktopMenu = ({
3138
onOpenChange={handleOpenChange}
3239
defaultOpen={defaultOpen}
3340
modal={modal ?? false}
41+
open={open}
3442
{...rootOptions}
3543
>
3644
<DropdownMenu.Trigger

0 commit comments

Comments
 (0)