Clean up sorting logic (#20)

* fix(sorting): fix sorting bug when data changes, sorting is not preserved

* remove console log messages

* clean up logic in set_rows clause

* fix(sorting): fix sorting code to be more straightforward and add coments

Author: Buuntu

src/hooks.tsx

@@ -22,6 +22,7 @@ const createReducer = <T extends DataType>() => (
   switch (action.type) {
     case 'SET_ROWS':
       let rows = [...action.data];
+      // preserve sorting if a sort is already enabled when data changes
       if (state.sortColumn) {
         rows = sortByColumn(action.data, state.sortColumn, state.columns);
       }
@@ -34,9 +35,17 @@ const createReducer = <T extends DataType>() => (
         );
       }
 
+      if (state.paginationEnabled === true) {
+        rows = getPaginatedData(
+          rows,
+          state.pagination.perPage,
+          state.pagination.page
+        );
+      }
+
       return {
         ...state,
-        rows: rows,
+        rows,
         originalRows: action.data,
       };
 
@@ -90,7 +99,9 @@ const createReducer = <T extends DataType>() => (
       const columnCopy = state.columns.map(column => {
         // if the row was found
         if (action.columnName === column.name) {
-          isAscending = column.sorted.asc;
+          // if it's undefined, start by setting to ascending, otherwise toggle
+          isAscending =
+            column.sorted.asc === undefined ? true : !column.sorted.asc;
           if (column.sort) {
             sortedRows = isAscending
               ? state.rows.sort(column.sort)
@@ -109,15 +120,16 @@ const createReducer = <T extends DataType>() => (
             ...column,
             sorted: {
               on: true,
-              asc: !column.sorted.asc,
+              asc: isAscending,
             },
           };
         }
+        // set sorting to false for all other columns
         return {
           ...column,
           sorted: {
             on: false,
-            asc: true,
+            asc: false,
           },
         };
       });
@@ -238,7 +250,6 @@ export const useTable = <T extends DataType>(
           sort: column.sort,
           sorted: {
             on: false,
-            asc: true,
           },
         };
       }),
@@ -379,12 +390,12 @@ const sortByColumn = <T extends DataType>(
 ): RowType<T>[] => {
   let isAscending = null;
   let sortedRows: RowType<T>[] = [...data];
-  // loop through all columns and set the sort parameter to off unless
-  // it's the specified column (only one column at a time for )
+
   columns.map(column => {
     // if the row was found
     if (sortColumn === column.name) {
-      isAscending = !column.sorted.asc;
+      isAscending = column.sorted.asc;
+
       if (column.sort) {
         sortedRows = isAscending
           ? data.sort(column.sort)

src/test/sorting.spec.tsx

@@ -118,12 +118,14 @@ test('Should render a table and preserve sorting when data changes', () => {
   ({ getByText } = within(firstRow));
   expect(getByText('Faulkner')).toBeInTheDocument();
 
+  fireEvent.click(firstNameColumn);
+
+  ({ getByText } = within(firstRow));
+  expect(getByText('Yesenia')).toBeInTheDocument();
+
   fireEvent.click(addRowButton);
 
   // expect(screen.queryAllByRole('table-row')).toHaveLength(12);
-
-  ({ getByText } = within(firstRow));
-  expect(getByText('Faulkner')).toBeInTheDocument();
 });
 
 test('Should sort by dates correctly', () => {

src/types.ts

@@ -14,7 +14,7 @@ export type ColumnStateType<T> = {
   sort?: ((a: RowType<T>, b: RowType<T>) => number) | undefined;
   sorted: {
     on: boolean;
-    asc: boolean;
+    asc?: boolean;
   };
   headerRender?: HeaderRenderType;
 };
@@ -28,7 +28,7 @@ export type HeaderType<T> = {
   hidden?: boolean;
   sorted: {
     on: boolean;
-    asc: boolean;
+    asc?: boolean;
   };
   sort?: ((a: RowType<T>, b: RowType<T>) => number) | undefined;
   render: () => React.ReactNode;