data-trails: ability to (de)serialize parents and current index (#78782)

fix: ability to (de)serialize parents and current index

- refactored so only history stores parent relations and current index

- rounded indirect parent links

public/app/features/trails/DataTrail.test.tsx

@@ -36,18 +36,14 @@ describe('DataTrail', () => {
       expect(trail.state.topScene).toBeInstanceOf(MetricSelectScene);
     });
 
-    it('Should set stepIndex to 0', () => {
+    it('Should set history current step to 0', () => {
-      expect(trail.state.stepIndex).toBe(0);
-    });
-
-    it('Should set parentIndex to -1', () => {
-      expect(trail.state.parentIndex).toBe(-1);
-    });
-
-    it('Should set history currentStep to 0', () => {
       expect(trail.state.history.state.currentStep).toBe(0);
     });
 
+    it('Should set history step 0 parentIndex to -1', () => {
+      expect(trail.state.history.state.steps[0].parentIndex).toBe(-1);
+    });
+
     describe('And metric is selected', () => {
       beforeEach(() => {
         trail.publishEvent(new MetricSelectedEvent('metric_bucket'));
@@ -66,16 +62,16 @@ describe('DataTrail', () => {
         expect(trail.state.history.state.steps[1].type).toBe('metric');
       });
 
-      it('Should set stepIndex to 1', () => {
+      it('Should set history current step to 1', () => {
-        expect(trail.state.stepIndex).toBe(1);
+        expect(trail.state.history.state.currentStep).toBe(1);
       });
 
       it('Should set history currentStep to 1', () => {
         expect(trail.state.history.state.currentStep).toBe(1);
       });
 
-      it('Should set parentIndex to 0', () => {
+      it('Should set history step 1 parentIndex to 0', () => {
-        expect(trail.state.parentIndex).toBe(0);
+        expect(trail.state.history.state.steps[1].parentIndex).toBe(0);
       });
     });
 
@@ -83,7 +79,7 @@ describe('DataTrail', () => {
       beforeEach(() => {
         trail.publishEvent(new MetricSelectedEvent('first_metric'));
         trail.publishEvent(new MetricSelectedEvent('second_metric'));
-        trail.goBackToStep(trail.state.history.state.steps[1]);
+        trail.state.history.goBackToStep(1);
       });
 
       it('Should restore state and url', () => {
@@ -91,10 +87,6 @@ describe('DataTrail', () => {
         expect(locationService.getSearchObject().metric).toBe('first_metric');
       });
 
-      it('Should set stepIndex to 1', () => {
-        expect(trail.state.stepIndex).toBe(1);
-      });
-
       it('Should set history currentStep to 1', () => {
         expect(trail.state.history.state.currentStep).toBe(1);
       });
@@ -112,16 +104,12 @@ describe('DataTrail', () => {
           expect(trail.state.history.state.steps.length).toBe(4);
         });
 
-        it('Should set stepIndex to 3', () => {
+        it('Should set history current step to 3', () => {
-          expect(trail.state.stepIndex).toBe(3);
-        });
-
-        it('Should set history currentStep to 1', () => {
           expect(trail.state.history.state.currentStep).toBe(3);
         });
 
-        it('Should set parentIndex to 1', () => {
+        it('Should set history step 3 parent index to 1', () => {
-          expect(trail.state.parentIndex).toBe(1);
+          expect(trail.state.history.state.steps[3].parentIndex).toBe(1);
         });
       });
     });

public/app/features/trails/DataTrail.tsx

@@ -44,10 +44,6 @@ export interface DataTrailState extends SceneObjectState {
 
   // Synced with url
   metric?: string;
-
-  // Indicates which step in the data trail this is
-  stepIndex: number;
-  parentIndex: number; // If there is no parent, this will be -1
 }
 
 export class DataTrail extends SceneObjectBase<DataTrailState> {
@@ -65,8 +61,6 @@ export class DataTrail extends SceneObjectBase<DataTrailState> {
       ],
       history: state.history ?? new DataTrailHistory({}),
       settings: state.settings ?? new DataTrailSettings({}),
-      stepIndex: state.stepIndex ?? 0,
-      parentIndex: state.parentIndex ?? -1,
       ...state,
     });
 
@@ -81,6 +75,29 @@ export class DataTrail extends SceneObjectBase<DataTrailState> {
     // Some scene elements publish this
     this.subscribeToEvent(MetricSelectedEvent, this._handleMetricSelectedEvent.bind(this));
 
+    // Pay attention to changes in history (i.e., changing the step)
+    this.state.history.subscribeToState((newState, oldState) => {
+      const oldNumberOfSteps = oldState.steps.length;
+      const newNumberOfSteps = newState.steps.length;
+
+      const newStepWasAppended = newNumberOfSteps > oldNumberOfSteps;
+
+      if (newStepWasAppended) {
+        // Do nothing because the state is already up to date -- it created a new step!
+        return;
+      }
+
+      if (oldState.currentStep === newState.currentStep) {
+        // The same step was clicked on -- no need to change anything.
+        return;
+      }
+
+      // History changed because a different node was selected
+      const step = newState.steps[newState.currentStep];
+
+      this.goBackToStep(step);
+    });
+
     return () => {
       if (!this.state.embedded) {
         getUrlSyncManager().cleanUp(this);
@@ -89,7 +106,7 @@ export class DataTrail extends SceneObjectBase<DataTrailState> {
     };
   }
 
-  public goBackToStep(step: DataTrailHistoryStep) {
+  private goBackToStep(step: DataTrailHistoryStep) {
     if (!this.state.embedded) {
       getUrlSyncManager().cleanUp(this);
     }

public/app/features/trails/DataTrailsHistory.tsx

@@ -26,6 +26,7 @@ export interface DataTrailHistoryStep {
   description: string;
   type: TrailStepType;
   trailState: DataTrailState;
+  parentIndex: number;
 }
 
 export type TrailStepType = 'filters' | 'time' | 'metric' | 'start';
@@ -37,6 +38,8 @@ export class DataTrailHistory extends SceneObjectBase<DataTrailsHistoryState> {
     this.addActivationHandler(this._onActivate.bind(this));
   }
 
+  private stepTransitionInProgress = false;
+
   public _onActivate() {
     const trail = getTrailFor(this);
 
@@ -45,19 +48,6 @@ export class DataTrailHistory extends SceneObjectBase<DataTrailsHistoryState> {
     }
 
     trail.subscribeToState((newState, oldState) => {
-      // Check if new and old state are at the same step index
-      // Then we know this is a history transition
-      const isMovingThroughHistory = newState.stepIndex !== oldState.stepIndex;
-
-      if (isMovingThroughHistory) {
-        this.setState({
-          ...this.state,
-          currentStep: newState.stepIndex,
-        });
-
-        return;
-      }
-
       if (newState.metric !== oldState.metric) {
         if (this.state.steps.length === 1) {
           // For the first step we want to update the starting state so that it contains data
@@ -84,10 +74,13 @@ export class DataTrailHistory extends SceneObjectBase<DataTrailsHistoryState> {
   }
 
   public addTrailStep(trail: DataTrail, type: TrailStepType) {
-    // Update the trail's new current step state, and note its parent step.
+    if (this.stepTransitionInProgress) {
+      // Do not add trail steps when step transition is in progress
+      return;
+    }
+
     const stepIndex = this.state.steps.length;
-    const parentIndex = type === 'start' ? -1 : trail.state.stepIndex;
+    const parentIndex = type === 'start' ? -1 : this.state.currentStep;
-    trail.setState({ ...trail.state, stepIndex, parentIndex });
 
     this.setState({
       currentStep: stepIndex,
@@ -97,11 +90,20 @@ export class DataTrailHistory extends SceneObjectBase<DataTrailsHistoryState> {
           description: 'Test',
           type,
           trailState: sceneUtils.cloneSceneObjectState(trail.state, { history: this }),
+          parentIndex,
         },
       ],
     });
   }
 
+  public goBackToStep(stepIndex: number) {
+    this.stepTransitionInProgress = true;
+
+    this.setState({ currentStep: stepIndex });
+
+    this.stepTransitionInProgress = false;
+  }
+
   renderStepTooltip(step: DataTrailHistoryStep) {
     return (
       <Stack direction="column">
@@ -114,20 +116,19 @@ export class DataTrailHistory extends SceneObjectBase<DataTrailsHistoryState> {
   public static Component = ({ model }: SceneComponentProps<DataTrailHistory>) => {
     const { steps, currentStep } = model.useState();
     const styles = useStyles2(getStyles);
-    const trail = getTrailFor(model);
 
     const { ancestry, alternatePredecessorStyle } = useMemo(() => {
       const ancestry = new Set<number>();
       let cursor = currentStep;
       while (cursor >= 0) {
         ancestry.add(cursor);
-        cursor = steps[cursor].trailState.parentIndex;
+        cursor = steps[cursor].parentIndex;
       }
 
       const alternatePredecessorStyle = new Map<number, string>();
 
       ancestry.forEach((index) => {
-        const parent = steps[index].trailState.parentIndex;
+        const parent = steps[index].parentIndex;
         if (parent + 1 !== index) {
           alternatePredecessorStyle.set(index, createAlternatePredecessorStyle(index, parent));
         }
@@ -152,13 +153,13 @@ export class DataTrailHistory extends SceneObjectBase<DataTrailsHistoryState> {
                 // To alter the look of steps with distant non-directly preceding parent
                 alternatePredecessorStyle.get(index) ?? '',
                 // To remove direct link for steps that don't have a direct parent
-                index !== step.trailState.parentIndex + 1 ? styles.stepOmitsDirectLeftLink : '',
+                index !== step.parentIndex + 1 ? styles.stepOmitsDirectLeftLink : '',
                 // To remove the direct parent link on the start node as well
                 index === 0 ? styles.stepOmitsDirectLeftLink : '',
                 // To darken steps that aren't the current step's ancesters
                 !ancestry.has(index) ? styles.stepIsNotAncestorOfCurrent : ''
               )}
-              onClick={() => trail.goBackToStep(step)}
+              onClick={() => model.goBackToStep(index)}
             ></button>
           </Tooltip>
         ))}
@@ -269,6 +270,8 @@ function createAlternatePredecessorStyle(index: number, parent: number) {
       borderStyle: 'solid',
       borderWidth: 2,
       borderBottom: 'none',
+      borderTopLeftRadius: 8,
+      borderTopRightRadius: 8,
       top: -10,
       left: 3 - distanceToParent,
       background: 'none',

public/app/features/trails/TrailStore/TrailStore.ts

@@ -13,7 +13,9 @@ export interface SerializedTrail {
     urlValues: SceneObjectUrlValues;
     type: TrailStepType;
     description: string;
+    parentIndex: number;
   }>;
+  currentStep: number;
 }
 
 export class TrailStore {
@@ -55,9 +57,15 @@ export class TrailStore {
 
     t.history.map((step) => {
       this._loadFromUrl(trail, step.urlValues);
+      const parentIndex = step.parentIndex ?? trail.state.history.state.steps.length - 1;
+      // Set the parent of the next trail step by setting the current step in history.
+      trail.state.history.setState({ currentStep: parentIndex });
       trail.state.history.addTrailStep(trail, step.type);
     });
 
+    const currentStep = t.currentStep ?? trail.state.history.state.steps.length - 1;
+    trail.state.history.setState({ currentStep });
+
     return trail;
   }
 
@@ -68,10 +76,12 @@ export class TrailStore {
         urlValues: getUrlSyncManager().getUrlState(stepTrail),
         type: step.type,
         description: step.description,
+        parentIndex: step.parentIndex,
       };
     });
     return {
       history,
+      currentStep: trail.state.history.state.currentStep,
     };
   }