From c646a62eb3f7aebdfe648bb71cb87545af4d3a9b Mon Sep 17 00:00:00 2001
From: Lydia Cheung <lideeyaa@gmail.com>
Date: Fri, 27 Jul 2018 02:03:15 -0400
Subject: [PATCH] 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
---
 source/Table/createMultiSort.jest.js | 45 ++++++++++++++++++++++++++++
 source/Table/createMultiSort.js      |  9 ++++++
 2 files changed, 54 insertions(+)

diff --git a/source/Table/createMultiSort.jest.js b/source/Table/createMultiSort.jest.js
index 9e6fd47e9..9a7f14660 100644
--- 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
index 512040ba5..fbfbbb764 100644
--- 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';