diff --git a/packages/scenes/src/core/types.ts b/packages/scenes/src/core/types.ts index 69b4fc002..b58542ed0 100644 --- a/packages/scenes/src/core/types.ts +++ b/packages/scenes/src/core/types.ts @@ -190,7 +190,6 @@ export function isSceneObject(obj: any): obj is SceneObject { export interface SceneObjectWithUrlSync extends SceneObject { getUrlState(): SceneObjectUrlValues; updateFromUrl(values: SceneObjectUrlValues): void; - shouldCreateHistoryStep?(values: SceneObjectUrlValues): boolean; } export interface SceneObjectUrlSyncHandler { @@ -198,6 +197,7 @@ export interface SceneObjectUrlSyncHandler { getUrlState(): SceneObjectUrlValues; updateFromUrl(values: SceneObjectUrlValues): void; shouldCreateHistoryStep?(values: SceneObjectUrlValues): boolean; + performBrowserHistoryAction?(callback: () => void): void; } export interface DataRequestEnricher { diff --git a/packages/scenes/src/services/SceneObjectUrlSyncConfig.ts b/packages/scenes/src/services/SceneObjectUrlSyncConfig.ts index 38b3db3af..f0e082d68 100644 --- a/packages/scenes/src/services/SceneObjectUrlSyncConfig.ts +++ b/packages/scenes/src/services/SceneObjectUrlSyncConfig.ts @@ -28,13 +28,13 @@ export class SceneObjectUrlSyncConfig implements SceneObjectUrlSyncHandler { this._sceneObject.updateFromUrl(values); } + public shouldCreateHistoryStep(values: SceneObjectUrlValues): boolean { + return this._nextChangeShouldAddHistoryStep; + } + public performBrowserHistoryAction(callback: () => void) { this._nextChangeShouldAddHistoryStep = true; callback(); this._nextChangeShouldAddHistoryStep = false; } - - public shouldCreateHistoryStep(values: SceneObjectUrlValues): boolean { - return this._nextChangeShouldAddHistoryStep; - } } diff --git a/packages/scenes/src/variables/components/VariableValueSelect.tsx b/packages/scenes/src/variables/components/VariableValueSelect.tsx index 11372d5fd..05891e2e4 100644 --- a/packages/scenes/src/variables/components/VariableValueSelect.tsx +++ b/packages/scenes/src/variables/components/VariableValueSelect.tsx @@ -98,7 +98,7 @@ export function VariableValueSelect({ model }: SceneComponentProps { - model.changeValueTo(newValue.value!, newValue.label!); + model.changeValueTo(newValue.value!, newValue.label!, true); queryController?.startProfile(model); if (hasCustomValue !== newValue.__isNew__) { @@ -179,14 +179,14 @@ export function VariableValueSelectMulti({ model }: SceneComponentProps { - model.changeValueTo(uncommittedValue); + model.changeValueTo(uncommittedValue, undefined, true); queryController?.startProfile(model); }} filterOption={filterNoOp} data-testid={selectors.pages.Dashboard.SubMenu.submenuItemValueDropDownValueLinkTexts(`${uncommittedValue}`)} onChange={(newValue, action) => { if (action.action === 'clear' && noValueOnClear) { - model.changeValueTo([]); + model.changeValueTo([], undefined, true); } setUncommittedValue(newValue.map((x) => x.value!)); }} diff --git a/packages/scenes/src/variables/groupby/GroupByVariable.tsx b/packages/scenes/src/variables/groupby/GroupByVariable.tsx index 71a2dbb62..d40811aeb 100644 --- a/packages/scenes/src/variables/groupby/GroupByVariable.tsx +++ b/packages/scenes/src/variables/groupby/GroupByVariable.tsx @@ -292,12 +292,13 @@ export function GroupByVariableRenderer({ model }: SceneComponentProps { model.changeValueTo( uncommittedValue.map((x) => x.value!), - uncommittedValue.map((x) => x.label!) + uncommittedValue.map((x) => x.label!), + true ); }} onChange={(newValue, action) => { if (action.action === 'clear' && noValueOnClear) { - model.changeValueTo([]); + model.changeValueTo([], undefined, true); } setUncommittedValue(newValue); }} diff --git a/packages/scenes/src/variables/variants/MultiValueVariable.ts b/packages/scenes/src/variables/variants/MultiValueVariable.ts index 245928b1f..edac0391c 100644 --- a/packages/scenes/src/variables/variants/MultiValueVariable.ts +++ b/packages/scenes/src/variables/variants/MultiValueVariable.ts @@ -260,7 +260,7 @@ export abstract class MultiValueVariable this.setStateHelper({ value, text, loading: false }); + + /** + * Because variable state changes can cause a whole chain of downstream state changes in other variables (that also cause URL update) + * Only some variable changes should add new history items to make sure the browser history contains valid URL states to go back to. + */ + if (isUserAction) { + this._urlSync.performBrowserHistoryAction?.(stateChangeAction); + } else { + stateChangeAction(); + } + this.publishEvent(new SceneVariableValueChangedEvent(this), true); } @@ -386,9 +397,11 @@ function findOptionMatchingCurrent( export class MultiValueUrlSyncHandler implements SceneObjectUrlSyncHandler { - public constructor(private _sceneObject: MultiValueVariable) {} + protected _nextChangeShouldAddHistoryStep = false; + + public constructor(protected _sceneObject: MultiValueVariable) {} - private getKey(): string { + protected getKey(): string { return `var-${this._sceneObject.state.name}`; } @@ -446,6 +459,16 @@ export class MultiValueUrlSyncHandler void) { + this._nextChangeShouldAddHistoryStep = true; + callback(); + this._nextChangeShouldAddHistoryStep = false; + } + + public shouldCreateHistoryStep(values: SceneObjectUrlValues): boolean { + return this._nextChangeShouldAddHistoryStep; + } } function handleLegacyUrlAllValue(value: string | string[]) {