Updates edit column menu to support hparam columns

tensorboard/webapp/BUILD

@@ -301,6 +301,7 @@ tf_ng_web_test_suite(
         "//tensorboard/webapp/widgets/content_wrapping_input:content_wrapping_input_tests",
         "//tensorboard/webapp/widgets/custom_modal:custom_modal_test",
         "//tensorboard/webapp/widgets/data_table:data_table_test",
+        "//tensorboard/webapp/widgets/data_table:utils_test",
         "//tensorboard/webapp/widgets/dropdown:dropdown_tests",
         "//tensorboard/webapp/widgets/experiment_alias:experiment_alias_test",
         "//tensorboard/webapp/widgets/filter_input:filter_input_test",

tensorboard/webapp/hparams/_redux/BUILD

@@ -65,8 +65,10 @@ tf_ts_library(
         ":types",
         ":utils",
         "//tensorboard/webapp/hparams:_types",
+        "//tensorboard/webapp/persistent_settings",
         "//tensorboard/webapp/runs/actions",
         "//tensorboard/webapp/widgets/data_table:types",
+        "//tensorboard/webapp/widgets/data_table:utils",
         "@npm//@ngrx/store",
     ],
 )
@@ -167,13 +169,15 @@ tf_ts_library(
         "//tensorboard/webapp/app_routing/actions",
         "//tensorboard/webapp/core/actions",
         "//tensorboard/webapp/hparams:types",
+        "//tensorboard/webapp/persistent_settings",
         "//tensorboard/webapp/runs/actions",
         "//tensorboard/webapp/runs/data_source:testing",
         "//tensorboard/webapp/runs/store:testing",
         "//tensorboard/webapp/testing:utils",
         "//tensorboard/webapp/util:types",
         "//tensorboard/webapp/webapp_data_source:http_client_testing",
         "//tensorboard/webapp/widgets/data_table:types",
+        "//tensorboard/webapp/widgets/data_table:utils",
         "@npm//@ngrx/effects",
         "@npm//@ngrx/store",
         "@npm//@types/jasmine",

tensorboard/webapp/hparams/_redux/hparams_reducers.ts

@@ -13,7 +13,9 @@ See the License for the specific language governing permissions and
 limitations under the License.
 ==============================================================================*/
 import {Action, ActionReducer, createReducer, on} from '@ngrx/store';
-import {ColumnHeader, Side} from '../../widgets/data_table/types';
+import {DataTableUtils} from '../../widgets/data_table/utils';
+import {persistentSettingsLoaded} from '../../persistent_settings';
+import {Side} from '../../widgets/data_table/types';
 import * as actions from './hparams_actions';
 import {HparamsState} from './types';
 
@@ -32,6 +34,16 @@ const initialState: HparamsState = {
 
 const reducer: ActionReducer<HparamsState, Action> = createReducer(
   initialState,
+  on(persistentSettingsLoaded, (state, {partialSettings}) => {
+    const {dashboardDisplayedHparamColumns: storedColumns} = partialSettings;
+    if (storedColumns) {
+      return {
+        ...state,
+        dashboardDisplayedHparamColumns: storedColumns,
+      };
+    }
+    return state;
+  }),
   on(actions.hparamsFetchSessionGroupsSucceeded, (state, action) => {
     const nextDashboardSpecs = action.hparamsAndMetricsSpecs;
     const nextDashboardSessionGroups = action.sessionGroups;
@@ -141,28 +153,12 @@ const reducer: ActionReducer<HparamsState, Action> = createReducer(
     actions.dashboardHparamColumnOrderChanged,
     (state, {source, destination, side}) => {
       const {dashboardDisplayedHparamColumns: columns} = state;
-      const sourceIndex = columns.findIndex(
-        (column: ColumnHeader) => column.name === source.name
+      const newColumns = DataTableUtils.moveColumn(
+        columns,
+        source,
+        destination,
+        side
       );
-      let destinationIndex = columns.findIndex(
-        (column: ColumnHeader) => column.name === destination.name
-      );
-      if (sourceIndex === -1 || sourceIndex === destinationIndex) {
-        return state;
-      }
-      if (destinationIndex === -1) {
-        // Use side as a backup to determine source position if destination isn't found.
-        if (side !== undefined) {
-          destinationIndex = side === Side.LEFT ? 0 : columns.length - 1;
-        } else {
-          return state;
-        }
-      }
-
-      const newColumns = [...columns];
-      newColumns.splice(sourceIndex, 1);
-      newColumns.splice(destinationIndex, 0, source);
-
       return {
         ...state,
         dashboardDisplayedHparamColumns: newColumns,

tensorboard/webapp/hparams/_redux/hparams_reducers_test.ts

@@ -18,8 +18,82 @@ import * as actions from './hparams_actions';
 import {reducers} from './hparams_reducers';
 import {buildHparamSpec, buildHparamsState, buildMetricSpec} from './testing';
 import {ColumnHeaderType, Side} from '../../widgets/data_table/types';
+import {DataTableUtils} from '../../widgets/data_table/utils';
+import {persistentSettingsLoaded} from '../../persistent_settings';
 
 describe('hparams/_redux/hparams_reducers_test', () => {
+  describe('#persistentSettingsLoaded', () => {
+    it('loads dashboardDisplayedHparamColumns from the persistent settings storage', () => {
+      const state = buildHparamsState({
+        dashboardDisplayedHparamColumns: [],
+      });
+      const state2 = reducers(
+        state,
+        persistentSettingsLoaded({
+          partialSettings: {
+            dashboardDisplayedHparamColumns: [
+              {
+                type: ColumnHeaderType.HPARAM,
+                name: 'conv_layers',
+                displayName: 'Conv Layers',
+                enabled: true,
+              },
+              {
+                type: ColumnHeaderType.HPARAM,
+                name: 'conv_kernel_size',
+                displayName: 'Conv Kernel Size',
+                enabled: true,
+              },
+            ],
+          },
+        })
+      );
+
+      expect(state2.dashboardDisplayedHparamColumns).toEqual([
+        {
+          type: ColumnHeaderType.HPARAM,
+          name: 'conv_layers',
+          displayName: 'Conv Layers',
+          enabled: true,
+        },
+        {
+          type: ColumnHeaderType.HPARAM,
+          name: 'conv_kernel_size',
+          displayName: 'Conv Kernel Size',
+          enabled: true,
+        },
+      ]);
+    });
+
+    it('does nothing if persistent settings does not contain dashboardDisplayedHparamColumns', () => {
+      const state = buildHparamsState({
+        dashboardDisplayedHparamColumns: [
+          {
+            type: ColumnHeaderType.HPARAM,
+            name: 'conv_layers',
+            displayName: 'Conv Layers',
+            enabled: true,
+          },
+        ],
+      });
+      const state2 = reducers(
+        state,
+        persistentSettingsLoaded({
+          partialSettings: {},
+        })
+      );
+
+      expect(state2.dashboardDisplayedHparamColumns).toEqual([
+        {
+          type: ColumnHeaderType.HPARAM,
+          name: 'conv_layers',
+          displayName: 'Conv Layers',
+          enabled: true,
+        },
+      ]);
+    });
+  });
+
   describe('hparamsFetchSessionGroupsSucceeded', () => {
     it('saves action.hparamsAndMetricsSpecs as dashboardSpecs', () => {
       const state = buildHparamsState({
@@ -594,104 +668,15 @@ describe('hparams/_redux/hparams_reducers_test', () => {
       },
     ];
 
-    it('does nothing if source is not found', () => {
+    it('moves source to destination using moveColumn', () => {
       const state = buildHparamsState({
         dashboardDisplayedHparamColumns: fakeColumns,
       });
-      const state2 = reducers(
-        state,
-        actions.dashboardHparamColumnOrderChanged({
-          source: {
-            type: ColumnHeaderType.HPARAM,
-            name: 'nonexistent_param',
-            displayName: 'Nonexistent param',
-            enabled: false,
-          },
-          destination: {
-            type: ColumnHeaderType.HPARAM,
-            name: 'conv_kernel_size',
-            displayName: 'Conv Kernel Size',
-            enabled: true,
-          },
-          side: Side.LEFT,
-        })
-      );
-
-      expect(state2.dashboardDisplayedHparamColumns).toEqual(fakeColumns);
-    });
-
-    it('does nothing if source equals dest', () => {
-      const state = buildHparamsState({
-        dashboardDisplayedHparamColumns: fakeColumns,
-      });
-      const state2 = reducers(
-        state,
-        actions.dashboardHparamColumnOrderChanged({
-          source: {
-            type: ColumnHeaderType.HPARAM,
-            name: 'conv_kernel_size',
-            displayName: 'Conv Kernel Size',
-            enabled: false,
-          },
-          destination: {
-            type: ColumnHeaderType.HPARAM,
-            name: 'conv_kernel_size',
-            displayName: 'Conv Kernel Size',
-            enabled: true,
-          },
-          side: Side.LEFT,
-        })
-      );
-
-      expect(state2.dashboardDisplayedHparamColumns).toEqual(fakeColumns);
-    });
-
-    [
-      {
-        testDesc: 'to front if side is left',
-        side: Side.LEFT,
-        expectedResult: [
-          fakeColumns[1],
-          fakeColumns[0],
-          ...fakeColumns.slice(2),
-        ],
-      },
-      {
-        testDesc: 'to back if side is right',
-        side: Side.RIGHT,
-        expectedResult: [
-          fakeColumns[0],
-          ...fakeColumns.slice(2),
-          fakeColumns[1],
-        ],
-      },
-    ].forEach(({testDesc, side, expectedResult}) => {
-      it(`if destination not found, moves source ${testDesc}`, () => {
-        const state = buildHparamsState({
-          dashboardDisplayedHparamColumns: fakeColumns,
-        });
-        const state2 = reducers(
-          state,
-          actions.dashboardHparamColumnOrderChanged({
-            source: fakeColumns[1],
-            destination: {
-              type: ColumnHeaderType.HPARAM,
-              name: 'nonexistent param',
-              displayName: 'Nonexistent param',
-              enabled: true,
-            },
-            side,
-          })
-        );
-
-        expect(state2.dashboardDisplayedHparamColumns).toEqual(expectedResult);
-      });
-    });
+      const moveColumnSpy = spyOn(
+        DataTableUtils,
+        'moveColumn'
+      ).and.callThrough();
 
-    it('swaps source and destination positions if destination is found', () => {
-      const state = buildHparamsState({
-        dashboardDisplayedHparamColumns: fakeColumns,
-      });
       const state2 = reducers(
         state,
         actions.dashboardHparamColumnOrderChanged({
@@ -701,6 +686,13 @@ describe('hparams/_redux/hparams_reducers_test', () => {
         })
       );
 
+      // Edge cases are tested by moveColumn tests.
+      expect(moveColumnSpy).toHaveBeenCalledWith(
+        fakeColumns,
+        fakeColumns[1],
+        fakeColumns[0],
+        Side.LEFT
+      );
       expect(state2.dashboardDisplayedHparamColumns).toEqual([
         fakeColumns[1],
         fakeColumns[0],

tensorboard/webapp/hparams/_redux/hparams_selectors.ts

@@ -47,8 +47,14 @@ export const getDashboardDefaultHparamFilters = createSelector(
 );
 
 export const getDashboardDisplayedHparamColumns = createSelector(
+  getDashboardHparamsAndMetricsSpecs,
   getHparamsState,
-  (state) => state.dashboardDisplayedHparamColumns
+  ({hparams}, state) => {
+    const hparamSet = new Set(hparams.map((hparam) => hparam.name));
+    return state.dashboardDisplayedHparamColumns.filter((column) =>
+      hparamSet.has(column.name)
+    );
+  }
 );
 
 export const getDashboardHparamFilterMap = createSelector(