Merge branch 'dgreene1:master' into onSelect-halfSelected-fix

dgreene1 · Aug 24, 2023 · 13c61cb · 13c61cb
2 parents 550a14c + fe97c08
commit 13c61cb
Show file tree

Hide file tree

Showing 6 changed files with 174 additions and 11 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,7 +1,7 @@
 {
   "name": "react-accessible-treeview",
   "description": "A react component that implements the treeview pattern as described by the WAI-ARIA Authoring Practices.",
-  "version": "2.6.3",
+  "version": "2.7.0",
   "author": "lissitz (https://github.com/lissitz)",
   "main": "dist/react-accessible-treeview.cjs.js",
   "module": "dist/react-accessible-treeview.esm.js",

diff --git a/src/TreeView/index.tsx b/src/TreeView/index.tsx
@@ -254,8 +254,9 @@ const useTree = ({
 
   const toggledControlledIds = symmetricDifference(
     new Set(controlledSelectedIds),
-    controlledIds
+    selectedIds
   );
+
   useEffect(() => {
     if (!!controlledSelectedIds) {
       toggledControlledIds.size &&

diff --git a/src/TreeView/utils.ts b/src/TreeView/utils.ts
@@ -214,7 +214,9 @@ export const propagateSelectChange = (
       if (enabledChildren.length === 0) break;
       const some = enabledChildren.some(
         (x) =>
-          selectedIds.has(x) || changes.some.has(x) || halfSelectedIds.has(x)
+          selectedIds.has(x) ||
+          changes.some.has(x) ||
+          (halfSelectedIds.has(x) && !changes.none.has(x))
       );
       if (!some) {
         const selectedAncestorId = getAncestors(

diff --git a/src/__tests__/ControlledTree.test.tsx b/src/__tests__/ControlledTree.test.tsx
@@ -1,6 +1,6 @@
 import "@testing-library/jest-dom/extend-expect";
 import { render, fireEvent } from "@testing-library/react";
-import React from "react";
+import React, { useState } from "react";
 import TreeView, { flattenTree } from "..";
 import { NodeId, INode } from "../TreeView/types";
 
@@ -151,6 +151,31 @@ function MultiSelectCheckboxControlled(
   );
 }
 
+function MultiSelectCheckboxControlledWithStateInside() {
+  const [selectedIds, setSelectedIds] = useState([12]);
+
+  return (
+    <div>
+      <button
+        onClick={() => setSelectedIds([])}
+        data-testid="clear-selected-nodes"
+      >
+        Clear Selected Nodes
+      </button>
+      <button
+        onClick={() => setSelectedIds([16])}
+        data-testid="select-only-vegetables"
+      >
+        Select Only Vegetables
+      </button>
+      <MultiSelectCheckboxControlled
+        selectedIds={selectedIds}
+        data={dataWithoutIds}
+      />
+    </div>
+  );
+}
+
 describe("Data without ids", () => {
   test("SelectedIds should be rendered", () => {
     const { queryAllByRole } = render(
@@ -228,6 +253,125 @@ describe("Data without ids", () => {
     expect(newNodes[19]).toHaveAttribute("aria-checked", "true");
   });
 
+  test("SelectedIds should clear previous selection including manual selection without rerendering", () => {
+    const { queryAllByRole, queryAllByTestId } = render(
+      <MultiSelectCheckboxControlledWithStateInside />
+    );
+
+    const nodes = queryAllByRole("treeitem");
+    expect(nodes[11]).toHaveAttribute("aria-checked", "true");
+
+    nodes[4].focus();
+    if (document.activeElement == null)
+      throw new Error(
+        `Expected to find an active element on the document (after focusing the second element with role["treeitem"]), but did not.`
+      );
+    fireEvent.click(nodes[4].getElementsByClassName("checkbox-icon")[0]);
+
+    expect(nodes[11]).toHaveAttribute("aria-checked", "true");
+    expect(nodes[4]).toHaveAttribute("aria-checked", "true");
+
+    fireEvent.click(queryAllByTestId("clear-selected-nodes")[0]);
+
+    const newNodes = queryAllByRole("treeitem");
+    expect(newNodes[11]).toHaveAttribute("aria-checked", "false");
+    expect(newNodes[4]).toHaveAttribute("aria-checked", "false");
+
+    [3, 4].forEach((i) => {
+      nodes[i].focus();
+      if (document.activeElement == null)
+        throw new Error(
+          `Expected to find an active element on the document (after focusing the second element with role["treeitem"]), but did not.`
+        );
+      fireEvent.click(nodes[i].getElementsByClassName("checkbox-icon")[0]);
+    });
+
+    expect(nodes[3]).toHaveAttribute("aria-checked", "true");
+    expect(nodes[4]).toHaveAttribute("aria-checked", "true");
+
+    fireEvent.click(queryAllByTestId("clear-selected-nodes")[0]);
+
+    expect(nodes[3]).toHaveAttribute("aria-checked", "false");
+    expect(nodes[4]).toHaveAttribute("aria-checked", "false");
+
+    fireEvent.click(queryAllByTestId("select-only-vegetables")[0]);
+
+    [3, 4].forEach((i) => {
+      nodes[i].focus();
+      if (document.activeElement == null)
+        throw new Error(
+          `Expected to find an active element on the document (after focusing the second element with role["treeitem"]), but did not.`
+        );
+      fireEvent.click(nodes[i].getElementsByClassName("checkbox-icon")[0]);
+    });
+
+    expect(nodes[15]).toHaveAttribute("aria-checked", "true");
+    expect(nodes[16]).toHaveAttribute("aria-checked", "true");
+    expect(nodes[17]).toHaveAttribute("aria-checked", "true");
+    expect(nodes[18]).toHaveAttribute("aria-checked", "true");
+    expect(nodes[19]).toHaveAttribute("aria-checked", "true");
+    expect(nodes[20]).toHaveAttribute("aria-checked", "true");
+    expect(nodes[3]).toHaveAttribute("aria-checked", "true");
+    expect(nodes[4]).toHaveAttribute("aria-checked", "true");
+
+    fireEvent.click(queryAllByTestId("select-only-vegetables")[0]);
+
+    expect(nodes[15]).toHaveAttribute("aria-checked", "true");
+    expect(nodes[16]).toHaveAttribute("aria-checked", "true");
+    expect(nodes[17]).toHaveAttribute("aria-checked", "true");
+    expect(nodes[18]).toHaveAttribute("aria-checked", "true");
+    expect(nodes[19]).toHaveAttribute("aria-checked", "true");
+    expect(nodes[20]).toHaveAttribute("aria-checked", "true");
+    expect(nodes[3]).toHaveAttribute("aria-checked", "false");
+    expect(nodes[4]).toHaveAttribute("aria-checked", "false");
+
+    [3, 4].forEach((i) => {
+      nodes[i].focus();
+      if (document.activeElement == null)
+        throw new Error(
+          `Expected to find an active element on the document (after focusing the second element with role["treeitem"]), but did not.`
+        );
+      fireEvent.click(nodes[i].getElementsByClassName("checkbox-icon")[0]);
+    });
+
+    fireEvent.click(queryAllByTestId("clear-selected-nodes")[0]);
+
+    expect(nodes[15]).toHaveAttribute("aria-checked", "false");
+    expect(nodes[16]).toHaveAttribute("aria-checked", "false");
+    expect(nodes[17]).toHaveAttribute("aria-checked", "false");
+    expect(nodes[18]).toHaveAttribute("aria-checked", "false");
+    expect(nodes[19]).toHaveAttribute("aria-checked", "false");
+    expect(nodes[20]).toHaveAttribute("aria-checked", "false");
+    expect(nodes[3]).toHaveAttribute("aria-checked", "false");
+    expect(nodes[4]).toHaveAttribute("aria-checked", "false");
+  });
+
+  test("SelectedIds should clear previous selection and half-selection on all nested levels", () => {
+    const { queryAllByRole, queryAllByTestId } = render(
+      <MultiSelectCheckboxControlledWithStateInside />
+    );
+
+    const nodes = queryAllByRole("treeitem");
+    expect(nodes[11]).toHaveAttribute("aria-checked", "true");
+
+    nodes[12].focus();
+    if (document.activeElement == null)
+      throw new Error(
+        `Expected to find an active element on the document (after focusing the second element with role["treeitem"]), but did not.`
+      );
+    fireEvent.click(nodes[12].getElementsByClassName("checkbox-icon")[0]);
+
+    expect(nodes[11]).toHaveAttribute("aria-checked", "true");
+    expect(nodes[10]).toHaveAttribute("aria-checked", "mixed");
+    expect(nodes[6]).toHaveAttribute("aria-checked", "mixed");
+
+    fireEvent.click(queryAllByTestId("select-only-vegetables")[0]);
+
+    expect(nodes[11]).toHaveAttribute("aria-checked", "false");
+    expect(nodes[10]).toHaveAttribute("aria-checked", "false");
+    expect(nodes[6]).toHaveAttribute("aria-checked", "false");
+  });
+
   test("SelectedIds should select all children if parent node selected", () => {
     const { queryAllByRole } = render(
       <MultiSelectCheckboxControlled selectedIds={[16]} data={dataWithoutIds} />
@@ -328,19 +472,35 @@ describe("Data without ids", () => {
     const getNodes = () => container.querySelectorAll('[role="treeitem"]');
 
     const { rerender, container } = render(
-      <MultiSelectCheckboxControlled selectedIds={[1]} data={data} defaultExpandedIds={[1,2,3]} />
+      <MultiSelectCheckboxControlled
+        selectedIds={[1]}
+        data={data}
+        defaultExpandedIds={[1, 2, 3]}
+      />
     );
 
-    getNodes().forEach(node => {
+    getNodes().forEach((node) => {
       expect(node).toHaveAttribute("aria-checked", "true");
     });
 
-    rerender(<MultiSelectCheckboxControlled selectedIds={[6]} data={data} defaultExpandedIds={[1,2,3]} />);
+    rerender(
+      <MultiSelectCheckboxControlled
+        selectedIds={[6]}
+        data={data}
+        defaultExpandedIds={[1, 2, 3]}
+      />
+    );
     expect(getNodes()[0]).toHaveAttribute("aria-checked", "mixed");
     expect(getNodes()[1]).toHaveAttribute("aria-checked", "mixed");
     expect(getNodes()[4]).toHaveAttribute("aria-checked", "true");
 
-    rerender(<MultiSelectCheckboxControlled selectedIds={[]} data={data} defaultExpandedIds={[1,2,3]} />);
+    rerender(
+      <MultiSelectCheckboxControlled
+        selectedIds={[]}
+        data={data}
+        defaultExpandedIds={[1, 2, 3]}
+      />
+    );
     expect(getNodes()[0]).toHaveAttribute("aria-checked", "false");
     expect(getNodes()[1]).toHaveAttribute("aria-checked", "false");
   });

diff --git a/website/package.json b/website/package.json
@@ -17,7 +17,7 @@
     "@reach/tooltip": "^0.2.2",
     "classnames": "^2.2.6",
     "react": "^16.8.4",
-    "react-accessible-treeview": "^2.6.3",
+    "react-accessible-treeview": "^2.7.0",
     "react-dom": "^16.8.4",
     "react-icons": "^3.7.0"
   },