select all clears selection when all are selected

Recidiviz · Oct 28, 2020 · eeb4680 · eeb4680
1 parent 15a2b43
commit eeb4680
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 10 deletions.
diff --git a/public-dashboard-client/src/controls/CohortSelect.js b/public-dashboard-client/src/controls/CohortSelect.js
@@ -131,7 +131,12 @@ function CustomSelect({
       let newSelection;
 
       if (selectedItem.id === SELECT_ALL_ID) {
-        newSelection = [...optionsFromData];
+        // if all are already selected, deselect all
+        if (selected.length === optionsFromData.length) {
+          newSelection = [];
+        } else {
+          newSelection = [...optionsFromData];
+        }
       } else {
         newSelection = [...selected];
 

diff --git a/public-dashboard-client/src/controls/CohortSelect.test.js b/public-dashboard-client/src/controls/CohortSelect.test.js
@@ -215,24 +215,41 @@ test("passes highlighted option to callback", () => {
 test("supports select-all", () => {
   const { getByRole } = openMenu();
   const selectAll = getByRole("option", { name: /select all/i });
+
   act(() => userEvent.click(selectAll));
-  // should have no effect because everything is already selected; does NOT de-select all
+
+  // de-selects all
   testOptions.forEach((opt) => {
-    expect(
-      getByRole("option", { name: opt.label, selected: true })
-    ).toBeVisible();
+    expect(getByRole("option", { name: opt.label })).toHaveAttribute(
+      "aria-selected",
+      "false"
+    );
   });
+
+  // click again to select all
+  act(() => userEvent.click(selectAll));
+  testOptions.forEach((opt) => {
+    expect(getByRole("option", { name: opt.label })).toHaveAttribute(
+      "aria-selected",
+      "true"
+    );
+  });
+
   // now de-select some manually and try again
   testOptions.slice(2, 6).forEach((opt) => {
-    act(() =>
-      userEvent.click(getByRole("option", { name: opt.label, selected: true }))
+    act(() => userEvent.click(getByRole("option", { name: opt.label })));
+    expect(getByRole("option", { name: opt.label })).toHaveAttribute(
+      "aria-selected",
+      "false"
     );
   });
+
   act(() => userEvent.click(selectAll));
   // everything is selected again
   testOptions.forEach((opt) => {
-    expect(
-      getByRole("option", { name: opt.label, selected: true })
-    ).toBeVisible();
+    expect(getByRole("option", { name: opt.label })).toHaveAttribute(
+      "aria-selected",
+      "true"
+    );
   });
 });