Skip to content

Commit 125cc76

Browse files
authored
Merge pull request #166 from dgreene1/bug/UIEN-4263
Bug fix: Keyboard shortcut should not select node when clickAction is set to focus.
2 parents fce10b0 + 9291924 commit 125cc76

File tree

2 files changed

+81
-23
lines changed

2 files changed

+81
-23
lines changed

src/TreeView/index.tsx

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,7 @@ const handleKeyDown = ({
661661
const element = getTreeNode(data, tabbableId);
662662
const id = element.id;
663663
if (event.ctrlKey) {
664-
if (event.key === "a") {
664+
if (event.key === "a" && clickAction !== clickActions.focus) {
665665
event.preventDefault();
666666
const dataWithoutRoot = data.filter((x) => x.parent !== null);
667667
const ids = dataWithoutRoot
@@ -678,7 +678,8 @@ const handleKeyDown = ({
678678
});
679679
} else if (
680680
event.shiftKey &&
681-
(event.key === "Home" || event.key === "End")
681+
(event.key === "Home" || event.key === "End") &&
682+
clickAction !== clickActions.focus
682683
) {
683684
const newId =
684685
event.key === "Home"
@@ -711,16 +712,18 @@ const handleKeyDown = ({
711712
event.preventDefault();
712713
const previous = getPreviousAccessible(data, id, expandedIds);
713714
if (previous != null && !disabledIds.has(previous)) {
714-
dispatch({
715-
type: treeTypes.changeSelectMany,
716-
ids: propagateSelect
717-
? propagatedIds(data, [previous], disabledIds)
718-
: [previous],
719-
select: true,
720-
multiSelect,
721-
lastInteractedWith: previous,
722-
lastManuallyToggled: previous,
723-
});
715+
if (clickAction !== clickActions.focus) {
716+
dispatch({
717+
type: treeTypes.changeSelectMany,
718+
ids: propagateSelect
719+
? propagatedIds(data, [previous], disabledIds)
720+
: [previous],
721+
select: true,
722+
multiSelect,
723+
lastInteractedWith: previous,
724+
lastManuallyToggled: previous,
725+
});
726+
}
724727
dispatch({
725728
type: treeTypes.focus,
726729
id: previous,
@@ -733,16 +736,18 @@ const handleKeyDown = ({
733736
event.preventDefault();
734737
const next = getNextAccessible(data, id, expandedIds);
735738
if (next != null && !disabledIds.has(next)) {
736-
dispatch({
737-
type: treeTypes.changeSelectMany,
738-
ids: propagateSelect
739-
? propagatedIds(data, [next], disabledIds)
740-
: [next],
741-
multiSelect,
742-
select: true,
743-
lastInteractedWith: next,
744-
lastManuallyToggled: next,
745-
});
739+
if (clickAction !== clickActions.focus) {
740+
dispatch({
741+
type: treeTypes.changeSelectMany,
742+
ids: propagateSelect
743+
? propagatedIds(data, [next], disabledIds)
744+
: [next],
745+
multiSelect,
746+
select: true,
747+
lastInteractedWith: next,
748+
lastManuallyToggled: next,
749+
});
750+
}
746751
dispatch({
747752
type: treeTypes.focus,
748753
id: next,

src/__tests__/ClickActionFocus.test.tsx

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ function FocusClickActionTreeView() {
4141
<TreeView
4242
data={data}
4343
aria-label="directory tree"
44+
togglableSelect
45+
multiSelect
4446
clickAction="FOCUS"
4547
nodeRenderer={({ element, getNodeProps }) => (
4648
<div {...getNodeProps()}>{element.name}</div>
@@ -64,5 +66,56 @@ test("should not select on enter/space keydown when click action is focus", () =
6466
);
6567
}
6668
fireEvent.keyDown(node, { key: " " });
67-
expect(nodes[0]).not.toHaveAttribute("aria-selected");
69+
expect(nodes[0]).toHaveAttribute("aria-selected", "false");
70+
});
71+
72+
test("should not select on control keydown when click action is focus", () => {
73+
const { queryAllByRole } = render(<FocusClickActionTreeView />);
74+
75+
const nodes = queryAllByRole("treeitem");
76+
nodes[0].focus();
77+
78+
if (document.activeElement == null) {
79+
throw new Error(
80+
`Expected to find an active element on the document (after focusing the first element with role["treeitem"]), but did not.`
81+
);
82+
}
83+
84+
fireEvent.keyDown(document.activeElement, { key: "a", ctrlKey: true });
85+
nodes.forEach((x) => expect(x).toHaveAttribute("aria-selected", "false"));
86+
87+
fireEvent.keyDown(document.activeElement, {
88+
key: "Home",
89+
ctrlKey: true,
90+
shiftKey: true,
91+
});
92+
nodes.forEach((x) => expect(x).toHaveAttribute("aria-selected", "false"));
93+
94+
fireEvent.keyDown(document.activeElement, {
95+
key: "End",
96+
ctrlKey: true,
97+
shiftKey: true,
98+
});
99+
nodes.forEach((x) => expect(x).toHaveAttribute("aria-selected", "false"));
100+
});
101+
102+
test("should not select on shift + arrow up/down keydown when click action is focus", () => {
103+
const { queryAllByRole } = render(<FocusClickActionTreeView />);
104+
105+
const nodes = queryAllByRole("treeitem");
106+
nodes[0].focus();
107+
108+
if (document.activeElement == null) {
109+
throw new Error(
110+
`Expected to find an active element on the document (after focusing the first element with role["treeitem"]), but did not.`
111+
);
112+
}
113+
fireEvent.keyDown(document.activeElement, {
114+
key: "ArrowDown",
115+
shiftKey: true,
116+
});
117+
nodes.forEach((x) => expect(x).toHaveAttribute("aria-selected", "false"));
118+
nodes[4].focus();
119+
fireEvent.keyDown(document.activeElement, { key: "ArrowUp", shiftKey: true });
120+
nodes.forEach((x) => expect(x).toHaveAttribute("aria-selected", "false"));
68121
});

0 commit comments

Comments
 (0)