Merge pull request #152 from bindable-ui/djedi/hotfix/table-sort-but-fx

Fix table sorting bug with reverseSort

Author: djedi

dev-app/app.html

@@ -52,7 +52,7 @@
                     <c-nav-horizontal-item
                         position="right"
                         href="https://github.com/bindable-ui/bindable"
-                        title="v1.11.8"
+                        title="v1.11.10"
                     ></c-nav-horizontal-item>
                 </c-nav-horizontal>
             </l-box>

dev-app/routes/components/tables/table/properties/index.ts

@@ -155,30 +155,41 @@ export class TableProperties {
                 colHeadName: 'gender',
                 colHeadValue: 'Gender',
             },
+            {
+                colClass: 't85',
+                colHeadName: 'number',
+                colHeadValue: 'revSort',
+                sort: true,
+                reverseSort: true,
+            },
         ];
 
         this.sortableData = [
             {
                 gender: 'Male',
                 name: 'Luke Skywalker',
                 ship: 'X-Wing',
+                number: 4,
             },
             {
                 _status: 'critical',
                 gender: 'Male',
                 name: 'Fin',
                 ship: 'Tie Fighter',
+                number: 2,
             },
             {
                 gender: 'Male',
                 name: 'Han Solo',
                 ship: 'M. Falcon',
+                number: 3,
             },
             {
                 _status: 'healthy',
                 gender: 'Female',
                 name: 'Rey',
                 ship: 'M. Falcon',
+                number: 1,
             },
         ];
     }

package.json

@@ -1,7 +1,7 @@
 {
   "name": "@bindable-ui/bindable",
   "description": "An Aurelia component library",
-  "version": "1.11.9",
+  "version": "1.11.10",
   "repository": {
     "type": "git",
     "url": "https://github.com/bindable-ui/bindable"

src/components/tables/c-table/c-table.test.ts

@@ -579,7 +579,7 @@ describe('c-table component', () => {
                 expect(col.sortClass).toBe(component.styles.sortDesc);
             });
 
-            it('will sort desc when col reverseSort', () => {
+            it('will sort asc still when col reverseSort', () => {
                 const col: any = {
                     _class: 'code',
                     _status: 'warning',
@@ -594,25 +594,7 @@ describe('c-table component', () => {
 
                 component.setColSortClass(col);
 
-                expect(col.sortClass).toBe(component.styles.sortDesc);
-            });
-
-            it('will sort desc when table reverseSort', () => {
-                const col: any = {
-                    _class: 'code',
-                    _status: 'warning',
-                    colClass: 't150',
-                    colHeadName: 'name',
-                    colHeadValue: 'Name',
-                    sort: true,
-                };
-
-                component.reverseSort = true;
-                component.defaultSortCol = `${col.colHeadName}`;
-
-                component.setColSortClass(col);
-
-                expect(col.sortClass).toBe(component.styles.sortDesc);
+                expect(col.sortClass).toBe(component.styles.sortAsc);
             });
         });
 

src/components/tables/c-table/c-table.ts

@@ -190,19 +190,18 @@ export class CTable {
         }
 
         let sortClass = this.styles.sortNone;
-        const isReverse = /^-/.test(this.defaultSortCol);
+        const isDescending = /^-/.test(this.defaultSortCol);
         const defaultSortCol = this.defaultSortCol.replace(/^-/, '');
 
+        // I think the classes are backwards
+        const asc = this.styles.sortAsc;
+        const desc = this.styles.sortDesc;
+
         if (col.colHeadName === defaultSortCol) {
+            const reversedColumn: boolean = Boolean(col.reverseSort);
             const {reverseSort = false} = col;
 
-            if (isReverse) {
-                sortClass = this.styles.sortDesc;
-            } else if (reverseSort) {
-                sortClass = col.reverseSort ? this.styles.sortDesc : this.styles.sortAsc;
-            } else {
-                sortClass = this.reverseSort ? this.styles.sortDesc : this.styles.sortAsc;
-            }
+            sortClass = isDescending ? desc : asc;
         }
 
         col.sortClass = sortClass;
@@ -217,21 +216,26 @@ export class CTable {
         });
 
         let reverse: boolean;
+        const reversedColumn: boolean = Boolean(col.reverseSort);
+
+        const asc = this.styles.sortAsc;
+        const desc = this.styles.sortDesc;
+
         // Use reverseSort property of column or reverse-sort binding of table (in that order)
         // to determine initial sort. After initial sort, just toggle.
         if (col.sortClass === this.styles.sortNone) {
-            if (typeof col.reverseSort === 'boolean') {
-                col.sortClass = col.reverseSort ? this.styles.sortDesc : this.styles.sortAsc;
-                reverse = col.reverseSort;
+            if (reversedColumn) {
+                col.sortClass = desc;
+                reverse = true;
             } else {
-                col.sortClass = this.reverseSort ? this.styles.sortDesc : this.styles.sortAsc;
-                reverse = this.reverseSort;
+                col.sortClass = asc;
+                reverse = false;
             }
-        } else if (col.sortClass === this.styles.sortAsc) {
-            col.sortClass = this.styles.sortDesc;
+        } else if (col.sortClass === asc) {
+            col.sortClass = desc;
             reverse = true;
         } else {
-            col.sortClass = this.styles.sortAsc;
+            col.sortClass = asc;
             reverse = false;
         }
         if (this.actions && this.actions.sortColumn) {

yarn.lock

@@ -2217,9 +2217,9 @@ caniuse-api@^3.0.0:
     lodash.uniq "^4.5.0"
 
 caniuse-lite@^1.0.0, caniuse-lite@^1.0.30001109, caniuse-lite@^1.0.30001317:
-  version "1.0.30001328"
-  resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001328.tgz#0ed7a2ca65ec45872c613630201644237ba1e329"
-  integrity sha512-Ue55jHkR/s4r00FLNiX+hGMMuwml/QGqqzVeMQ5thUewznU2EdULFvI3JR7JJid6OrjJNfFvHY2G2dIjmRaDDQ==
+  version "1.0.30001436"
+  resolved "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001436.tgz"
+  integrity sha512-ZmWkKsnC2ifEPoWUvSAIGyOYwT+keAaaWPHiQ9DfMqS1t6tfuyFYoWR78TeZtznkEQ64+vGXH9cZrElwR2Mrxg==
 
 capture-exit@^2.0.0:
   version "2.0.0"