Fix: Table column reordering + always render sort icons (#1479)

yavin-dev · Sep 28, 2021 · 0c84ab8 · 0c84ab8
1 parent ca01788
commit 0c84ab8
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 51 deletions.
diff --git a/packages/core/addon/components/navi-table-sort-icon.hbs b/packages/core/addon/components/navi-table-sort-icon.hbs
@@ -1,5 +1,5 @@
 {{!-- Copyright 2021, Yahoo Holdings Inc. Licensed under the terms of the MIT license. See accompanying LICENSE.md file for terms. --}}
-<span class="navi-table-sort-icon {{this.sortClass}}" ...attributes>
-  <NaviIcon @icon="sort-asc" class="navi-table-sort-icon__caret sort-icon__caret-up" />
-  <NaviIcon @icon="sort-desc" class="navi-table-sort-icon__caret sort-icon__caret-down" />
+<span class="navi-table-sort-icon {{this.sortClass}} flex flex-column p-l-3" ...attributes>
+  <DenaliIcon @icon="arrowhead-up-solid" @size="extrasmall" class="navi-table-sort-icon__caret navi-table-sort-icon__caret-up" />
+  <DenaliIcon @icon="arrowhead-down-solid" @size="extrasmall" class="navi-table-sort-icon__caret navi-table-sort-icon__caret-down" />
 </span>
diff --git a/packages/core/addon/components/navi-visualizations/table.ts b/packages/core/addon/components/navi-visualizations/table.ts
@@ -290,12 +290,17 @@ export default class Table extends Component<Args> {
   }
 
   /**
-   * sends sort action when timeDimension header is clicked
-   * @param column clicked table column
+   * sends sort action when column header is clicked
    */
   @action
-  headerClicked(column: TableColumn): void {
-    if (!column.fragment.columnMetadata.isSortable) {
+  headerClicked({ target }: { target: HTMLElement }): void {
+    const columnId = target.closest('li')?.getAttribute('data-column-id');
+    if (!columnId) {
+      return;
+    }
+
+    const column = this.columns.find((c) => c.columnId === columnId);
+    if (!column?.fragment.columnMetadata.isSortable) {
       return;
     }
 

diff --git a/packages/core/addon/templates/components/table-renderer-vertical-collection.hbs b/packages/core/addon/templates/components/table-renderer-vertical-collection.hbs
@@ -2,14 +2,13 @@
 <div class={{this.tableHeadersClass}}>
   {{#unless @isEditingMode}}
     {{#let "table-renderer-vc" as |groupName|}}
-      <ol class="table-header-row-vc table-header-row-vc--view" {{sortable-group groupName=groupName onChange=@updateColumnOrder direction="x"}}>
+      <ol role="button" class="table-header-row-vc table-header-row-vc--view" {{sortable-group groupName=groupName onChange=@updateColumnOrder direction="x"}} {{on "click" @headerClicked}}>
         {{#each @columns as |column|}}
           <li
             class="table-header-cell {{column.fragment.type}}"
             data-name={{column.fragment.canonicalName}}
-            role="button"
+            data-column-id={{column.columnId}}
             {{sortable-handle}}
-            {{on "click" (fn @headerClicked column)}}
             {{sortable-item groupName=groupName model=column isDraggingDisabled=@isDraggingDisabled onDragStart=(set this "isDragged" true) onDragStop=(set this "isDragged" false)}}
           >
             <span

diff --git a/packages/core/addon/templates/components/table-renderer.hbs b/packages/core/addon/templates/components/table-renderer.hbs
@@ -21,14 +21,13 @@
       </div>
     {{else}}
       {{#let "table-renderer" as |groupName|}}
-        <ol class="table-header-row" {{sortable-group groupName=groupName onChange=@updateColumnOrder direction="x"}}>
+        <ol role="button" class="table-header-row" {{sortable-group groupName=groupName onChange=@updateColumnOrder direction="x"}} {{on "click" @headerClicked}}>
           {{#each @columns as |column|}}
             <li
               class="table-header-cell {{column.fragment.type}}"
-              role="button"
-              {{on "click" (fn @headerClicked column)}}
               {{sortable-item groupName=groupName model=column isDraggingDisabled=@isDraggingDisabled}}
               data-name={{column.fragment.canonicalName}}
+              data-column-id={{column.columnId}}
             >
               <span
                 class="table-header-cell__title {{if column.fragment.alias "table-header-cell__title--custom-name"}}"

diff --git a/packages/core/app/styles/navi-core/components/navi-table-sort-icon.scss b/packages/core/app/styles/navi-core/components/navi-table-sort-icon.scss
@@ -4,34 +4,26 @@
 */
 
 .navi-table-sort-icon {
-  display: flex;
-  flex-flow: column;
-  margin-left: 5px;
-
   &__caret {
-    color: map-get($denali-grey-colors, '400');
-    font-size: 14px;
-    height: 0;
-  }
+    color: map-get($denali-grey-colors, '500');
 
-  &--asc .sort-icon__caret-up,
-  &--desc .sort-icon__caret-down {
-    color: map-get($denali-grey-colors, '600');
-  }
-
-  &--none {
-    display: none;
+    &-up {
+      margin-bottom: -3px;
+    }
 
-    .table-header-cell:hover & {
-      display: inherit;
+    &-down {
+      margin-top: -3px;
     }
+  }
 
-    .table-header-row-vc & {
-      display: inherit;
+  &--asc &__caret-up,
+  &--desc &__caret-down {
+    color: map-get($denali-grey-colors, '700');
+  }
 
-      @media print {
-        display: none;
-      }
+  &--none {
+    @media print {
+      display: none;
     }
   }
 }
diff --git a/packages/core/app/styles/navi-core/components/navi-visualizations/table.scss b/packages/core/app/styles/navi-core/components/navi-visualizations/table.scss
@@ -100,10 +100,6 @@ $font-family-strong: 'HelveticaNeueW01-55Roma', Helvetica, Arial, sans-serif;
       flex: 1;
       font-family: $font-family-strong;
       min-width: 125px;
-
-      &__icon {
-        padding: 3px 5px;
-      }
     }
 
     &,
@@ -162,6 +158,10 @@ $font-family-strong: 'HelveticaNeueW01-55Roma', Helvetica, Arial, sans-serif;
         display: flex;
       }
 
+      .table-header-cell {
+        align-items: flex-end;
+      }
+
       &--edit {
         th:first-of-type > div {
           padding-left: 0;
@@ -172,6 +172,7 @@ $font-family-strong: 'HelveticaNeueW01-55Roma', Helvetica, Arial, sans-serif;
         }
 
         .table-header-cell {
+          align-items: center;
           background-color: $body-bg-color;
           border-bottom: 2px map-get($denali-grey-colors, '400') solid;
 
@@ -182,14 +183,6 @@ $font-family-strong: 'HelveticaNeueW01-55Roma', Helvetica, Arial, sans-serif;
         }
       }
 
-      .table-header-cell {
-        align-items: flex-end;
-
-        &__icon {
-          padding: 3px 5px 17px;
-        }
-      }
-
       &:not(.table-header-row-vc--edit) th .table-header-cell {
         height: 0;
         overflow-y: hidden;
@@ -212,10 +205,6 @@ $font-family-strong: 'HelveticaNeueW01-55Roma', Helvetica, Arial, sans-serif;
     min-width: 125px;
     padding: $cell-padding;
 
-    &__icon {
-      padding: 3px 5px;
-    }
-
     &__title {
       flex: 1;