Skip to content

Commit a16f0f0

Browse files
aojunhao123aojunhao123
authoredNov 11, 2024
feat(a11y): enhance keyboard interaction in search mode (#592)
* feat(a11y): enhance keyboard interaction in search mode * refactor(a11y): synchronize activeKey with search state * fix: lint fix * feat: default active first item * chore: remove useless test case * feat: default active first item * refactor: merge active effect logic * chore: adjust code style * fix: type fix * feat: adjust active effect logic * fix: lint fix * chore: remove useless code * feat: flatten tree to match first node * chore: add .vscode to gitignore file * feat: improve flatten treeData * feat: improve flatten treeData * chore: adjust code style * perf: optimize tree node searching with flattened data * chore: add comment * revert: restore recursive node search * chore: remove unnecessary logic * chore: remove unnecessary logic * chore: adjust logic * test: use testing-library * fix: keep active matched item when search
1 parent 919dced commit a16f0f0

6 files changed

+291
-77
lines changed
 

‎.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ dist
2525
build
2626
lib
2727
coverage
28+
.vscode
2829
yarn.lock
2930
package-lock.json
3031
es

‎src/OptionList.tsx

+79-34
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,6 @@ const OptionList: React.ForwardRefRenderFunction<ReviseRefOptionListProps> = (_,
7676
(prev, next) => next[0] && prev[1] !== next[1],
7777
);
7878

79-
// ========================== Active ==========================
80-
const [activeKey, setActiveKey] = React.useState<Key>(null);
81-
const activeEntity = keyEntities[activeKey as SafeKey];
82-
8379
// ========================== Values ==========================
8480
const mergedCheckedKeys = React.useMemo(() => {
8581
if (!checkable) {
@@ -97,18 +93,29 @@ const OptionList: React.ForwardRefRenderFunction<ReviseRefOptionListProps> = (_,
9793
// Single mode should scroll to current key
9894
if (open && !multiple && checkedKeys.length) {
9995
treeRef.current?.scrollTo({ key: checkedKeys[0] });
100-
setActiveKey(checkedKeys[0]);
10196
}
10297
// eslint-disable-next-line react-hooks/exhaustive-deps
10398
}, [open]);
10499

105-
// ========================== Search ==========================
106-
const lowerSearchValue = String(searchValue).toLowerCase();
107-
const filterTreeNode = (treeNode: EventDataNode<any>) => {
108-
if (!lowerSearchValue) {
109-
return false;
100+
// ========================== Events ==========================
101+
const onListMouseDown: React.MouseEventHandler<HTMLDivElement> = event => {
102+
event.preventDefault();
103+
};
104+
105+
const onInternalSelect = (__: Key[], info: TreeEventInfo) => {
106+
const { node } = info;
107+
108+
if (checkable && isCheckDisabled(node)) {
109+
return;
110+
}
111+
112+
onSelect(node.key, {
113+
selected: !checkedKeys.includes(node.key),
114+
});
115+
116+
if (!multiple) {
117+
toggleOpen(false);
110118
}
111-
return String(treeNode[treeNodeFilterProp]).toLowerCase().includes(lowerSearchValue);
112119
};
113120

114121
// =========================== Keys ===========================
@@ -122,13 +129,6 @@ const OptionList: React.ForwardRefRenderFunction<ReviseRefOptionListProps> = (_,
122129
return searchValue ? searchExpandedKeys : expandedKeys;
123130
}, [expandedKeys, searchExpandedKeys, treeExpandedKeys, searchValue]);
124131

125-
React.useEffect(() => {
126-
if (searchValue) {
127-
setSearchExpandedKeys(getAllKeys(treeData, fieldNames));
128-
}
129-
// eslint-disable-next-line react-hooks/exhaustive-deps
130-
}, [searchValue]);
131-
132132
const onInternalExpand = (keys: Key[]) => {
133133
setExpandedKeys(keys);
134134
setSearchExpandedKeys(keys);
@@ -138,26 +138,71 @@ const OptionList: React.ForwardRefRenderFunction<ReviseRefOptionListProps> = (_,
138138
}
139139
};
140140

141-
// ========================== Events ==========================
142-
const onListMouseDown: React.MouseEventHandler<HTMLDivElement> = event => {
143-
event.preventDefault();
141+
// ========================== Search ==========================
142+
const lowerSearchValue = String(searchValue).toLowerCase();
143+
const filterTreeNode = (treeNode: EventDataNode<any>) => {
144+
if (!lowerSearchValue) {
145+
return false;
146+
}
147+
return String(treeNode[treeNodeFilterProp]).toLowerCase().includes(lowerSearchValue);
144148
};
145149

146-
const onInternalSelect = (__: Key[], info: TreeEventInfo) => {
147-
const { node } = info;
150+
React.useEffect(() => {
151+
if (searchValue) {
152+
setSearchExpandedKeys(getAllKeys(treeData, fieldNames));
153+
}
154+
// eslint-disable-next-line react-hooks/exhaustive-deps
155+
}, [searchValue]);
148156

149-
if (checkable && isCheckDisabled(node)) {
157+
// ========================== Get First Selectable Node ==========================
158+
const getFirstMatchingNode = (nodes: EventDataNode<any>[]): EventDataNode<any> | null => {
159+
for (const node of nodes) {
160+
if (node.disabled || node.selectable === false) {
161+
continue;
162+
}
163+
164+
if (searchValue) {
165+
if (filterTreeNode(node)) {
166+
return node;
167+
}
168+
} else {
169+
return node;
170+
}
171+
172+
if (node[fieldNames.children]) {
173+
const matchInChildren = getFirstMatchingNode(node[fieldNames.children]);
174+
if (matchInChildren) {
175+
return matchInChildren;
176+
}
177+
}
178+
}
179+
return null;
180+
};
181+
182+
// ========================== Active ==========================
183+
const [activeKey, setActiveKey] = React.useState<Key>(null);
184+
const activeEntity = keyEntities[activeKey as SafeKey];
185+
186+
React.useEffect(() => {
187+
if (!open) {
150188
return;
151189
}
190+
let nextActiveKey = null;
152191

153-
onSelect(node.key, {
154-
selected: !checkedKeys.includes(node.key),
155-
});
192+
const getFirstNode = () => {
193+
const firstNode = getFirstMatchingNode(memoTreeData);
194+
return firstNode ? firstNode[fieldNames.value] : null;
195+
};
156196

157-
if (!multiple) {
158-
toggleOpen(false);
197+
// single mode active first checked node
198+
if (!multiple && checkedKeys.length && !searchValue) {
199+
nextActiveKey = checkedKeys[0];
200+
} else {
201+
nextActiveKey = getFirstNode();
159202
}
160-
};
203+
204+
setActiveKey(nextActiveKey);
205+
}, [open, searchValue]);
161206

162207
// ========================= Keyboard =========================
163208
React.useImperativeHandle(ref, () => ({
@@ -176,8 +221,8 @@ const OptionList: React.ForwardRefRenderFunction<ReviseRefOptionListProps> = (_,
176221
// >>> Select item
177222
case KeyCode.ENTER: {
178223
if (activeEntity) {
179-
const { selectable, value } = activeEntity?.node || {};
180-
if (selectable !== false) {
224+
const { selectable, value, disabled } = activeEntity?.node || {};
225+
if (selectable !== false && !disabled) {
181226
onInternalSelect(null, {
182227
node: { key: activeKey },
183228
selected: !checkedKeys.includes(value),
@@ -197,10 +242,10 @@ const OptionList: React.ForwardRefRenderFunction<ReviseRefOptionListProps> = (_,
197242
}));
198243

199244
const loadDataFun = useMemo(
200-
() => searchValue ? null : (loadData as any),
245+
() => (searchValue ? null : (loadData as any)),
201246
[searchValue, treeExpandedKeys || expandedKeys],
202247
([preSearchValue], [nextSearchValue, nextExcludeSearchExpandedKeys]) =>
203-
preSearchValue !== nextSearchValue && !!(nextSearchValue || nextExcludeSearchExpandedKeys)
248+
preSearchValue !== nextSearchValue && !!(nextSearchValue || nextExcludeSearchExpandedKeys),
204249
);
205250

206251
// ========================== Render ==========================

‎tests/Select.SearchInput.spec.js

+100
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
/* eslint-disable no-undef */
22
import React, { useState } from 'react';
33
import { mount } from 'enzyme';
4+
import { render, fireEvent } from '@testing-library/react';
45
import TreeSelect, { TreeNode } from '../src';
6+
import KeyCode from 'rc-util/lib/KeyCode';
57

68
describe('TreeSelect.SearchInput', () => {
79
it('select item will clean searchInput', () => {
@@ -198,4 +200,102 @@ describe('TreeSelect.SearchInput', () => {
198200
nodes.first().simulate('click');
199201
expect(called).toBe(1);
200202
});
203+
204+
describe('keyboard events', () => {
205+
it('should select first matched node when press enter', () => {
206+
const onSelect = jest.fn();
207+
const { getByRole } = render(
208+
<TreeSelect
209+
showSearch
210+
open
211+
onSelect={onSelect}
212+
treeData={[
213+
{ value: '1', label: '1' },
214+
{ value: '2', label: '2', disabled: true },
215+
{ value: '3', label: '3' },
216+
]}
217+
/>,
218+
);
219+
220+
// Search and press enter, should select first matched non-disabled node
221+
const input = getByRole('combobox');
222+
fireEvent.change(input, { target: { value: '1' } });
223+
fireEvent.keyDown(input, { keyCode: KeyCode.ENTER });
224+
expect(onSelect).toHaveBeenCalledWith('1', expect.anything());
225+
onSelect.mockReset();
226+
227+
// Search disabled node and press enter, should not select
228+
fireEvent.change(input, { target: { value: '2' } });
229+
fireEvent.keyDown(input, { keyCode: KeyCode.ENTER });
230+
expect(onSelect).not.toHaveBeenCalled();
231+
onSelect.mockReset();
232+
});
233+
234+
it('should not select node when no matches found', () => {
235+
const onSelect = jest.fn();
236+
const { getByRole } = render(
237+
<TreeSelect
238+
showSearch
239+
onSelect={onSelect}
240+
open
241+
treeData={[
242+
{ value: '1', label: '1' },
243+
{ value: '2', label: '2' },
244+
]}
245+
/>,
246+
);
247+
248+
// Search non-existent value and press enter, should not select any node
249+
const input = getByRole('combobox');
250+
fireEvent.change(input, { target: { value: 'not-exist' } });
251+
fireEvent.keyDown(input, { keyCode: KeyCode.ENTER });
252+
expect(onSelect).not.toHaveBeenCalled();
253+
});
254+
255+
it('should ignore enter press when all matched nodes are disabled', () => {
256+
const onSelect = jest.fn();
257+
const { getByRole } = render(
258+
<TreeSelect
259+
showSearch
260+
onSelect={onSelect}
261+
open
262+
treeData={[
263+
{ value: '1', label: '1', disabled: true },
264+
{ value: '2', label: '2', disabled: true },
265+
]}
266+
/>,
267+
);
268+
269+
// When all matched nodes are disabled, press enter should not select any node
270+
const input = getByRole('combobox');
271+
fireEvent.change(input, { target: { value: '1' } });
272+
fireEvent.keyDown(input, { keyCode: KeyCode.ENTER });
273+
expect(onSelect).not.toHaveBeenCalled();
274+
});
275+
276+
it('should activate first matched node when searching', () => {
277+
const { getByRole, container } = render(
278+
<TreeSelect
279+
showSearch
280+
open
281+
treeData={[
282+
{ value: '1', label: '1' },
283+
{ value: '2', label: '2', disabled: true },
284+
{ value: '3', label: '3' },
285+
]}
286+
/>,
287+
);
288+
289+
// When searching, first matched non-disabled node should be activated
290+
const input = getByRole('combobox');
291+
fireEvent.change(input, { target: { value: '1' } });
292+
expect(container.querySelector('.rc-tree-select-tree-treenode-active')).toHaveTextContent(
293+
'1',
294+
);
295+
296+
// Should skip disabled nodes
297+
fireEvent.change(input, { target: { value: '2' } });
298+
expect(container.querySelectorAll('.rc-tree-select-tree-treenode-active')).toHaveLength(0);
299+
});
300+
});
201301
});

‎tests/Select.spec.tsx

+23-21
Original file line numberDiff line numberDiff line change
@@ -438,13 +438,13 @@ describe('TreeSelect.basic', () => {
438438
keyUp(KeyCode.DOWN);
439439
keyDown(KeyCode.ENTER);
440440
keyUp(KeyCode.ENTER);
441-
matchValue(['parent']);
441+
matchValue(['child']);
442442

443443
keyDown(KeyCode.UP);
444444
keyUp(KeyCode.UP);
445445
keyDown(KeyCode.ENTER);
446446
keyUp(KeyCode.ENTER);
447-
matchValue(['parent', 'child']);
447+
matchValue(['child', 'parent']);
448448
});
449449

450450
it('selectable works with keyboard operations', () => {
@@ -467,12 +467,12 @@ describe('TreeSelect.basic', () => {
467467

468468
keyDown(KeyCode.DOWN);
469469
keyDown(KeyCode.ENTER);
470-
expect(onChange).toHaveBeenCalledWith(['parent'], expect.anything(), expect.anything());
471-
onChange.mockReset();
470+
expect(onChange).not.toHaveBeenCalled();
472471

473472
keyDown(KeyCode.UP);
474473
keyDown(KeyCode.ENTER);
475-
expect(onChange).not.toHaveBeenCalled();
474+
expect(onChange).toHaveBeenCalledWith(['parent'], expect.anything(), expect.anything());
475+
onChange.mockReset();
476476
});
477477

478478
it('active index matches value', () => {
@@ -535,6 +535,24 @@ describe('TreeSelect.basic', () => {
535535
keyDown(KeyCode.UP);
536536
expect(wrapper.find('.rc-tree-select-tree-treenode-active').text()).toBe('11 label');
537537
});
538+
539+
it('should active first un-disabled option when dropdown is opened', () => {
540+
const treeData = [
541+
{ key: '0', value: '0', title: '0 label', disabled: true },
542+
{ key: '1', value: '1', title: '1 label' },
543+
{ key: '2', value: '2', title: '2 label' },
544+
];
545+
546+
const wrapper = mount(<TreeSelect treeData={treeData} />);
547+
548+
expect(wrapper.find('.rc-tree-select-tree-treenode-active')).toHaveLength(0);
549+
550+
wrapper.openSelect();
551+
552+
const activeNode = wrapper.find('.rc-tree-select-tree-treenode-active');
553+
expect(activeNode).toHaveLength(1);
554+
expect(activeNode.text()).toBe('1 label');
555+
});
538556
});
539557

540558
it('click in list should preventDefault', () => {
@@ -591,22 +609,6 @@ describe('TreeSelect.basic', () => {
591609
expect(container.querySelector('.rc-tree-select-selector').textContent).toBe('parent 1-0');
592610
});
593611

594-
it('should not add new tag when key enter is pressed if nothing is active', () => {
595-
const onSelect = jest.fn();
596-
597-
const wrapper = mount(
598-
<TreeSelect open treeDefaultExpandAll multiple onSelect={onSelect}>
599-
<TreeNode value="parent 1-0" title="parent 1-0">
600-
<TreeNode value="leaf1" title="my leaf" disabled />
601-
<TreeNode value="leaf2" title="your leaf" disabled />
602-
</TreeNode>
603-
</TreeSelect>,
604-
);
605-
606-
wrapper.find('input').first().simulate('keydown', { which: KeyCode.ENTER });
607-
expect(onSelect).not.toHaveBeenCalled();
608-
});
609-
610612
it('should not select parent if some children is disabled', () => {
611613
const onChange = jest.fn();
612614

0 commit comments

Comments
 (0)