Merge pull request plotly#164 from plotly/118-ascending-sort

Sorting sorts ascending first and displays correct icon
HammadTheOne · Oct 24, 2018 · a580bd0 · a580bd0
1 parent 7b1b5a1
commit a580bd0
Show file tree

Hide file tree

Showing 12 changed files with 36 additions and 30 deletions.
diff --git a/packages/dash-table/CHANGELOG.md b/packages/dash-table/CHANGELOG.md
@@ -399,4 +399,8 @@ Derived properties allow the component to expose complex state that can be usefu
 
 ## RC8 - Improve props typing
 
-    Issue: https://github.com/plotly/dash-table/issues/143    
+    Issue: https://github.com/plotly/dash-table/issues/143    
+
+## RC9 - Sort ascending on first click 
+    - Sorts ascending when first clicked, [#118](https://github.com/plotly/dash-table/issues/118)
+    - Flips icons displayed so that they are pointing up on ascending and down on descending.
diff --git a/packages/dash-table/dash_table/bundle.js b/packages/dash-table/dash_table/bundle.js
diff --git a/packages/dash-table/dash_table/demo.js b/packages/dash-table/dash_table/demo.js
diff --git a/packages/dash-table/dash_table/package.json b/packages/dash-table/dash_table/package.json
@@ -1,6 +1,6 @@
 {
   "name": "dash-table",
-  "version": "3.1.0rc8",
+  "version": "3.1.0rc9",
   "description": "Dash table",
   "main": "build/index.js",
   "scripts": {

diff --git a/packages/dash-table/package.json b/packages/dash-table/package.json
@@ -1,6 +1,6 @@
 {
   "name": "dash-table",
-  "version": "3.1.0rc8",
+  "version": "3.1.0rc9",
   "description": "Dash table",
   "main": "build/index.js",
   "scripts": {

diff --git a/packages/dash-table/src/dash-table/Table.js b/packages/dash-table/src/dash-table/Table.js
@@ -8,6 +8,14 @@ import Logger from 'core/Logger';
 
 import genRandomId from './utils/generate';
 
+function isFrontEnd(value) {
+    return ['fe', true, false].indexOf(value) !== -1;
+}
+
+function isBackEnd(value) {
+    return ['be', false].indexOf(value) !== -1;
+}
+
 export default class Table extends Component {
     constructor(props) {
         super(props);
@@ -23,14 +31,6 @@ export default class Table extends Component {
             pagination_mode
         } = this.props;
 
-        function isFrontEnd(value: any) {
-            return ['fe', true, false].indexOf(value) !== -1;
-        }
-
-        function isBackEnd(value: any) {
-            return ['be', false].indexOf(value) !== -1;
-        }
-
         const isValid = isFrontEnd(pagination_mode) ||
             (isBackEnd(filtering) && isBackEnd(sorting));
 

diff --git a/packages/dash-table/src/dash-table/derived/header/content.tsx b/packages/dash-table/src/dash-table/derived/header/content.tsx
@@ -29,14 +29,16 @@ function doSort(columnId: ColumnId, sortSettings: SortSettings, sortType: Sortin
         let direction: SortDirection;
         switch (getSorting(columnId, sortSettings)) {
             case SortDirection.Descending:
-                direction = SortDirection.Ascending;
+                direction = SortDirection.None;
                 break;
             case SortDirection.Ascending:
-                direction = SortDirection.None;
+                direction = SortDirection.Descending;
                 break;
             case SortDirection.None:
+                direction = SortDirection.Ascending;
+                break;
             default:
-                direction = SortDirection.Descending;
+                direction = SortDirection.Ascending;
                 break;
         }
 
@@ -68,9 +70,9 @@ function getSorting(columnId: ColumnId, settings: SortSettings): SortDirection {
 function getSortingIcon(columnId: ColumnId, sortSettings: SortSettings) {
     switch (getSorting(columnId, sortSettings)) {
         case SortDirection.Descending:
-            return '↑';
-        case SortDirection.Ascending:
             return '↓';
+        case SortDirection.Ascending:
+            return '↑';
         case SortDirection.None:
         default:
             return '↕';

diff --git a/packages/dash-table/tests/cypress/tests/server/copy_paste_test.ts b/packages/dash-table/tests/cypress/tests/server/copy_paste_test.ts
@@ -101,7 +101,7 @@ describe('copy paste', () => {
             cy.get('tr th.column-2 .sort').last().click();
 
             DashTable.getCell(0, 0).click();
-            DashTable.getCell(0, 0).within(() => cy.get('.dash-cell-value').should('have.value', '103'));
+            DashTable.getCell(0, 0).within(() => cy.get('.dash-cell-value').should('have.value', '11'));
 
             DOM.focused.type(`${Key.Meta}c`);
 
@@ -113,7 +113,7 @@ describe('copy paste', () => {
                 .within(() => cy.get('.dash-cell-value').should('have.html', 'MODIFIED'));
             DashTable
                 .getCell(1, 0)
-                .within(() => cy.get('.dash-cell-value').should('have.value', '103'));
+                .within(() => cy.get('.dash-cell-value').should('have.value', '11'));
 
             DashTable.getCell(1, 1).click();
             DOM.focused.type(`${Key.Meta}c`);
@@ -126,7 +126,7 @@ describe('copy paste', () => {
                 .within(() => cy.get('.dash-cell-value').should('have.value', 'MODIFIED'));
             DashTable
                 .getCell(1, 0)
-                .within(() => cy.get('.dash-cell-value').should('have.html', '103'));
+                .within(() => cy.get('.dash-cell-value').should('have.html', '11'));
         });
     });
 });
diff --git a/packages/dash-table/tests/cypress/tests/server/delete_row_test.ts b/packages/dash-table/tests/cypress/tests/server/delete_row_test.ts
@@ -10,7 +10,7 @@ describe('delete', () => {
     });
 
     it('can delete row when sorted', () => {
-        cy.get('tr th.column-0 .sort').last().click({ force: true });
+        cy.get('tr th.column-0 .sort').last().click({ force: true }).click({ force: true });
         DashTable.getCell(0, 0).within(() => cy.get('.dash-cell-value').should('have.html', '28155'));
         DashTable.getDelete(0).click();
         DashTable.getCell(0, 0).within(() => cy.get('.dash-cell-value').should('have.html', '28154'));

diff --git a/packages/dash-table/tests/cypress/tests/standalone/delete_row_test.ts b/packages/dash-table/tests/cypress/tests/standalone/delete_row_test.ts
@@ -10,7 +10,7 @@ describe('delete', () => {
     });
 
     it('can delete row when sorted', () => {
-        cy.get('tr th.column-0 .sort').last().click({ force: true });
+        cy.get('tr th.column-0 .sort').last().click({ force: true }).click({ force: true });
         DashTable.getCell(0, 0).within(() => cy.get('.dash-cell-value').should('have.html', '4999'));
         DashTable.getDelete(0).click();
         DashTable.getCell(0, 0).within(() => cy.get('.dash-cell-value').should('have.html', '4998'));

diff --git a/packages/dash-table/tests/cypress/tests/standalone/select_row_test.ts b/packages/dash-table/tests/cypress/tests/standalone/select_row_test.ts
@@ -18,7 +18,7 @@ describe('select row', () => {
         it('select, sort, new row is not selected', () => {
             DashTable.getSelect(0).within(() => cy.get('input').click());
             DashTable.getSelect(0).within(() => cy.get('input').should('be.checked'));
-            cy.get('tr th.column-0 .sort').last().click({ force: true });
+            cy.get('tr th.column-0 .sort').last().click({ force: true }).click({ force: true });
             DashTable.getSelect(0).within(() => cy.get('input').should('not.be.checked'));
         });
     });

diff --git a/packages/dash-table/tests/cypress/tests/standalone/sorting_test.ts b/packages/dash-table/tests/cypress/tests/standalone/sorting_test.ts
@@ -11,9 +11,9 @@ describe('sort', () => {
         DashTable.getCell(2, 6).within(() => cy.get('.Select-value-label').should('have.html', 'Tropical Beaches'));
         DashTable.getCell(3, 6).within(() => cy.get('.Select-value-label').should('have.html', 'Humid'));
         cy.get('tr th.column-6 .sort').last().click();
-        DashTable.getCell(0, 6).within(() => cy.get('.Select-value-label').should('have.html', 'Wet'));
-        DashTable.getCell(1, 6).within(() => cy.get('.Select-value-label').should('have.html', 'Wet'));
-        DashTable.getCell(2, 6).within(() => cy.get('.Select-value-label').should('have.html', 'Wet'));
-        DashTable.getCell(3, 6).within(() => cy.get('.Select-value-label').should('have.html', 'Wet'));
+        DashTable.getCell(0, 6).within(() => cy.get('.Select-value-label').should('have.html', 'Humid'));
+        DashTable.getCell(1, 6).within(() => cy.get('.Select-value-label').should('have.html', 'Humid'));
+        DashTable.getCell(2, 6).within(() => cy.get('.Select-value-label').should('have.html', 'Humid'));
+        DashTable.getCell(3, 6).within(() => cy.get('.Select-value-label').should('have.html', 'Humid'));
     });
 });