Skip to content

Commit 759581f

Browse files
author
叶文俊
committed
feat: optimize code
1 parent 8e64124 commit 759581f

File tree

2 files changed

+73
-69
lines changed

2 files changed

+73
-69
lines changed

src/Menu.tsx

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,24 @@ const Menu = React.forwardRef<MenuRef, MenuProps>((props, ref) => {
387387
setMergedActiveKey(undefined);
388388
});
389389

390+
// ======================== Select ========================
391+
// >>>>> Select keys
392+
const [internalSelectKeys, setMergedSelectKeys] = useControlledState(
393+
defaultSelectedKeys || [],
394+
selectedKeys,
395+
);
396+
const mergedSelectKeys = React.useMemo(() => {
397+
if (Array.isArray(internalSelectKeys)) {
398+
return internalSelectKeys;
399+
}
400+
401+
if (internalSelectKeys === null || internalSelectKeys === undefined) {
402+
return EMPTY_LIST;
403+
}
404+
405+
return [internalSelectKeys];
406+
}, [internalSelectKeys]);
407+
// >>>>> accept ref
390408
useImperativeHandle(ref, () => {
391409
return {
392410
list: containerRef.current,
@@ -400,7 +418,6 @@ const Menu = React.forwardRef<MenuRef, MenuProps>((props, ref) => {
400418
let shouldFocusKey: string;
401419
// find the item to focus on based on whether it is selectable
402420
if (selectable) {
403-
const mergedSelectKeys = getMergedSelectKeys();
404421
// if there is already selected items, select first item to focus
405422
if (mergedSelectKeys.length && keys.includes(mergedSelectKeys[0])) {
406423
shouldFocusKey = mergedSelectKeys[0];
@@ -426,27 +443,6 @@ const Menu = React.forwardRef<MenuRef, MenuProps>((props, ref) => {
426443
};
427444
});
428445

429-
// ======================== Select ========================
430-
// >>>>> Select keys
431-
const [internalSelectKeys, setMergedSelectKeys] = useControlledState(
432-
defaultSelectedKeys || [],
433-
selectedKeys,
434-
);
435-
const mergedSelectKeys = React.useMemo(() => {
436-
if (Array.isArray(internalSelectKeys)) {
437-
return internalSelectKeys;
438-
}
439-
440-
if (internalSelectKeys === null || internalSelectKeys === undefined) {
441-
return EMPTY_LIST;
442-
}
443-
444-
return [internalSelectKeys];
445-
}, [internalSelectKeys]);
446-
function getMergedSelectKeys() {
447-
return mergedSelectKeys;
448-
}
449-
450446
// >>>>> Trigger select
451447
const triggerSelection = (info: MenuInfo) => {
452448
if (selectable) {

tests/Focus.spec.tsx

Lines changed: 55 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -228,30 +228,34 @@ describe('Focus', () => {
228228
);
229229
};
230230
const { getByTestId, container } = render(<TestApp />);
231-
// ================ check keydown ==============
232-
// first item
233-
keyDown(container, KeyCode.DOWN);
234-
isActive(container, 0);
235-
// second item
236-
keyDown(container, KeyCode.DOWN);
237-
isActive(container, 1);
238-
// select second item
239-
keyDown(container, KeyCode.ENTER);
231+
// let focusSpy = jest;
232+
let focusSpy: jest.SpyInstance | undefined;
233+
try {
234+
// ================ check keydown ==============
235+
// first item
236+
keyDown(container, KeyCode.DOWN);
237+
isActive(container, 0);
238+
// second item
239+
keyDown(container, KeyCode.DOWN);
240+
isActive(container, 1);
241+
// select second item
242+
keyDown(container, KeyCode.ENTER);
240243

241-
// mock focus on item 0 to make sure it gets focused
242-
const item0 = getByTestId('0');
243-
const focusSpy = jest.spyOn(item0, 'focus').mockImplementation(() => {});
244-
menuRef.current.focus();
245-
expect(focusSpy).toHaveBeenCalled();
244+
// mock focus on item 0 to make sure it gets focused
245+
const item0 = getByTestId('0');
246+
focusSpy = jest.spyOn(item0, 'focus').mockImplementation(() => {});
247+
menuRef.current.focus();
248+
expect(focusSpy).toHaveBeenCalled();
246249

247-
// ================ check click ==============
248-
// click third item
249-
const item2 = getByTestId('2');
250-
fireEvent.click(item2);
251-
menuRef.current.focus();
252-
expect(focusSpy).toHaveBeenCalled();
253-
// cleanup
254-
focusSpy.mockRestore();
250+
// ================ check click ==============
251+
// click third item
252+
const item2 = getByTestId('2');
253+
fireEvent.click(item2);
254+
menuRef.current.focus();
255+
expect(focusSpy).toHaveBeenCalled();
256+
} finally {
257+
focusSpy.mockRestore();
258+
}
255259
});
256260
it('When selectable is configured, the focus should move to the selected item if there is a selection, else to the first item, not retain on last focused item', async () => {
257261
const menuRef = React.createRef<MenuRef>();
@@ -274,32 +278,36 @@ describe('Focus', () => {
274278
);
275279
};
276280
const { getByTestId, container } = render(<TestApp />);
277-
// ================ check keydown ==============
278-
// first item
279-
keyDown(container, KeyCode.DOWN);
280-
isActive(container, 0);
281-
// second item
282-
keyDown(container, KeyCode.DOWN);
283-
isActive(container, 1);
284-
// select second item
285-
keyDown(container, KeyCode.ENTER);
286-
// mock focus on item 1 to make sure it gets focused
287-
const item1 = getByTestId('1');
288-
const focusSpy = jest.spyOn(item1, 'focus').mockImplementation(() => {});
289-
menuRef.current.focus();
290-
expect(focusSpy).toHaveBeenCalled();
281+
let focusSpy: jest.SpyInstance | undefined;
282+
let focusSpy2: jest.SpyInstance | undefined;
283+
try {
284+
// ================ check keydown ==============
285+
// first item
286+
keyDown(container, KeyCode.DOWN);
287+
isActive(container, 0);
288+
// second item
289+
keyDown(container, KeyCode.DOWN);
290+
isActive(container, 1);
291+
// select second item
292+
keyDown(container, KeyCode.ENTER);
293+
// mock focus on item 1 to make sure it gets focused
294+
const item1 = getByTestId('1');
295+
focusSpy = jest.spyOn(item1, 'focus').mockImplementation(() => {});
296+
menuRef.current.focus();
297+
expect(focusSpy).toHaveBeenCalled();
291298

292-
// ================ check click ==============
293-
// click third item
294-
const item2 = getByTestId('2');
295-
const focusSpy2 = jest.spyOn(item2, 'focus').mockImplementation(() => {});
296-
fireEvent.click(item2);
297-
menuRef.current.focus();
298-
// mock focus on item 2 to make sure it gets focused
299-
expect(focusSpy2).toHaveBeenCalled();
300-
// cleanup
301-
focusSpy.mockRestore();
302-
focusSpy2.mockRestore();
299+
// ================ check click ==============
300+
// click third item
301+
const item2 = getByTestId('2');
302+
focusSpy2 = jest.spyOn(item2, 'focus').mockImplementation(() => {});
303+
fireEvent.click(item2);
304+
menuRef.current.focus();
305+
// mock focus on item 2 to make sure it gets focused
306+
expect(focusSpy2).toHaveBeenCalled();
307+
} finally {
308+
focusSpy?.mockRestore();
309+
focusSpy2?.mockRestore();
310+
}
303311
});
304312
});
305313
/* eslint-enable */

0 commit comments

Comments
 (0)