From ac1b9e44a2df061dfce56836077a86bee0fed226 Mon Sep 17 00:00:00 2001 From: Darren Janeczek <38694490+darrenjaneczek@users.noreply.github.com> Date: Mon, 4 Dec 2023 10:04:58 -0500 Subject: [PATCH] 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 | 38 ++++++---------- public/app/features/trails/DataTrail.tsx | 31 ++++++++++--- .../app/features/trails/DataTrailsHistory.tsx | 45 ++++++++++--------- .../features/trails/TrailStore/TrailStore.ts | 10 +++++ 4 files changed, 71 insertions(+), 53 deletions(-) diff --git a/public/app/features/trails/DataTrail.test.tsx b/public/app/features/trails/DataTrail.test.tsx index 5bb6ae2dc13..31c6869ca63 100644 --- a/public/app/features/trails/DataTrail.test.tsx +++ b/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', () => { - 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', () => { + it('Should set history current step 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', () => { - expect(trail.state.stepIndex).toBe(1); + it('Should set history current step to 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', () => { - expect(trail.state.parentIndex).toBe(0); + it('Should set history step 1 parentIndex to 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', () => { - expect(trail.state.stepIndex).toBe(3); - }); - - it('Should set history currentStep to 1', () => { + it('Should set history current step to 3', () => { expect(trail.state.history.state.currentStep).toBe(3); }); - it('Should set parentIndex to 1', () => { - expect(trail.state.parentIndex).toBe(1); + it('Should set history step 3 parent index to 1', () => { + expect(trail.state.history.state.steps[3].parentIndex).toBe(1); }); }); }); diff --git a/public/app/features/trails/DataTrail.tsx b/public/app/features/trails/DataTrail.tsx index 85549d3dbb1..91c40dab47b 100644 --- a/public/app/features/trails/DataTrail.tsx +++ b/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 { @@ -65,8 +61,6 @@ export class DataTrail extends SceneObjectBase { ], 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 { // 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 { }; } - public goBackToStep(step: DataTrailHistoryStep) { + private goBackToStep(step: DataTrailHistoryStep) { if (!this.state.embedded) { getUrlSyncManager().cleanUp(this); } diff --git a/public/app/features/trails/DataTrailsHistory.tsx b/public/app/features/trails/DataTrailsHistory.tsx index e37cab2f545..b5bdbb6eeae 100644 --- a/public/app/features/trails/DataTrailsHistory.tsx +++ b/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 { this.addActivationHandler(this._onActivate.bind(this)); } + private stepTransitionInProgress = false; + public _onActivate() { const trail = getTrailFor(this); @@ -45,19 +48,6 @@ export class DataTrailHistory extends SceneObjectBase { } 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 { } 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; - trail.setState({ ...trail.state, stepIndex, parentIndex }); + const parentIndex = type === 'start' ? -1 : this.state.currentStep; this.setState({ currentStep: stepIndex, @@ -97,11 +90,20 @@ export class DataTrailHistory extends SceneObjectBase { 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 ( @@ -114,20 +116,19 @@ export class DataTrailHistory extends SceneObjectBase { public static Component = ({ model }: SceneComponentProps) => { const { steps, currentStep } = model.useState(); const styles = useStyles2(getStyles); - const trail = getTrailFor(model); const { ancestry, alternatePredecessorStyle } = useMemo(() => { const ancestry = new Set(); let cursor = currentStep; while (cursor >= 0) { ancestry.add(cursor); - cursor = steps[cursor].trailState.parentIndex; + cursor = steps[cursor].parentIndex; } const alternatePredecessorStyle = new Map(); 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 { // 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)} > ))} @@ -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', diff --git a/public/app/features/trails/TrailStore/TrailStore.ts b/public/app/features/trails/TrailStore/TrailStore.ts index b47f0815804..e7ea4e4c06b 100644 --- a/public/app/features/trails/TrailStore/TrailStore.ts +++ b/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, }; }