Column Customization: Use dataTableColumnEdited action when dragging columns in data table (#6291)

* Motivation for features / changes
Before this change, dragging the header in the data table called the
dataTableColumnDrag action. This action relied on the global
rangeSelectionEnabled boolean to decide if it edits the range of single
selection headers. This boolean no longer is guaranteed to represent the
table which is being edited which can lead to a bug where the single
selection table shows the range selection headers and vise versa.

* Technical description of changes
This moves the decision on which columns to edit to the
ScalarCardDataTable component. It also reuses the dataTableColumnEdited
action which is used by the ScalarColumnEditor component.

* Screenshots of UI changes
This is the broken state which is possible before this change. Note that
the single selection has the range columns.
<img width="900" alt="Screenshot 2023-04-05 at 11 52 24 AM"
src="https://user-images.githubusercontent.com/8672809/230177414-bdadc95c-e416-4cce-abcf-6cca55c9dd61.png">

Author: JamesHollyer

tensorboard/webapp/metrics/actions/index.ts

@@ -234,13 +234,6 @@ export const sortingDataTable = createAction(
   props<SortingInfo>()
 );
 
-export const dataTableColumnDrag = createAction(
-  '[Metrics] Data table column dragged',
-  props<{
-    newOrder: ColumnHeader[];
-  }>()
-);
-
 export const dataTableColumnEdited = createAction(
   '[Metrics] Data table columns edited in edit menu',
   props<HeaderEditInfo>()

tensorboard/webapp/metrics/store/metrics_reducers.ts

@@ -1252,19 +1252,6 @@ const reducer = createReducer(
       linkedTimeSelection: null,
     };
   }),
