Use maximum step as default for single step selection. (#6372)

## Motivation for features / changes

We are changing single step selection to default to the maximum step. We
think this is more useful than making the default the minimum step.

This also means that when step selection changes from single selection
to range selection, the minimum value of the chart is added as the start
of the range and the previous single step now becomes the end of the
range. Similarly, when step selection changes from range selection to
single selection, the new single selection is the previous maximum value
of the range.

## Technical description of changes

Where default value is set to "min" step, instead set it to the "max"
step. Where range selection is enabled, ensure the new start of the
range is the "min" step.

I have three internal CLs that accompagny this change.
* cl/528745120 adjusts or comments out code to allow the import to
succeed. It will be submitted prior to merging this PR.
* cl/528745440 is a set of harmless screenshot-updates. It will be
patched into the import CL and submitted at the same time as the import.
* cl/528745657 is a set of changes to uncomment out the code from the
first CL and make remaining adjustments. It will be submitted after the
import.

## Detailed steps to verify changes work correctly (as executed by you)

* I tested it manually quite a bit.
* I imported it into the internal repo and ensured that end-to-end tests
pass (after adjusting for the change in behavior).

Author: bmd3k

tensorboard/webapp/metrics/store/metrics_reducers.ts

@@ -1141,8 +1141,8 @@ const reducer = createReducer(
 
     // Updates cardStepIndex only when toggle to enable linked time.
     if (nextLinkedTimeEnabled) {
-      const {min} = state.stepMinMax;
-      const startStep = min === Infinity ? 0 : min;
+      const {max} = state.stepMinMax;
+      const startStep = max === -Infinity ? 0 : max;
       nextLinkedTimeSelection = state.linkedTimeSelection ?? {
         start: {step: startStep},
         end: null,
@@ -1202,15 +1202,19 @@ const reducer = createReducer(
         };
       }
       if (!linkedTimeSelection.end) {
+        // Enabling range selection from single selection selects the first
+        // step as the start of the range. The previous start step from single
+        // selection is now the end step.
         linkedTimeSelection = {
-          ...linkedTimeSelection,
-          end: {step: state.stepMinMax.max},
+          start: {step: state.stepMinMax.min},
+          end: linkedTimeSelection.start,
         };
       }
     } else {
       if (linkedTimeSelection) {
+        // Disabling range selection keeps the largest step from the range.
         linkedTimeSelection = {
-          ...linkedTimeSelection,
+          start: linkedTimeSelection.end ?? linkedTimeSelection.start,
           end: null,
         };
       }

tensorboard/webapp/metrics/store/metrics_reducers_test.ts

@@ -4057,8 +4057,8 @@ describe('metrics reducers', () => {
 
         const state2 = reducers(state1, actions.rangeSelectionToggled({}));
         expect(state2.linkedTimeSelection).toEqual({
-          start: {step: 100},
-          end: {step: -Infinity},
+          start: {step: Infinity},
+          end: {step: 100},
         });
       });
 
@@ -4073,7 +4073,7 @@ describe('metrics reducers', () => {
 
         const state2 = reducers(state1, actions.rangeSelectionToggled({}));
         expect(state2.linkedTimeSelection).toEqual({
-          start: {step: 100},
+          start: {step: 1000},
           end: null,
         });
       });
@@ -4358,15 +4358,15 @@ describe('metrics reducers', () => {
         });
       });
 
-      it('sets linkedTimeSelection to min step when linkedTimeSelection is null before toggling', () => {
+      it('sets linkedTimeSelection to max step when linkedTimeSelection is null before toggling', () => {
         const state1 = buildMetricsState({
           stepMinMax: {min: 10, max: 100},
         });
 
         const state2 = reducers(state1, actions.linkedTimeToggled({}));
 
         expect(state2.linkedTimeSelection).toEqual({
-          start: {step: 10},
+          start: {step: 100},
           end: null,
         });
       });

tensorboard/webapp/metrics/store/metrics_selectors.ts

@@ -440,7 +440,7 @@ export const getMetricsLinkedTimeSelectionSetting = createSelector(
     if (!state.linkedTimeSelection) {
       return {
         start: {
-          step: stepMinMax.min,
+          step: stepMinMax.max,
         },
         end: null,
       };
@@ -599,15 +599,33 @@ export const getMetricsCardTimeSelection = createSelector(
       globalRangeSelectionEnabled
     );
 
-    const startStep = cardState.timeSelection?.start.step ?? minMaxStep.minStep;
-    const endStep = cardState.timeSelection?.end?.step ?? minMaxStep.maxStep;
+    let timeSelection = cardState.timeSelection;
+    if (!timeSelection) {
+      timeSelection = {
+        start: {step: minMaxStep.minStep},
+        end: {step: minMaxStep.maxStep},
+      };
+    }
+    if (rangeSelectionEnabled) {
+      if (!timeSelection.end) {
+        // Enabling range selection from single selection selects the first
+        // step as the start of the range. The previous start step from single
+        // selection is now the end step.
+        timeSelection = {
+          start: {step: minMaxStep.minStep},
+          end: timeSelection.start,
+        };
+      }
+    } else {
+      // Disabling range selection keeps the largest step from the range.
+      timeSelection = {
+        start: timeSelection.end ?? timeSelection.start,
+        end: null,
+      };
+    }
 
-    // The default time selection
     return formatTimeSelection(
-      {
-        start: {step: startStep},
-        end: {step: endStep},
-      },
+      timeSelection,
       minMaxStep,
       rangeSelectionEnabled
     );

tensorboard/webapp/metrics/store/metrics_selectors_test.ts

@@ -549,7 +549,7 @@ describe('metrics selectors', () => {
         ).toBeUndefined();
       });
 
-      it('uses max step as end value if none exists', () => {
+      it('uses min step as start value if end value does not exists', () => {
         const state = appStateFromMetricsState(
           buildMetricsState({
             ...partialState,
@@ -562,7 +562,7 @@ describe('metrics selectors', () => {
                   maxStep: 10,
                 },
                 timeSelection: {
-                  start: {step: 0},
+                  start: {step: 5},
                   end: null,
                 },
               },
@@ -572,7 +572,7 @@ describe('metrics selectors', () => {
 
         expect(selectors.getMetricsCardTimeSelection(state, 'card1')).toEqual({
           start: {step: 0},
-          end: {step: 10},
+          end: {step: 5},
         });
       });
 
@@ -619,7 +619,7 @@ describe('metrics selectors', () => {
         );
 
         expect(selectors.getMetricsCardTimeSelection(state, 'card1')).toEqual({
-          start: {step: 0},
+          start: {step: 5},
           end: null,
         });
       });
@@ -642,7 +642,7 @@ describe('metrics selectors', () => {
         );
 
         expect(selectors.getMetricsCardTimeSelection(state, 'card1')).toEqual({
-          start: {step: 0},
+          start: {step: 5},
           end: null,
         });
       });
@@ -1436,7 +1436,7 @@ describe('metrics selectors', () => {
         })
       );
       expect(selectors.getMetricsLinkedTimeSelectionSetting(state)).toEqual({
-        start: {step: 0},
+        start: {step: 1000},
         end: null,
       });
     });

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

@@ -3971,7 +3971,7 @@ describe('scalar card', () => {
         );
         const fakeEvent = new MouseEvent('mousemove', {
           clientX: 25 + controllerStartPosition,
-          movementX: 1,
+          movementX: -1,
         });
         testController.mouseMove(fakeEvent);