Add sparse sorting to DataTable (#3749)

* Co-authored-by: Josh Black <joshblack@github.com>

* Add another value for blank equality comparison

* Update sortBy comment

* chore: add changeset

---------

Co-authored-by: Josh Black <joshblack@users.noreply.github.com>

Author: thyeggman

.changeset/metal-horses-teach.md

@@ -0,0 +1,7 @@
+---
+'@primer/react': minor
+---
+
+Add suport for sparse sorting to DataTable
+
+<!-- Changed components: DataTable -->

src/DataTable/__tests__/DataTable.test.tsx

@@ -551,6 +551,81 @@ describe('DataTable', () => {
       })
     })
 
+    it('should sort sparsely populated columns with blank values at the end', async () => {
+      const user = userEvent.setup()
+
+      render(
+        <DataTable
+          data={[
+            {
+              id: 1,
+              value: null,
+            },
+            {
+              id: 2,
+              value: 2,
+            },
+            {
+              id: 3,
+              value: '',
+            },
+            {
+              id: 4,
+              value: 3,
+            },
+            {
+              id: 5,
+              value: 1,
+            },
+          ]}
+          columns={[
+            {
+              header: 'Value',
+              field: 'value',
+              sortBy: true,
+            },
+          ]}
+          initialSortColumn="value"
+          initialSortDirection="ASC"
+        />,
+      )
+
+      const header = screen.getByRole('columnheader', {
+        name: 'Value',
+      })
+      expect(header).toHaveAttribute('aria-sort', 'ascending')
+
+      // Change to descending
+      await user.click(screen.getByText('Value'))
+
+      let rows = screen
+        .getAllByRole('row')
+        .filter(row => {
+          return queryByRole(row, 'cell')
+        })
+        .map(row => {
+          const cell = getByRole(row, 'cell')
+          return cell.textContent
+        })
+
+      expect(rows).toEqual(['3', '2', '1', '', ''])
+
+      // Change to ascending
+      await user.click(screen.getByText('Value'))
+
+      rows = screen
+        .getAllByRole('row')
+        .filter(row => {
+          return queryByRole(row, 'cell')
+        })
+        .map(row => {
+          const cell = getByRole(row, 'cell')
+          return cell.textContent
+        })
+
+      expect(rows).toEqual(['1', '2', '3', '', ''])
+    })
+
     it('should change the sort direction on mouse click', async () => {
       const user = userEvent.setup()
       render(

src/DataTable/useTable.ts

@@ -116,7 +116,8 @@ export function useTable<Data extends UniqueRow>({
   }
 
   /**
-   * Sort the rows of a table with the given column sort state
+   * Sort the rows of a table with the given column sort state. If the data in the table is sparse,
+   * blank values will be ordered last regardless of the sort direction.
    */
   function sortRows(state: Exclude<ColumnSortState, null>) {
     const header = headers.find(header => {
@@ -156,12 +157,23 @@ export function useTable<Data extends UniqueRow>({
         const valueA = get(a, header.column.field)
         const valueB = get(b, header.column.field)
 
-        if (state.direction === SortDirection.ASC) {
+        if (valueA && valueB) {
+          if (state.direction === SortDirection.ASC) {
+            // @ts-ignore todo
+            return sortMethod(valueA, valueB)
+          }
           // @ts-ignore todo
-          return sortMethod(valueA, valueB)
+          return sortMethod(valueB, valueA)
+        }
+
+        if (valueA) {
+          return -1
+        }
+
+        if (valueB) {
+          return 1
         }
-        // @ts-ignore todo
-        return sortMethod(valueB, valueA)
+        return 0
       })
     })
   }