Fix createMultiSort bug (#1051)

* Fix bug to clear all non-selected key-value pairs from sortDirection object when only one sort column is selected * Add tests for resetting sort-direction fields in createMultiSort function * Add test to verify the ability to shift+click more than once
bvaughn · Jul 27, 2018 · c646a62 · c646a62
1 parent 0e7680b
commit c646a62
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 0 deletions.
diff --git a/source/Table/createMultiSort.jest.js b/source/Table/createMultiSort.jest.js
@@ -86,6 +86,29 @@ describe('createMultiSort', () => {
       simulate(multiSort.sort, 'a');
       expect(multiSort.sortBy).toEqual(['a']);
     });
+
+    it('resets sort-direction fields', () => {
+      const multiSort = createMultiSort(jest.fn(), {
+        defaultSortBy: ['a', 'b'],
+        defaultSortDirection: {
+          a: 'DESC',
+          b: 'ASC',
+        },
+      });
+      expect(multiSort.sortBy).toEqual(['a', 'b']);
+      expect(multiSort.sortDirection.a).toEqual('DESC');
+      expect(multiSort.sortDirection.b).toEqual('ASC');
+
+      simulate(multiSort.sort, 'a');
+      expect(multiSort.sortBy).toEqual(['a']);
+      expect(multiSort.sortDirection.a).toEqual('ASC');
+      expect(multiSort.sortDirection.b).toEqual(undefined);
+
+      simulate(multiSort.sort, 'b');
+      expect(multiSort.sortBy).toEqual(['b']);
+      expect(multiSort.sortDirection.a).toEqual(undefined);
+      expect(multiSort.sortDirection.b).toEqual('ASC');
+    });
   });
 
   describe('on shift click', () => {
@@ -124,6 +147,28 @@ describe('createMultiSort', () => {
       expect(multiSort.sortDirection.a).toBe('ASC');
       expect(multiSort.sortDirection.b).toBe('ASC');
     });
+
+    it('able to shift+click more than once', () => {
+      const multiSort = createMultiSort(jest.fn());
+
+      simulate(multiSort.sort, 'a');
+      expect(multiSort.sortBy).toEqual(['a']);
+      expect(multiSort.sortDirection.a).toBe('ASC');
+
+      simulate(multiSort.sort, 'b', 'shift');
+      expect(multiSort.sortBy).toEqual(['a', 'b']);
+      expect(multiSort.sortDirection.a).toBe('ASC');
+      expect(multiSort.sortDirection.b).toBe('ASC');
+
+      simulate(multiSort.sort, 'b');
+      expect(multiSort.sortBy).toEqual(['b']);
+      expect(multiSort.sortDirection.b).toBe('DESC');
+
+      simulate(multiSort.sort, 'a', 'shift');
+      expect(multiSort.sortBy).toEqual(['b', 'a']);
+      expect(multiSort.sortDirection.a).toBe('ASC');
+      expect(multiSort.sortDirection.b).toBe('DESC');
+    });
   });
 
   ['control', 'meta'].forEach(modifier => {

diff --git a/source/Table/createMultiSort.js b/source/Table/createMultiSort.js
@@ -73,9 +73,18 @@ export default function createMultiSort(
         delete sortDirection[dataKey];
       }
     } else {
+      // Clear sortBy array of all non-selected keys
       sortBy.length = 0;
       sortBy.push(dataKey);
 
+      // Clear sortDirection object of all non-selected keys
+      const sortDirectionKeys = Object.keys(sortDirection);
+      sortDirectionKeys.forEach(key => {
+        if (key !== dataKey) delete sortDirection[key];
+      });
+
+      // If key is already selected, reverse sort direction.
+      // Else, set sort direction to default direction.
       if (sortDirection.hasOwnProperty(dataKey)) {
         sortDirection[dataKey] =
           sortDirection[dataKey] === 'ASC' ? 'DESC' : 'ASC';