Fixes the RowSelect Reducer #1901 (#1925)

* Fixes the RowSelect Reducer #1901

* Delete .size-snapshot.json

* Delete useFilters.test.js.snap

Co-authored-by: boatcoder@gmail.com <Mark Jones>
Co-authored-by: Tanner Linsley <tannerlinsley@gmail.com>

.gitignore

@@ -10,5 +10,6 @@ react-table.css
 .idea
 .DS_Store
 .history
+.vscode
 package-lock.json
 coverage

src/plugin-hooks/tests/useFiltersAndRowSelect.test.js

@@ -0,0 +1,258 @@
+import React from 'react'
+import { render, fireEvent } from '@testing-library/react'
+import { useTable } from '../../hooks/useTable'
+import { useRowSelect } from '../useRowSelect'
+import { useFilters } from '../useFilters'
+
+const dataPiece = [
+  {
+    firstName: 'tanner',
+    lastName: 'linsley',
+    age: 29,
+    visits: 100,
+    status: 'In Relationship',
+    progress: 50,
+  },
+  {
+    firstName: 'derek',
+    lastName: 'perkins',
+    age: 40,
+    visits: 40,
+    status: 'Single',
+    progress: 80,
+  },
+  {
+    firstName: 'jaylen',
+    lastName: 'linsley',
+    age: 26,
+    visits: 99,
+    status: 'In Relationship',
+    progress: 70,
+  },
+]
+
+const data = [
+  ...dataPiece,
+  ...dataPiece,
+  ...dataPiece,
+  ...dataPiece,
+  ...dataPiece,
+  ...dataPiece,
+]
+
+function Table({ columns, data }) {
+  // Use the state and functions returned from useTable to build your UI
+  const {
+    getTableProps,
+    getTableBodyProps,
+    headerGroups,
+    rows,
+    prepareRow,
+    state,
+  } = useTable(
+    {
+      columns,
+      data,
+      defaultColumn,
+    },
+    useFilters,
+    useRowSelect
+  )
+
+  // Render the UI for your table
+  return (
+    <>
+      <table {...getTableProps()}>
+        <thead>
+          {headerGroups.map(headerGroup => (
+            <tr {...headerGroup.getHeaderGroupProps()}>
+              {headerGroup.headers.map(column => (
+                <th {...column.getHeaderProps()}>
+                  {column.render('Header')}
+                  {column.canFilter ? column.render('Filter') : null}
+                </th>
+              ))}
+            </tr>
+          ))}
+        </thead>
+        <tbody {...getTableBodyProps()}>
+          {rows.map(
+            (row, i) =>
+              prepareRow(row) || (
+                <tr {...row.getRowProps()}>
+                  {row.cells.map(cell => {
+                    return (
+                      <td {...cell.getCellProps()}>{cell.render('Cell')}</td>
+                    )
+                  })}
+                </tr>
+              )
+          )}
+        </tbody>
+      </table>
+      <p>
+        Selected Rows:{' '}
+        <span data-testid="selected-count">
+          {Object.keys(state.selectedRowIds).length}
+        </span>
+      </p>
+      <pre>
+        <code>
+          {JSON.stringify({ selectedRowIds: state.selectedRowIds }, null, 2)}
+        </code>
+      </pre>
+    </>
+  )
+}
+
+const defaultColumn = {
+  Cell: ({ cell: { value }, column: { id } }) => `${id}: ${value}`,
+  Filter: ({ column: { filterValue, setFilter } }) => (
+    <input
+      value={filterValue || ''}
+      onChange={e => {
+        setFilter(e.target.value || undefined) // Set undefined to remove the filter entirely
+      }}
+      placeholder="Search..."
+    />
+  ),
+}
+
+const IndeterminateCheckbox = React.forwardRef(
+  ({ indeterminate, ...rest }, ref) => {
+    const defaultRef = React.useRef()
+    const resolvedRef = ref || defaultRef
+
+    React.useEffect(() => {
+      resolvedRef.current.indeterminate = indeterminate
+    }, [resolvedRef, indeterminate])
+
+    return <input type="checkbox" ref={resolvedRef} {...rest} />
+  }
+)
+
+function App() {
+  const columns = React.useMemo(
+    () => [
+      {
+        id: 'selection',
+        // The header can use the table's getToggleAllRowsSelectedProps method
+        // to render a checkbox
+        Header: ({ getToggleAllRowsSelectedProps }) => (
+          <div>
+            <label>
+              <IndeterminateCheckbox {...getToggleAllRowsSelectedProps()} />{' '}
+              Select All
+            </label>
+          </div>
+        ),
+        // The cell can use the individual row's getToggleRowSelectedProps method
+        // to the render a checkbox
+        Cell: ({ row }) => (
+          <div>
+            <label>
+              <IndeterminateCheckbox {...row.getToggleRowSelectedProps()} />{' '}
+              Select Row
+            </label>
+          </div>
+        ),
+      },
+      {
+        id: 'selectedStatus',
+        Cell: ({ row }) => (
+          <div>
+            Row {row.id} {row.isSelected ? 'Selected' : 'Not Selected'}
+          </div>
+        ),
+      },
+      {
+        Header: 'Name',
+        columns: [
+          {
+            Header: 'First Name',
+            accessor: 'firstName',
+          },
+          {
+            Header: 'Last Name',
+            accessor: 'lastName',
+          },
+        ],
+      },
+      {
+        Header: 'Info',
+        columns: [
+          {
+            Header: 'Age',
+            accessor: 'age',
+          },
+          {
+            id: 'visits',
+            Header: 'Visits',
+            accessor: 'visits',
+            filter: 'equals',
+          },
+          {
+            Header: 'Status',
+            accessor: 'status',
+          },
+          {
+            Header: 'Profile Progress',
+            accessor: 'progress',
+          },
+        ],
+      },
+    ],
+    []
+  )
+
+  return <Table columns={columns} data={data} />
+}
+
+test('Select/Clear All while filtered only affects visible rows', () => {
+  const { getAllByPlaceholderText, getByLabelText, getByTestId } = render(
+    <App />
+  )
+  const selectedCount = getByTestId('selected-count')
+  const selectAllCheckbox = getByLabelText('Select All')
+  const fe = fireEvent
+  const filterInputs = getAllByPlaceholderText('Search...')
+
+  fireEvent.change(filterInputs[3], { target: { value: '40' } })
+  expect(selectedCount.textContent).toBe('0') // No selection has been made
+
+  expect(selectAllCheckbox.checked).toBe(false)
+  fireEvent.click(selectAllCheckbox)
+  expect(selectAllCheckbox.checked).toBe(true)
+  expect(selectedCount.textContent).toBe('6') // "Select All" has happened
+
+  // This filter hides all the rows (nothing matches it)
+  fireEvent.change(filterInputs[3], { target: { value: '10' } })
+  expect(selectedCount.textContent).toBe('6') // Filtering does not alter the selection
+
+  expect(selectAllCheckbox.checked).toBe(false) // None of the selected items are visible, this should be false
+  fireEvent.click(selectAllCheckbox) // The selection is is for no rows
+  expect(selectAllCheckbox.checked).toBe(false) // So clicking this checkbox does nothing
+  expect(selectedCount.textContent).toBe('6') // And does not affect the existing selection
+
+  fireEvent.change(filterInputs[3], { target: { value: '100' } })
+  expect(selectAllCheckbox.checked).toBe(false) // None of the selected items are visible, this should be false
+  fireEvent.click(selectAllCheckbox)
+  expect(selectAllCheckbox.checked).toBe(true) // Now all of the visible rows are ALSO selected
+  expect(selectedCount.textContent).toBe('12') // Clearing all should leave the original 6 selected
+
+  // Now deselect all the rows that match the filter
+  fireEvent.click(selectAllCheckbox)
+  expect(selectAllCheckbox.checked).toBe(false) // Now all of the visible rows are ALSO selected
+  expect(selectedCount.textContent).toBe('6') // Clearing all should leave the original 6 selected
+
+  fireEvent.change(filterInputs[3], { target: { value: '' } })
+  expect(selectedCount.textContent).toBe('6') // Clearing the Filter does not alter the selection
+
+  expect(selectAllCheckbox.checked).toBe(false) // Only a subset are selected so this button should show indeterminant
+  fireEvent.click(selectAllCheckbox)
+  expect(selectAllCheckbox.checked).toBe(true) // Now all of the visible rows are ALSO selected
+  expect(selectedCount.textContent).toBe(data.length.toString()) // Select All should select ALL of the rows
+
+  fireEvent.click(selectAllCheckbox)
+  expect(selectedCount.textContent).toBe('0') // Select All should now clear ALL of the rows
+})