-  on(actions.dataTableColumnDrag, (state, {newOrder}) => {
-    if (state.rangeSelectionEnabled) {
-      return {
-        ...state,
-        rangeSelectionHeaders: newOrder,
-      };
-    }
-
-    return {
-      ...state,
-      singleSelectionHeaders: newOrder,
-    };
-  }),
   on(actions.dataTableColumnEdited, (state, {dataTableMode, headers}) => {
     const enabledNewHeaders: ColumnHeader[] = [];
     const disabledNewHeaders: ColumnHeader[] = [];

tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html

@@ -200,7 +200,7 @@
       [columnCustomizationEnabled]="columnCustomizationEnabled"
       [smoothingEnabled]="smoothingEnabled"
       (sortDataBy)="sortDataBy($event)"
-      (orderColumns)="reorderColumnHeaders.emit($event)"
+      (editColumnHeaders)="editColumnHeaders.emit($event)"
     >
     </scalar-card-data-table>
   </div>

tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts

@@ -44,7 +44,7 @@ import {
   TooltipDatum,
 } from '../../../widgets/line_chart_v2/types';
 import {CardState} from '../../store';
-import {TooltipSort, XAxisType} from '../../types';
+import {HeaderEditInfo, TooltipSort, XAxisType} from '../../types';
 import {
   ColumnHeader,
   ColumnHeaderType,
@@ -108,7 +108,7 @@ export class ScalarCardComponent<Downloader> {
   @Output() onStepSelectorToggled =
     new EventEmitter<TimeSelectionToggleAffordance>();
   @Output() onDataTableSorting = new EventEmitter<SortingInfo>();
-  @Output() reorderColumnHeaders = new EventEmitter<ColumnHeader[]>();
+  @Output() editColumnHeaders = new EventEmitter<HeaderEditInfo>();
   @Output() openSlideoutColumnEditMenu = new EventEmitter<void>();
 
   @Output() onLineChartZoom = new EventEmitter<Extent>();

tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts

@@ -68,7 +68,7 @@ import {Extent} from '../../../widgets/line_chart_v2/lib/public_types';
 import {ScaleType} from '../../../widgets/line_chart_v2/types';
 import {
   cardMinMaxChanged,
-  dataTableColumnDrag,
+  dataTableColumnEdited,
   metricsCardFullSizeToggled,
   metricsCardStateUpdated,
   sortingDataTable,
@@ -92,12 +92,13 @@ import {
   getSingleSelectionHeaders,
   RunToSeries,
 } from '../../store';
-import {CardId, CardMetadata, XAxisType} from '../../types';
+import {CardId, CardMetadata, HeaderEditInfo, XAxisType} from '../../types';
 import {CardRenderer} from '../metrics_view_types';
 import {getTagDisplayName} from '../utils';
 import {DataDownloadDialogContainer} from './data_download_dialog_container';
 import {
   ColumnHeader,
+  DataTableMode,
   MinMaxStep,
   PartialSeries,
   PartitionedSeries,
@@ -182,7 +183,7 @@ function isMinMaxStepValid(minMax: MinMaxStep | undefined): boolean {
       (onStepSelectorToggled)="onStepSelectorToggled($event)"
       (onDataTableSorting)="onDataTableSorting($event)"
       (onLineChartZoom)="onLineChartZoom($event)"
-      (reorderColumnHeaders)="reorderColumnHeaders($event)"
+      (editColumnHeaders)="editColumnHeaders($event)"
       (onCardStateChanged)="onCardStateChanged($event)"
       (openSlideoutColumnEditMenu)="openSlideoutColumnEditMenu()"
     ></scalar-card-component>
@@ -663,8 +664,8 @@ export class ScalarCardContainer implements CardRenderer, OnInit, OnDestroy {
     );
   }
 
-  reorderColumnHeaders(headers: ColumnHeader[]) {
-    this.store.dispatch(dataTableColumnDrag({newOrder: headers}));
+  editColumnHeaders(headerEditInfo: HeaderEditInfo) {
+    this.store.dispatch(dataTableColumnEdited(headerEditInfo));
   }
 
   openSlideoutColumnEditMenu() {

tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ts

@@ -21,9 +21,11 @@ import {
 } from '@angular/core';
 import {TimeSelection} from '../../../widgets/card_fob/card_fob_types';
 import {findClosestIndex} from '../../../widgets/line_chart_v2/sub_view/line_chart_interactive_utils';
+import {HeaderEditInfo} from '../../types';
 import {
   ColumnHeader,
   ColumnHeaderType,
+  DataTableMode,
   ScalarCardDataSeries,
   ScalarCardPoint,
   ScalarCardSeriesMetadataMap,
@@ -43,7 +45,7 @@ import {isDatumVisible} from './utils';
       [columnCustomizationEnabled]="columnCustomizationEnabled"
       [smoothingEnabled]="smoothingEnabled"
       (sortDataBy)="sortDataBy.emit($event)"
-      (orderColumns)="orderColumns.emit($event)"
+      (orderColumns)="orderColumns($event)"
     ></tb-data-table>
   `,
   changeDetection: ChangeDetectionStrategy.OnPush,
@@ -58,7 +60,7 @@ export class ScalarCardDataTable {
   @Input() smoothingEnabled!: boolean;
 
   @Output() sortDataBy = new EventEmitter<SortingInfo>();
-  @Output() orderColumns = new EventEmitter<ColumnHeader[]>();
+  @Output() editColumnHeaders = new EventEmitter<HeaderEditInfo>();
 
   getMinPointInRange(
     points: ScalarCardPoint[],
@@ -278,6 +280,15 @@ export class ScalarCardDataTable {
         return makeValueSortable(point[header]);
     }
   }
+
+  orderColumns(headers: ColumnHeader[]) {
+    this.editColumnHeaders.emit({
+      headers: headers,
+      dataTableMode: this.stepOrLinkedTimeSelection.end
+        ? DataTableMode.RANGE
+        : DataTableMode.SINGLE,
+    });
+  }
 }
 
 function makeValueSortable(value: number | string | null | undefined) {

tensorboard/webapp/metrics/views/card_renderer/scalar_card_test.ts

@@ -81,6 +81,7 @@ import {
   stepSelectorToggled,
   timeSelectionChanged,
   metricsSlideoutMenuOpened,
+  dataTableColumnEdited,
 } from '../../actions';
 import {PluginType} from '../../data_source';
 import {
@@ -111,6 +112,7 @@ import {ScalarCardFobController} from './scalar_card_fob_controller';
 import {
   ColumnHeader,
   ColumnHeaderType,
+  DataTableMode,
   ScalarCardPoint,
   ScalarCardSeriesMetadata,
   SeriesType,
@@ -3932,6 +3934,74 @@ describe('scalar card', () => {
 
         expect(dataTableComponent).toBeFalsy();
       }));
+
+      it('emits dataTableColumnEdited with DataTableMode.SINGLE when orderColumns is called while in Single Selection', fakeAsync(() => {
+        store.overrideSelector(getCardStateMap, {
+          card1: {
+            dataMinMax: {
+              minStep: 0,
+              maxStep: 100,
+            },
+          },
+        });
+        store.overrideSelector(getMetricsCardTimeSelection, {
+          start: {step: 1},
+          end: null,
+        });
+        store.overrideSelector(selectors.getMetricsStepSelectorEnabled, true);
+        const fixture = createComponent('card1');
+        fixture.detectChanges();
+        const scalarCardDataTable = fixture.debugElement.query(
+          By.directive(ScalarCardDataTable)
+        );
+
+        const headers = [
+          {type: ColumnHeaderType.RUN, enabled: true},
+          {type: ColumnHeaderType.VALUE, enabled: true},
+        ];
+        scalarCardDataTable.componentInstance.orderColumns(headers);
+
+        expect(dispatchedActions).toEqual([
+          dataTableColumnEdited({
+            headers,
+            dataTableMode: DataTableMode.SINGLE,
+          }),
+        ]);
+      }));
+
+      it('emits dataTableColumnEdited with DataTableMode.RANGE when orderColumns is called while in Range Selection', fakeAsync(() => {
+        store.overrideSelector(getCardStateMap, {
+          card1: {
+            dataMinMax: {
+              minStep: 0,
+              maxStep: 100,
+            },
+          },
+        });
+        store.overrideSelector(getMetricsCardTimeSelection, {
+          start: {step: 1},
+          end: {step: 20},
+        });
+        store.overrideSelector(selectors.getMetricsStepSelectorEnabled, true);
+        const fixture = createComponent('card1');
+        fixture.detectChanges();
+        const scalarCardDataTable = fixture.debugElement.query(
+          By.directive(ScalarCardDataTable)
+        );
+
+        const headers = [
+          {type: ColumnHeaderType.RUN, enabled: true},
+          {type: ColumnHeaderType.VALUE, enabled: true},
+        ];
+        scalarCardDataTable.componentInstance.orderColumns(headers);
+
+        expect(dispatchedActions).toEqual([
+          dataTableColumnEdited({
+            headers,
+            dataTableMode: DataTableMode.RANGE,
+          }),
+        ]);
+      }));
     });
   });
 });