src/plugin-hooks/useRowSelect.js

@@ -95,22 +95,23 @@ function reducer(state, action, previousState, instance) {
     const selectAll =
       typeof setSelected !== 'undefined' ? setSelected : !isAllRowsSelected
 
-    if (selectAll) {
-      const selectedRowIds = {}
+    // Only remove/add the rows that are visible on the screen
+    //  Leave all the other rows that are selected alone.
+    const selectedRowIds = Object.assign({}, state.selectedRowIds)
 
+    if (selectAll) {
       Object.keys(nonGroupedRowsById).forEach(rowId => {
         selectedRowIds[rowId] = true
       })
-
-      return {
-        ...state,
-        selectedRowIds,
-      }
+    } else {
+      Object.keys(flatRowsById).forEach(rowId => {
+        delete selectedRowIds[rowId]
+      })
     }
 
     return {
       ...state,
-      selectedRowIds: {},
+      selectedRowIds,
     }
   }
 
@@ -130,7 +131,7 @@ function reducer(state, action, previousState, instance) {
       return state
     }
 
-    let newSelectedRowIds = { ...state.selectedRowIds }
+    const newSelectedRowIds = { ...state.selectedRowIds }
 
     const handleRowById = id => {
       const row = rowsById[id]

yarn.lock

@@ -756,7 +756,11 @@
   resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.8.4.tgz#d79f5a2040f7caa24d53e563aad49cbc05581308"
   integrity sha512-neAp3zt80trRVBI1x0azq6c57aNBqYZH8KhMm3TaB7wEI5Q4A2SHfBHE8w9gOhI/lrqxtEbXZgQIrHP+wvSGwQ==
   dependencies:
-    regenerator-runtime "^0.13.2"
+    "@babel/helper-plugin-utils" "^7.8.3"
+    "@babel/plugin-transform-react-display-name" "^7.8.3"
+    "@babel/plugin-transform-react-jsx" "^7.8.3"
+    "@babel/plugin-transform-react-jsx-self" "^7.8.3"
+    "@babel/plugin-transform-react-jsx-source" "^7.8.3"
 
 "@babel/template@^7.4.0", "@babel/template@^7.8.3":
   version "7.8.3"
@@ -1679,7 +1683,7 @@ ajv@6.5.3:
   resolved "https://registry.yarnpkg.com/ajv/-/ajv-6.5.3.tgz#71a569d189ecf4f4f321224fecb166f071dd90f9"
   integrity sha512-LqZ9wY+fx3UMiiPd741yB2pj3hhil+hQc8taf4o2QGRFpWgZ2V5C8HA165DY9sS3fJwsk7uT7ZlFEyC3Ig3lLg==
   dependencies:
-    fast-deep-equal "^2.0.1"
+    fast-deep-equal "^3.1.1"
     fast-json-stable-stringify "^2.0.0"
     json-schema-traverse "^0.4.1"
     uri-js "^4.2.2"
@@ -3890,10 +3894,10 @@ extsprintf@^1.2.0:
   resolved "https://registry.yarnpkg.com/extsprintf/-/extsprintf-1.4.0.tgz#e2689f8f356fad62cca65a3a91c5df5f9551692f"
   integrity sha1-4mifjzVvrWLMplo6kcXfX5VRaS8=
 
-fast-deep-equal@^2.0.1:
-  version "2.0.1"
-  resolved "https://registry.yarnpkg.com/fast-deep-equal/-/fast-deep-equal-2.0.1.tgz#7b05218ddf9667bf7f370bf7fdb2cb15fdd0aa49"
-  integrity sha1-ewUhjd+WZ79/Nwv3/bLLFf3Qqkk=
+fast-deep-equal@^3.1.1:
+  version "3.1.1"
+  resolved "https://registry.yarnpkg.com/fast-deep-equal/-/fast-deep-equal-3.1.1.tgz#545145077c501491e33b15ec408c294376e94ae4"
+  integrity sha512-8UEa58QDLauDNfpbrX55Q9jrGHThw2ZMdOky5Gl1CDtVeJDPVrG4Jxx1N8jw2gkWaff5UUuX1KJd+9zGe2B+ZA==
 
 fast-deep-equal@^3.1.1:
   version "3.1.1"
@@ -3968,9 +3972,9 @@ figures@^2.0.0:
     escape-string-regexp "^1.0.5"
 
 figures@^3.0.0:
-  version "3.1.0"
-  resolved "https://registry.yarnpkg.com/figures/-/figures-3.1.0.tgz#4b198dd07d8d71530642864af2d45dd9e459c4ec"
-  integrity sha512-ravh8VRXqHuMvZt/d8GblBeqDMkdJMBdv/2KntFH+ra5MXkO7nxNKpzQ3n6QD/2da1kH0aWmNISdvhM7gl2gVg==
+  version "3.2.0"
+  resolved "https://registry.yarnpkg.com/figures/-/figures-3.2.0.tgz#625c18bd293c604dc4a8ddb2febf0c88341746af"
+  integrity sha512-yaduQFRKLXYOGgEn6AZau90j3ggSOyiqXU0F9JZfeXYhNa+Jk4X+s45A2zg5jns87GAFa34BBm2kXw4XpNcbdg==
   dependencies:
     escape-string-regexp "^1.0.5"
 
@@ -4188,6 +4192,11 @@ gensync@^1.0.0-beta.1:
   resolved "https://registry.yarnpkg.com/gensync/-/gensync-1.0.0-beta.1.tgz#58f4361ff987e5ff6e1e7a210827aa371eaac269"
   integrity sha512-r8EC6NO1sngH/zdD9fiRDLdcgnbayXah+mLgManTaIZJqEC1MZstmnox8KpnI2/fxQwrp5OpCOYWLp4rBl4Jcg==
 
+gensync@^1.0.0-beta.1:
+  version "1.0.0-beta.1"
+  resolved "https://registry.yarnpkg.com/gensync/-/gensync-1.0.0-beta.1.tgz#58f4361ff987e5ff6e1e7a210827aa371eaac269"
+  integrity sha512-r8EC6NO1sngH/zdD9fiRDLdcgnbayXah+mLgManTaIZJqEC1MZstmnox8KpnI2/fxQwrp5OpCOYWLp4rBl4Jcg==
+
 get-caller-file@^1.0.1:
   version "1.0.3"
   resolved "https://registry.yarnpkg.com/get-caller-file/-/get-caller-file-1.0.3.tgz#f978fa4c90d1dfe7ff2d6beda2a515e713bdcf4a"
@@ -6878,7 +6887,7 @@ prompts@^2.0.1, prompts@^2.2.1:
   integrity sha512-NfbbPPg/74fT7wk2XYQ7hAIp9zJyZp5Fu19iRbORqqy1BhtrkZ0fPafBU+7bmn8ie69DpT0R6QpJIN2oisYjJg==
   dependencies:
     kleur "^3.0.3"
-    sisteransi "^1.0.3"
+    sisteransi "^1.0.4"
 
 prop-types@^15.5.0, prop-types@^15.6.2, prop-types@^15.7.2:
   version "15.7.2"