From c86f4ae5e06c0e83bda4a4cc637466a2bbfd645d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Thu, 30 May 2024 13:24:19 +0200 Subject: [PATCH 01/11] UrlSync: Add browser history step when updating URL --- packages/scenes/src/components/SceneRefreshPicker.tsx | 6 ++---- packages/scenes/src/services/UrlSyncManager.ts | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/scenes/src/components/SceneRefreshPicker.tsx b/packages/scenes/src/components/SceneRefreshPicker.tsx index ab9d6e97a..dd7dcf06d 100644 --- a/packages/scenes/src/components/SceneRefreshPicker.tsx +++ b/packages/scenes/src/components/SceneRefreshPicker.tsx @@ -13,7 +13,7 @@ export const DEFAULT_INTERVALS = ['5s', '10s', '30s', '1m', '5m', '15m', '30m', export interface SceneRefreshPickerState extends SceneObjectState { // Refresh interval, e.g. 5s, 1m, 2h - refresh: string; + refresh?: string; autoEnabled?: boolean; autoMinInterval?: string; autoValue?: string; @@ -76,9 +76,7 @@ export class SceneRefreshPicker extends SceneObjectBase }; public getUrlState() { - return { - refresh: this.state.refresh, - }; + return { refresh: this.state.refresh !== '' ? this.state.refresh : undefined }; } public updateFromUrl(values: SceneObjectUrlValues) { diff --git a/packages/scenes/src/services/UrlSyncManager.ts b/packages/scenes/src/services/UrlSyncManager.ts index c8d9d6ec9..1c5c9d61b 100644 --- a/packages/scenes/src/services/UrlSyncManager.ts +++ b/packages/scenes/src/services/UrlSyncManager.ts @@ -117,7 +117,7 @@ export class UrlSyncManager implements UrlSyncManagerLike { if (Object.keys(mappedUpdated).length > 0) { this._ignoreNextLocationUpdate = true; - locationService.partial(mappedUpdated, true); + locationService.partial(mappedUpdated, false); } } }; From e7349fd4c2867bb03666c5f4bff6eff730bee985 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Mon, 26 Aug 2024 10:52:44 +0200 Subject: [PATCH 02/11] Update --- .../src/components/SceneRefreshPicker.tsx | 6 +- packages/scenes/src/core/SceneTimeRange.tsx | 57 +++---------------- packages/scenes/src/core/types.ts | 8 ++- .../src/services/SceneObjectUrlSyncConfig.ts | 4 ++ .../scenes/src/services/UrlSyncManager.ts | 10 +++- 5 files changed, 29 insertions(+), 56 deletions(-) diff --git a/packages/scenes/src/components/SceneRefreshPicker.tsx b/packages/scenes/src/components/SceneRefreshPicker.tsx index dd7dcf06d..ab9d6e97a 100644 --- a/packages/scenes/src/components/SceneRefreshPicker.tsx +++ b/packages/scenes/src/components/SceneRefreshPicker.tsx @@ -13,7 +13,7 @@ export const DEFAULT_INTERVALS = ['5s', '10s', '30s', '1m', '5m', '15m', '30m', export interface SceneRefreshPickerState extends SceneObjectState { // Refresh interval, e.g. 5s, 1m, 2h - refresh?: string; + refresh: string; autoEnabled?: boolean; autoMinInterval?: string; autoValue?: string; @@ -76,7 +76,9 @@ export class SceneRefreshPicker extends SceneObjectBase }; public getUrlState() { - return { refresh: this.state.refresh !== '' ? this.state.refresh : undefined }; + return { + refresh: this.state.refresh, + }; } public updateFromUrl(values: SceneObjectUrlValues) { diff --git a/packages/scenes/src/core/SceneTimeRange.tsx b/packages/scenes/src/core/SceneTimeRange.tsx index 4a59e5954..74b422593 100644 --- a/packages/scenes/src/core/SceneTimeRange.tsx +++ b/packages/scenes/src/core/SceneTimeRange.tsx @@ -1,4 +1,4 @@ -import { getTimeZone, rangeUtil, setWeekStart, TimeRange } from '@grafana/data'; +import { getTimeZone, setWeekStart, TimeRange } from '@grafana/data'; import { TimeZone } from '@grafana/schema'; import { SceneObjectUrlSyncConfig } from '../services/SceneObjectUrlSyncConfig'; @@ -8,7 +8,7 @@ import { SceneTimeRangeLike, SceneTimeRangeState, SceneObjectUrlValues } from '. import { getClosest } from './sceneGraph/utils'; import { parseUrlParam } from '../utils/parseUrlParam'; import { evaluateTimeRange } from '../utils/evaluateTimeRange'; -import { config, RefreshEvent } from '@grafana/runtime'; +import { config } from '@grafana/runtime'; export class SceneTimeRange extends SceneObjectBase implements SceneTimeRangeLike { protected _urlSync = new SceneObjectUrlSyncConfig(this, { keys: ['from', 'to', 'timezone'] }); @@ -24,8 +24,7 @@ export class SceneTimeRange extends SceneObjectBase impleme state.fiscalYearStartMonth, state.UNSAFE_nowDelay ); - const refreshOnActivate = state.refreshOnActivate ?? { percent: 10 }; - super({ from, to, timeZone, value, refreshOnActivate, ...state }); + super({ from, to, timeZone, value, ...state }); this.addActivationHandler(this._onActivate.bind(this)); } @@ -57,10 +56,6 @@ export class SceneTimeRange extends SceneObjectBase impleme setWeekStart(this.state.weekStart); } - if (rangeUtil.isRelativeTimeRange(this.state.value.raw)) { - this.refreshIfStale(); - } - // Deactivation handler that restore weekStart if it was changed return () => { if (this.state.weekStart) { @@ -69,19 +64,6 @@ export class SceneTimeRange extends SceneObjectBase impleme }; } - private refreshIfStale() { - let ms; - if (this.state?.refreshOnActivate?.percent !== undefined) { - ms = this.calculatePercentOfInterval(this.state.refreshOnActivate.percent); - } - if (this.state?.refreshOnActivate?.afterMs !== undefined) { - ms = Math.min(this.state.refreshOnActivate.afterMs, ms ?? Infinity); - } - if (ms !== undefined) { - this.refreshRange(ms); - } - } - /** * Will traverse up the scene graph to find the closest SceneTimeRangeLike with time zone set */ @@ -104,33 +86,6 @@ export class SceneTimeRange extends SceneObjectBase impleme return source; } - /** - * Refreshes time range if it is older than the invalidation interval - * @param refreshAfterMs invalidation interval (milliseconds) - * @private - */ - private refreshRange(refreshAfterMs: number) { - const value = evaluateTimeRange( - this.state.from, - this.state.to, - this.state.timeZone ?? getTimeZone(), - this.state.fiscalYearStartMonth, - this.state.UNSAFE_nowDelay - ); - - const diff = value.to.diff(this.state.value.to, 'milliseconds'); - if (diff >= refreshAfterMs) { - this.setState({ - value, - }); - } - } - - private calculatePercentOfInterval(percent: number): number { - const intervalMs = this.state.value.to.diff(this.state.value.from, 'milliseconds'); - return Math.ceil(intervalMs / percent); - } - public getTimeZone(): TimeZone { // Return local time zone if provided if (this.state.timeZone) { @@ -195,8 +150,6 @@ export class SceneTimeRange extends SceneObjectBase impleme this.state.UNSAFE_nowDelay ), }); - - this.publishEvent(new RefreshEvent(), true); }; public getUrlState() { @@ -240,4 +193,8 @@ export class SceneTimeRange extends SceneObjectBase impleme this.setState(update); } + + public shouldCreateHistoryEntry(values: SceneObjectUrlValues): boolean { + return true; + } } diff --git a/packages/scenes/src/core/types.ts b/packages/scenes/src/core/types.ts index 43548cb7f..289ed33f3 100644 --- a/packages/scenes/src/core/types.ts +++ b/packages/scenes/src/core/types.ts @@ -156,13 +156,13 @@ export interface SceneTimeRangeState extends SceneObjectState { /** * When set, the time range will invalidate relative ranges after the specified interval has elapsed */ - afterMs?: number + afterMs?: number; /** * When set, the time range will invalidate relative ranges after the specified percentage of the current interval has elapsed. * If both invalidate values are set, the smaller value will be used for the given interval. */ - percent?: number - } + percent?: number; + }; } export interface SceneTimeRangeLike extends SceneObject { @@ -179,12 +179,14 @@ export function isSceneObject(obj: any): obj is SceneObject { export interface SceneObjectWithUrlSync extends SceneObject { getUrlState(): SceneObjectUrlValues; updateFromUrl(values: SceneObjectUrlValues): void; + shouldCreateHistoryEntry?(values: SceneObjectUrlValues): boolean; } export interface SceneObjectUrlSyncHandler { getKeys(): string[]; getUrlState(): SceneObjectUrlValues; updateFromUrl(values: SceneObjectUrlValues): void; + shouldCreateHistoryEntry?(values: SceneObjectUrlValues): boolean; } export interface DataRequestEnricher { diff --git a/packages/scenes/src/services/SceneObjectUrlSyncConfig.ts b/packages/scenes/src/services/SceneObjectUrlSyncConfig.ts index 48c91b3bd..4a31efde1 100644 --- a/packages/scenes/src/services/SceneObjectUrlSyncConfig.ts +++ b/packages/scenes/src/services/SceneObjectUrlSyncConfig.ts @@ -26,4 +26,8 @@ export class SceneObjectUrlSyncConfig implements SceneObjectUrlSyncHandler { public updateFromUrl(values: SceneObjectUrlValues): void { this._sceneObject.updateFromUrl(values); } + + public shouldCreateHistoryEntry(values: SceneObjectUrlValues): boolean { + return this._sceneObject.shouldCreateHistoryEntry?.(values) ?? false; + } } diff --git a/packages/scenes/src/services/UrlSyncManager.ts b/packages/scenes/src/services/UrlSyncManager.ts index a8c19382b..bd4ce7555 100644 --- a/packages/scenes/src/services/UrlSyncManager.ts +++ b/packages/scenes/src/services/UrlSyncManager.ts @@ -41,7 +41,12 @@ export class UrlSyncManager implements UrlSyncManagerLike { this._urlKeyMapper.clear(); this._lastLocation = locationService.getLocation(); + // Sync current url with state this.handleNewObject(this._sceneRoot); + + // Get current url state and update url to match + const urlState = getUrlState(root); + locationService.partial(urlState, true); } public cleanUp(root: SceneObject) { @@ -107,8 +112,11 @@ export class UrlSyncManager implements UrlSyncManagerLike { } if (Object.keys(mappedUpdated).length > 0) { + const shouldCreateHistoryEntry = changedObject.urlSync.shouldCreateHistoryEntry?.(newUrlState); + const shouldReplace = shouldCreateHistoryEntry !== true; + writeSceneLog('UrlSyncManager', 'onStateChange updating URL'); - locationService.partial(mappedUpdated, true); + locationService.partial(mappedUpdated, shouldReplace); /// Mark the location already handled this._lastLocation = locationService.getLocation(); From c8320d07d90e759520532ddb0bc85c217e99b2f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Mon, 26 Aug 2024 10:54:46 +0200 Subject: [PATCH 03/11] restore changes --- packages/scenes/src/core/SceneTimeRange.tsx | 53 +++++++++++++++++++-- 1 file changed, 50 insertions(+), 3 deletions(-) diff --git a/packages/scenes/src/core/SceneTimeRange.tsx b/packages/scenes/src/core/SceneTimeRange.tsx index 74b422593..64ce3bad4 100644 --- a/packages/scenes/src/core/SceneTimeRange.tsx +++ b/packages/scenes/src/core/SceneTimeRange.tsx @@ -1,4 +1,4 @@ -import { getTimeZone, setWeekStart, TimeRange } from '@grafana/data'; +import { getTimeZone, rangeUtil, setWeekStart, TimeRange } from '@grafana/data'; import { TimeZone } from '@grafana/schema'; import { SceneObjectUrlSyncConfig } from '../services/SceneObjectUrlSyncConfig'; @@ -8,7 +8,7 @@ import { SceneTimeRangeLike, SceneTimeRangeState, SceneObjectUrlValues } from '. import { getClosest } from './sceneGraph/utils'; import { parseUrlParam } from '../utils/parseUrlParam'; import { evaluateTimeRange } from '../utils/evaluateTimeRange'; -import { config } from '@grafana/runtime'; +import { config, RefreshEvent } from '@grafana/runtime'; export class SceneTimeRange extends SceneObjectBase implements SceneTimeRangeLike { protected _urlSync = new SceneObjectUrlSyncConfig(this, { keys: ['from', 'to', 'timezone'] }); @@ -24,7 +24,8 @@ export class SceneTimeRange extends SceneObjectBase impleme state.fiscalYearStartMonth, state.UNSAFE_nowDelay ); - super({ from, to, timeZone, value, ...state }); + const refreshOnActivate = state.refreshOnActivate ?? { percent: 10 }; + super({ from, to, timeZone, value, refreshOnActivate, ...state }); this.addActivationHandler(this._onActivate.bind(this)); } @@ -56,6 +57,10 @@ export class SceneTimeRange extends SceneObjectBase impleme setWeekStart(this.state.weekStart); } + if (rangeUtil.isRelativeTimeRange(this.state.value.raw)) { + this.refreshIfStale(); + } + // Deactivation handler that restore weekStart if it was changed return () => { if (this.state.weekStart) { @@ -64,6 +69,19 @@ export class SceneTimeRange extends SceneObjectBase impleme }; } + private refreshIfStale() { + let ms; + if (this.state?.refreshOnActivate?.percent !== undefined) { + ms = this.calculatePercentOfInterval(this.state.refreshOnActivate.percent); + } + if (this.state?.refreshOnActivate?.afterMs !== undefined) { + ms = Math.min(this.state.refreshOnActivate.afterMs, ms ?? Infinity); + } + if (ms !== undefined) { + this.refreshRange(ms); + } + } + /** * Will traverse up the scene graph to find the closest SceneTimeRangeLike with time zone set */ @@ -86,6 +104,33 @@ export class SceneTimeRange extends SceneObjectBase impleme return source; } + /** + * Refreshes time range if it is older than the invalidation interval + * @param refreshAfterMs invalidation interval (milliseconds) + * @private + */ + private refreshRange(refreshAfterMs: number) { + const value = evaluateTimeRange( + this.state.from, + this.state.to, + this.state.timeZone ?? getTimeZone(), + this.state.fiscalYearStartMonth, + this.state.UNSAFE_nowDelay + ); + + const diff = value.to.diff(this.state.value.to, 'milliseconds'); + if (diff >= refreshAfterMs) { + this.setState({ + value, + }); + } + } + + private calculatePercentOfInterval(percent: number): number { + const intervalMs = this.state.value.to.diff(this.state.value.from, 'milliseconds'); + return Math.ceil(intervalMs / percent); + } + public getTimeZone(): TimeZone { // Return local time zone if provided if (this.state.timeZone) { @@ -150,6 +195,8 @@ export class SceneTimeRange extends SceneObjectBase impleme this.state.UNSAFE_nowDelay ), }); + + this.publishEvent(new RefreshEvent(), true); }; public getUrlState() { From 0e494cf6ebe728251ed58b299821e2ba486e57e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Mon, 26 Aug 2024 15:28:35 +0200 Subject: [PATCH 04/11] added tests --- .../src/services/UrlSyncManager.test.ts | 52 +++++++++++++++---- .../scenes/src/services/UrlSyncManager.ts | 15 +++++- 2 files changed, 56 insertions(+), 11 deletions(-) diff --git a/packages/scenes/src/services/UrlSyncManager.test.ts b/packages/scenes/src/services/UrlSyncManager.test.ts index ce12763f8..5318e8148 100644 --- a/packages/scenes/src/services/UrlSyncManager.test.ts +++ b/packages/scenes/src/services/UrlSyncManager.test.ts @@ -60,6 +60,7 @@ describe('UrlSyncManager', () => { beforeEach(() => { locationUpdates = []; + deactivate = () => {}; listenUnregister = locationService.getHistory().listen((location) => { locationUpdates.push(location); }); @@ -112,6 +113,35 @@ describe('UrlSyncManager', () => { obj.setState({ other: 'not synced' }); // Should not update url + expect(locationUpdates.length).toBe(2); + }); + }); + + describe('Initiating url from state', () => { + it('Should sync initial scene state with url', () => { + const obj = new TestObj({ name: 'test' }); + scene = new SceneFlexLayout({ + children: [new SceneFlexItem({ body: obj })], + }); + + urlManager = new UrlSyncManager(); + urlManager.initSync(scene); + + expect(locationUpdates.length).toBe(1); + expect(locationUpdates[0].search).toBe('?name=test'); + }); + + it('Should not update url if there is no difference', () => { + const obj = new TestObj({ name: 'test' }); + scene = new SceneFlexLayout({ + children: [new SceneFlexItem({ body: obj })], + }); + + locationService.partial({ name: 'test' }); + + urlManager = new UrlSyncManager(); + urlManager.initSync(scene); + expect(locationUpdates.length).toBe(1); }); }); @@ -238,7 +268,9 @@ describe('UrlSyncManager', () => { // Should use unique key based where it is in the scene expect(locationService.getSearchObject()).toEqual({ + from: 'now-6h', ['from-2']: 'now-10m', + to: 'now', ['to-2']: 'now', }); @@ -259,7 +291,7 @@ describe('UrlSyncManager', () => { // should not update the first object expect(outerTimeRange.state.from).toBe('now-20m'); // Should not cause another url update - expect(locationUpdates.length).toBe(3); + expect(locationUpdates.length).toBe(4); }); it('should handle dynamically added objects that use same key', () => { @@ -323,7 +355,7 @@ describe('UrlSyncManager', () => { obj.setState({ other: 'not synced' }); // Should not update url - expect(locationUpdates.length).toBe(1); + expect(locationUpdates.length).toBe(2); // When updating via url updateUrlStateAndSyncState({ array: ['A', 'B', 'C'] }, urlManager); @@ -448,7 +480,7 @@ describe('UrlSyncManager', () => { obj1.setState({ name: 'B' }); // Should not update url - expect(locationService.getSearchObject().name).toBeUndefined(); + expect(locationService.getSearchObject().name).toBe('A'); // When updating via url updateUrlStateAndSyncState({ name: 'Hello' }, urlManager); @@ -487,23 +519,23 @@ describe('UrlSyncManager', () => { describe('When init sync root is not scene root', () => { it('Should sync init root', async () => { - const scene = new TestObj({ + const scene = new TestObj({ name: 'scene-root', - nested: new TestObj({ + nested: new TestObj({ name: 'url-sync-root', - }) - }); + }), + }); urlManager = new UrlSyncManager(); - + locationService.push(`/?name=test1`); urlManager.initSync(scene.state.nested!); - deactivate = activateFullSceneTree(scene); + deactivate = activateFullSceneTree(scene); // Only updated the nested scene (as it's the only part of scene tree that is synced) expect(scene.state.nested?.state.name).toEqual('test1'); - + // Unchanged expect(scene.state.name).toEqual('scene-root'); }); diff --git a/packages/scenes/src/services/UrlSyncManager.ts b/packages/scenes/src/services/UrlSyncManager.ts index bd4ce7555..a8324041c 100644 --- a/packages/scenes/src/services/UrlSyncManager.ts +++ b/packages/scenes/src/services/UrlSyncManager.ts @@ -46,7 +46,10 @@ export class UrlSyncManager implements UrlSyncManagerLike { // Get current url state and update url to match const urlState = getUrlState(root); - locationService.partial(urlState, true); + + if (isUrlStateDifferent(urlState, this._paramsCache.getParams())) { + locationService.partial(urlState, true); + } } public cleanUp(root: SceneObject) { @@ -147,6 +150,16 @@ class UrlParamsCache { } } +function isUrlStateDifferent(sceneUrlState: SceneObjectUrlValues, currentParams: URLSearchParams) { + for (let key in sceneUrlState) { + if (!isUrlValueEqual(currentParams.getAll(key), sceneUrlState[key])) { + return true; + } + } + + return false; +} + let urlSyncManager: UrlSyncManagerLike | undefined; export function getUrlSyncManager(): UrlSyncManagerLike { From 3366a2949d06cc5ecdfea714787e423f924e22bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Tue, 27 Aug 2024 08:11:14 +0200 Subject: [PATCH 05/11] Before removing singleton --- packages/scenes/src/core/types.ts | 4 ++-- .../src/services/SceneObjectUrlSyncConfig.ts | 4 ++-- .../scenes/src/services/UrlSyncManager.ts | 22 +++++++++++++++++-- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/packages/scenes/src/core/types.ts b/packages/scenes/src/core/types.ts index 289ed33f3..b20c66b07 100644 --- a/packages/scenes/src/core/types.ts +++ b/packages/scenes/src/core/types.ts @@ -179,14 +179,14 @@ export function isSceneObject(obj: any): obj is SceneObject { export interface SceneObjectWithUrlSync extends SceneObject { getUrlState(): SceneObjectUrlValues; updateFromUrl(values: SceneObjectUrlValues): void; - shouldCreateHistoryEntry?(values: SceneObjectUrlValues): boolean; + shouldCreateHistoryStep?(values: SceneObjectUrlValues): boolean; } export interface SceneObjectUrlSyncHandler { getKeys(): string[]; getUrlState(): SceneObjectUrlValues; updateFromUrl(values: SceneObjectUrlValues): void; - shouldCreateHistoryEntry?(values: SceneObjectUrlValues): boolean; + shouldCreateHistoryStep?(values: SceneObjectUrlValues): boolean; } export interface DataRequestEnricher { diff --git a/packages/scenes/src/services/SceneObjectUrlSyncConfig.ts b/packages/scenes/src/services/SceneObjectUrlSyncConfig.ts index 4a31efde1..cae23f1e6 100644 --- a/packages/scenes/src/services/SceneObjectUrlSyncConfig.ts +++ b/packages/scenes/src/services/SceneObjectUrlSyncConfig.ts @@ -27,7 +27,7 @@ export class SceneObjectUrlSyncConfig implements SceneObjectUrlSyncHandler { this._sceneObject.updateFromUrl(values); } - public shouldCreateHistoryEntry(values: SceneObjectUrlValues): boolean { - return this._sceneObject.shouldCreateHistoryEntry?.(values) ?? false; + public shouldCreateHistoryStep(values: SceneObjectUrlValues): boolean { + return this._sceneObject.shouldCreateHistoryStep?.(values) ?? false; } } diff --git a/packages/scenes/src/services/UrlSyncManager.ts b/packages/scenes/src/services/UrlSyncManager.ts index a8324041c..975d5b1cd 100644 --- a/packages/scenes/src/services/UrlSyncManager.ts +++ b/packages/scenes/src/services/UrlSyncManager.ts @@ -12,17 +12,35 @@ import { getUrlState, isUrlValueEqual, syncStateFromUrl } from './utils'; export interface UrlSyncManagerLike { initSync(root: SceneObject): void; cleanUp(root: SceneObject): void; - getUrlState(root: SceneObject): SceneObjectUrlValues; handleNewLocation(location: Location): void; handleNewObject(sceneObj: SceneObject): void; } +export interface UrlSyncManagerOptions { + /** + * This will update the url to contain all scene url state + * when the scene is initialized. + */ + updateUrlOnInit?: boolean; + /** + * This is only supported by some objects if they implement + * shouldCreateHistoryStep where they can control what changes + * url changes should add a new browser history entry. + */ + createBrowserHistoryStep?: boolean; +} + export class UrlSyncManager implements UrlSyncManagerLike { private _urlKeyMapper = new UniqueUrlKeyMapper(); private _sceneRoot?: SceneObject; private _stateSub: Unsubscribable | null = null; private _lastLocation: Location | undefined; private _paramsCache = new UrlParamsCache(); + private _options: UrlSyncManagerOptions; + + public constructor(_options: UrlSyncManagerOptions = {}) { + this._options = _options; + } /** * Updates the current scene state to match URL state. @@ -115,7 +133,7 @@ export class UrlSyncManager implements UrlSyncManagerLike { } if (Object.keys(mappedUpdated).length > 0) { - const shouldCreateHistoryEntry = changedObject.urlSync.shouldCreateHistoryEntry?.(newUrlState); + const shouldCreateHistoryEntry = changedObject.urlSync.shouldCreateHistoryStep?.(newUrlState); const shouldReplace = shouldCreateHistoryEntry !== true; writeSceneLog('UrlSyncManager', 'onStateChange updating URL'); From 728207375486cadc06efc517912b8d86c9d317d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Tue, 27 Aug 2024 08:56:04 +0200 Subject: [PATCH 06/11] Update --- packages/scenes/src/core/sceneGraph/index.ts | 2 + .../scenes/src/core/sceneGraph/sceneGraph.ts | 18 ++++ .../scenes/src/services/UrlSyncManager.ts | 90 +++++++++++-------- 3 files changed, 71 insertions(+), 39 deletions(-) diff --git a/packages/scenes/src/core/sceneGraph/index.ts b/packages/scenes/src/core/sceneGraph/index.ts index c22c4454b..2c407ee8c 100644 --- a/packages/scenes/src/core/sceneGraph/index.ts +++ b/packages/scenes/src/core/sceneGraph/index.ts @@ -13,6 +13,7 @@ import { interpolate, getAncestor, getQueryController, + getUrlSyncManager, } from './sceneGraph'; export const sceneGraph = { @@ -30,4 +31,5 @@ export const sceneGraph = { findAllObjects, getAncestor, getQueryController, + getUrlSyncManager, }; diff --git a/packages/scenes/src/core/sceneGraph/sceneGraph.ts b/packages/scenes/src/core/sceneGraph/sceneGraph.ts index 169729210..cdbac86e4 100644 --- a/packages/scenes/src/core/sceneGraph/sceneGraph.ts +++ b/packages/scenes/src/core/sceneGraph/sceneGraph.ts @@ -10,6 +10,7 @@ import { getClosest } from './utils'; import { SceneQueryControllerLike, isQueryController } from '../../behaviors/SceneQueryController'; import { VariableInterpolation } from '@grafana/runtime'; import { QueryVariable } from '../../variables/variants/query/QueryVariable'; +import { UrlSyncManagerLike } from '../../services/UrlSyncManager'; /** * Get the closest node with variables @@ -269,3 +270,20 @@ export function getQueryController(sceneObject: SceneObject): SceneQueryControll return undefined; } + +/** + * Returns the closest SceneObject that has a state property with the + * name urlSyncManager that is of type UrlSyncManager + */ +export function getUrlSyncManager(sceneObject: SceneObject): UrlSyncManagerLike | undefined { + let parent: SceneObject | undefined = sceneObject; + + while (parent) { + if ('urlSyncManager' in parent.state) { + return parent.state.urlSyncManager as UrlSyncManagerLike; + } + parent = parent.parent; + } + + return undefined; +} diff --git a/packages/scenes/src/services/UrlSyncManager.ts b/packages/scenes/src/services/UrlSyncManager.ts index 975d5b1cd..b3b3bf729 100644 --- a/packages/scenes/src/services/UrlSyncManager.ts +++ b/packages/scenes/src/services/UrlSyncManager.ts @@ -5,9 +5,10 @@ import { locationService } from '@grafana/runtime'; import { SceneObjectStateChangedEvent } from '../core/events'; import { SceneObject, SceneObjectUrlValues } from '../core/types'; import { writeSceneLog } from '../utils/writeSceneLog'; -import { Unsubscribable } from 'rxjs'; +import { Subscription } from 'rxjs'; import { UniqueUrlKeyMapper } from './UniqueUrlKeyMapper'; import { getUrlState, isUrlValueEqual, syncStateFromUrl } from './utils'; +import { BusEventWithPayload } from '@grafana/data'; export interface UrlSyncManagerLike { initSync(root: SceneObject): void; @@ -30,10 +31,18 @@ export interface UrlSyncManagerOptions { createBrowserHistoryStep?: boolean; } +/** + * Notify the url sync manager of a new object that has been added to the scene + * that needs to init state from URL. + */ +export class NewSceneObjectAddedEvent extends BusEventWithPayload { + public static readonly type = 'new-scene-object-added'; +} + export class UrlSyncManager implements UrlSyncManagerLike { private _urlKeyMapper = new UniqueUrlKeyMapper(); private _sceneRoot?: SceneObject; - private _stateSub: Unsubscribable | null = null; + private _subs: Subscription | undefined; private _lastLocation: Location | undefined; private _paramsCache = new UrlParamsCache(); private _options: UrlSyncManagerOptions; @@ -46,15 +55,27 @@ export class UrlSyncManager implements UrlSyncManagerLike { * Updates the current scene state to match URL state. */ public initSync(root: SceneObject) { - if (this._stateSub) { + if (this._subs) { writeSceneLog('UrlSyncManager', 'Unregister previous scene state subscription', this._sceneRoot?.state.key); - this._stateSub.unsubscribe(); + this._subs.unsubscribe(); } writeSceneLog('UrlSyncManager', 'init', root.state.key); this._sceneRoot = root; - this._stateSub = root.subscribeToEvent(SceneObjectStateChangedEvent, this.#onStateChanged); + this._subs = new Subscription(); + + this._subs.add( + root.subscribeToEvent(SceneObjectStateChangedEvent, (evt) => { + this.handleSceneObjectStateChanged(evt.payload.changedObject); + }) + ); + + this._subs.add( + root.subscribeToEvent(NewSceneObjectAddedEvent, (evt) => { + this.handleNewObject(evt.payload); + }) + ); this._urlKeyMapper.clear(); this._lastLocation = locationService.getLocation(); @@ -78,9 +99,10 @@ export class UrlSyncManager implements UrlSyncManagerLike { writeSceneLog('UrlSyncManager', 'Clean up'); - if (this._stateSub) { - this._stateSub.unsubscribe(); - this._stateSub = null; + if (this._subs) { + this._subs.unsubscribe(); + this._subs = undefined; + writeSceneLog( 'UrlSyncManager', 'Root deactived, unsub to state', @@ -114,36 +136,36 @@ export class UrlSyncManager implements UrlSyncManagerLike { syncStateFromUrl(sceneObj, this._paramsCache.getParams(), this._urlKeyMapper); } - #onStateChanged = ({ payload }: SceneObjectStateChangedEvent) => { - const changedObject = payload.changedObject; + private handleSceneObjectStateChanged(changedObject: SceneObject) { + if (!changedObject.urlSync) { + return; + } - if (changedObject.urlSync) { - const newUrlState = changedObject.urlSync.getUrlState(); + const newUrlState = changedObject.urlSync.getUrlState(); - const searchParams = locationService.getSearch(); - const mappedUpdated: SceneObjectUrlValues = {}; + const searchParams = locationService.getSearch(); + const mappedUpdated: SceneObjectUrlValues = {}; - for (const [key, newUrlValue] of Object.entries(newUrlState)) { - const uniqueKey = this._urlKeyMapper.getUniqueKey(key, changedObject); - const currentUrlValue = searchParams.getAll(uniqueKey); + for (const [key, newUrlValue] of Object.entries(newUrlState)) { + const uniqueKey = this._urlKeyMapper.getUniqueKey(key, changedObject); + const currentUrlValue = searchParams.getAll(uniqueKey); - if (!isUrlValueEqual(currentUrlValue, newUrlValue)) { - mappedUpdated[uniqueKey] = newUrlValue; - } + if (!isUrlValueEqual(currentUrlValue, newUrlValue)) { + mappedUpdated[uniqueKey] = newUrlValue; } + } - if (Object.keys(mappedUpdated).length > 0) { - const shouldCreateHistoryEntry = changedObject.urlSync.shouldCreateHistoryStep?.(newUrlState); - const shouldReplace = shouldCreateHistoryEntry !== true; + if (Object.keys(mappedUpdated).length > 0) { + const shouldCreateHistoryEntry = changedObject.urlSync.shouldCreateHistoryStep?.(newUrlState); + const shouldReplace = shouldCreateHistoryEntry !== true; - writeSceneLog('UrlSyncManager', 'onStateChange updating URL'); - locationService.partial(mappedUpdated, shouldReplace); + writeSceneLog('UrlSyncManager', 'onStateChange updating URL'); + locationService.partial(mappedUpdated, shouldReplace); - /// Mark the location already handled - this._lastLocation = locationService.getLocation(); - } + /// Mark the location already handled + this._lastLocation = locationService.getLocation(); } - }; + } public getUrlState(root: SceneObject): SceneObjectUrlValues { return getUrlState(root); @@ -177,13 +199,3 @@ function isUrlStateDifferent(sceneUrlState: SceneObjectUrlValues, currentParams: return false; } - -let urlSyncManager: UrlSyncManagerLike | undefined; - -export function getUrlSyncManager(): UrlSyncManagerLike { - if (!urlSyncManager) { - urlSyncManager = new UrlSyncManager(); - } - - return urlSyncManager; -} From 720b397473a59ea80f1de07fdc3b08d820ed4577 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Tue, 27 Aug 2024 10:41:15 +0200 Subject: [PATCH 07/11] Progress --- packages/scenes-app/src/demos/urlSyncTest.tsx | 4 +-- .../src/contexts/SceneContextObject.tsx | 8 +++--- .../src/contexts/SceneContextProvider.tsx | 9 +------ packages/scenes/src/index.ts | 2 +- .../src/services/UrlSyncContextProvider.ts | 12 ++++++--- .../scenes/src/services/UrlSyncManager.ts | 27 +++++++++++++++---- packages/scenes/src/services/useUrlSync.ts | 8 +++--- 7 files changed, 44 insertions(+), 26 deletions(-) diff --git a/packages/scenes-app/src/demos/urlSyncTest.tsx b/packages/scenes-app/src/demos/urlSyncTest.tsx index 29d97a3d0..3140bd1c4 100644 --- a/packages/scenes-app/src/demos/urlSyncTest.tsx +++ b/packages/scenes-app/src/demos/urlSyncTest.tsx @@ -18,10 +18,10 @@ import { SceneTimeRange, SceneVariableSet, VariableValueSelectors, - getUrlSyncManager, } from '@grafana/scenes'; import { getQueryRunnerWithRandomWalkQuery } from './utils'; import { Button, Stack } from '@grafana/ui'; +import { NewSceneObjectAddedEvent } from '@grafana/scenes/src/services/UrlSyncManager'; export function getUrlSyncTest(defaults: SceneAppPageState) { return new SceneAppPage({ @@ -102,7 +102,7 @@ class DynamicSubScene extends SceneObjectBase { private addScene() { const scene = buildNewSubScene(); - getUrlSyncManager().handleNewObject(scene); + this.publishEvent(new NewSceneObjectAddedEvent(scene), true); this.setState({ scene }); } diff --git a/packages/scenes-react/src/contexts/SceneContextObject.tsx b/packages/scenes-react/src/contexts/SceneContextObject.tsx index 9ace0c2dc..cdee74998 100644 --- a/packages/scenes-react/src/contexts/SceneContextObject.tsx +++ b/packages/scenes-react/src/contexts/SceneContextObject.tsx @@ -4,7 +4,7 @@ import { SceneObjectState, SceneVariable, SceneVariableSet, - getUrlSyncManager, + NewSceneObjectAddedEvent, } from '@grafana/scenes'; import { writeSceneLog } from '../utils'; @@ -23,7 +23,7 @@ export class SceneContextObject extends SceneObjectBase } public addToScene(obj: SceneObject) { - getUrlSyncManager().handleNewObject(obj); + this.publishEvent(new NewSceneObjectAddedEvent(obj), true); this.setState({ children: [...this.state.children, obj] }); writeSceneLog('SceneContext', `Adding to scene: ${obj.constructor.name} key: ${obj.state.key}`); @@ -54,7 +54,7 @@ export class SceneContextObject extends SceneObjectBase public addVariable(variable: SceneVariable) { let set = this.state.$variables as SceneVariableSet; - getUrlSyncManager().handleNewObject(variable); + this.publishEvent(new NewSceneObjectAddedEvent(set), true); if (set) { set.setState({ variables: [...set.state.variables, variable] }); @@ -72,6 +72,8 @@ export class SceneContextObject extends SceneObjectBase } public addChildContext(ctx: SceneContextObject) { + this.publishEvent(new NewSceneObjectAddedEvent(ctx), true); + this.setState({ childContexts: [...(this.state.childContexts ?? []), ctx] }); writeSceneLog('SceneContext', `Adding child context: ${ctx.constructor.name} key: ${ctx.state.key}`); diff --git a/packages/scenes-react/src/contexts/SceneContextProvider.tsx b/packages/scenes-react/src/contexts/SceneContextProvider.tsx index 3193f934d..c21c9c6ee 100644 --- a/packages/scenes-react/src/contexts/SceneContextProvider.tsx +++ b/packages/scenes-react/src/contexts/SceneContextProvider.tsx @@ -1,11 +1,5 @@ import React, { createContext, useContext, useEffect, useState } from 'react'; -import { - SceneTimeRangeState, - SceneTimeRange, - behaviors, - UrlSyncContextProvider, - getUrlSyncManager, -} from '@grafana/scenes'; +import { SceneTimeRangeState, SceneTimeRange, behaviors, UrlSyncContextProvider } from '@grafana/scenes'; import { SceneContextObject, SceneContextObjectState } from './SceneContextObject'; @@ -51,7 +45,6 @@ export function SceneContextProvider({ children, timeRange, withQueryController const childContext = new SceneContextObject(state); if (parentContext) { - getUrlSyncManager().handleNewObject(childContext); parentContext.addChildContext(childContext); } diff --git a/packages/scenes/src/index.ts b/packages/scenes/src/index.ts index 0fd858704..90c581b37 100644 --- a/packages/scenes/src/index.ts +++ b/packages/scenes/src/index.ts @@ -69,7 +69,7 @@ export { AdHocFiltersVariable } from './variables/adhoc/AdHocFiltersVariable'; export { GroupByVariable } from './variables/groupby/GroupByVariable'; export { type MacroVariableConstructor } from './variables/macros/types'; -export { type UrlSyncManagerLike, UrlSyncManager, getUrlSyncManager } from './services/UrlSyncManager'; +export { type UrlSyncManagerLike, UrlSyncManager, NewSceneObjectAddedEvent } from './services/UrlSyncManager'; export { useUrlSync } from './services/useUrlSync'; export { UrlSyncContextProvider } from './services/UrlSyncContextProvider'; export { SceneObjectUrlSyncConfig } from './services/SceneObjectUrlSyncConfig'; diff --git a/packages/scenes/src/services/UrlSyncContextProvider.ts b/packages/scenes/src/services/UrlSyncContextProvider.ts index f2858da67..3c70e7a40 100644 --- a/packages/scenes/src/services/UrlSyncContextProvider.ts +++ b/packages/scenes/src/services/UrlSyncContextProvider.ts @@ -1,7 +1,8 @@ import { SceneObject } from '../core/types'; +import { UrlSyncManagerOptions } from './UrlSyncManager'; import { useUrlSync } from './useUrlSync'; -export interface UrlSyncContextProviderProps { +export interface UrlSyncContextProviderProps extends UrlSyncManagerOptions { scene: SceneObject; children: React.ReactNode; } @@ -10,8 +11,13 @@ export interface UrlSyncContextProviderProps { * Right now this is actually not defining a context, but think it might in the future (with UrlSyncManager as the context value) */ -export function UrlSyncContextProvider({ children, scene }: UrlSyncContextProviderProps) { - const isInitialized = useUrlSync(scene); +export function UrlSyncContextProvider({ + children, + scene, + updateUrlOnInit, + createBrowserHistorySteps, +}: UrlSyncContextProviderProps) { + const isInitialized = useUrlSync(scene, { updateUrlOnInit, createBrowserHistorySteps }); if (!isInitialized) { return null; diff --git a/packages/scenes/src/services/UrlSyncManager.ts b/packages/scenes/src/services/UrlSyncManager.ts index b3b3bf729..04b5dbbeb 100644 --- a/packages/scenes/src/services/UrlSyncManager.ts +++ b/packages/scenes/src/services/UrlSyncManager.ts @@ -9,6 +9,7 @@ import { Subscription } from 'rxjs'; import { UniqueUrlKeyMapper } from './UniqueUrlKeyMapper'; import { getUrlState, isUrlValueEqual, syncStateFromUrl } from './utils'; import { BusEventWithPayload } from '@grafana/data'; +import { useMemo } from 'react'; export interface UrlSyncManagerLike { initSync(root: SceneObject): void; @@ -28,7 +29,7 @@ export interface UrlSyncManagerOptions { * shouldCreateHistoryStep where they can control what changes * url changes should add a new browser history entry. */ - createBrowserHistoryStep?: boolean; + createBrowserHistorySteps?: boolean; } /** @@ -83,11 +84,13 @@ export class UrlSyncManager implements UrlSyncManagerLike { // Sync current url with state this.handleNewObject(this._sceneRoot); - // Get current url state and update url to match - const urlState = getUrlState(root); + if (this._options.updateUrlOnInit) { + // Get current url state and update url to match + const urlState = getUrlState(root); - if (isUrlStateDifferent(urlState, this._paramsCache.getParams())) { - locationService.partial(urlState, true); + if (isUrlStateDifferent(urlState, this._paramsCache.getParams())) { + locationService.partial(urlState, true); + } } } @@ -199,3 +202,17 @@ function isUrlStateDifferent(sceneUrlState: SceneObjectUrlValues, currentParams: return false; } + +/** + * Creates a new memoized instance of the UrlSyncManager based on options + */ +export function useUrlSyncManager(options: UrlSyncManagerOptions): UrlSyncManagerLike { + return useMemo( + () => + new UrlSyncManager({ + updateUrlOnInit: options.updateUrlOnInit, + createBrowserHistorySteps: options.createBrowserHistorySteps, + }), + [options.updateUrlOnInit, options.createBrowserHistorySteps] + ); +} diff --git a/packages/scenes/src/services/useUrlSync.ts b/packages/scenes/src/services/useUrlSync.ts index 653cb8f2b..6a2b098e5 100644 --- a/packages/scenes/src/services/useUrlSync.ts +++ b/packages/scenes/src/services/useUrlSync.ts @@ -1,14 +1,14 @@ import { SceneObject } from '../core/types'; import { useEffect, useState } from 'react'; import { useLocation } from 'react-router-dom'; -import { getUrlSyncManager } from './UrlSyncManager'; import { locationService } from '@grafana/runtime'; import { writeSceneLog } from '../utils/writeSceneLog'; +import { UrlSyncManagerOptions, useUrlSyncManager } from './UrlSyncManager'; -export function useUrlSync(sceneRoot: SceneObject): boolean { - const urlSyncManager = getUrlSyncManager(); +export function useUrlSync(sceneRoot: SceneObject, options: UrlSyncManagerOptions = {}): boolean { const location = useLocation(); const [isInitialized, setIsInitialized] = useState(false); + const urlSyncManager = useUrlSyncManager(options); useEffect(() => { urlSyncManager.initSync(sceneRoot); @@ -22,7 +22,7 @@ export function useUrlSync(sceneRoot: SceneObject): boolean { const locationToHandle = latestLocation !== location ? latestLocation : location; if (latestLocation !== location) { - writeSceneLog('useUrlSync', 'latestLocation different from location') + writeSceneLog('useUrlSync', 'latestLocation different from location'); } urlSyncManager.handleNewLocation(locationToHandle); From 6b606439cb01c06b2dad84b7e7289fb9c8e42cf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 28 Aug 2024 11:28:23 +0200 Subject: [PATCH 08/11] Progress --- .../scenes-app/src/pages/DemoListPage.tsx | 4 +++ .../src/components/SceneApp/SceneApp.tsx | 26 +++++++++++-------- .../components/SceneApp/SceneAppPageView.tsx | 8 +++--- .../scenes/src/components/SceneApp/types.ts | 5 ++-- packages/scenes/src/core/types.ts | 14 ++++++++++ .../src/services/UrlSyncContextProvider.ts | 5 ++-- .../src/services/UrlSyncManager.test.ts | 10 +++---- .../scenes/src/services/UrlSyncManager.ts | 22 +++------------- packages/scenes/src/services/useUrlSync.ts | 6 ++--- 9 files changed, 55 insertions(+), 45 deletions(-) diff --git a/packages/scenes-app/src/pages/DemoListPage.tsx b/packages/scenes-app/src/pages/DemoListPage.tsx index a601444b5..9fc19ea1c 100644 --- a/packages/scenes-app/src/pages/DemoListPage.tsx +++ b/packages/scenes-app/src/pages/DemoListPage.tsx @@ -24,6 +24,10 @@ import { css } from '@emotion/css'; function getDemoSceneApp() { return new SceneApp({ name: 'scenes-demos-app', + urlSyncOptions: { + updateUrlOnInit: true, + createBrowserHistorySteps: true, + }, pages: [ new SceneAppPage({ title: 'Demos', diff --git a/packages/scenes/src/components/SceneApp/SceneApp.tsx b/packages/scenes/src/components/SceneApp/SceneApp.tsx index e5491358d..fc4583fc4 100644 --- a/packages/scenes/src/components/SceneApp/SceneApp.tsx +++ b/packages/scenes/src/components/SceneApp/SceneApp.tsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { createContext } from 'react'; import { Route, Switch } from 'react-router-dom'; import { DataRequestEnricher, SceneComponentProps } from '../../core/types'; @@ -20,20 +20,24 @@ export class SceneApp extends SceneObjectBase implements DataRequ const { pages } = model.useState(); return ( - - {pages.map((page) => ( - renderSceneComponentWithRouteProps(page, props)} - > - ))} - + + + {pages.map((page) => ( + renderSceneComponentWithRouteProps(page, props)} + > + ))} + + ); }; } +export const SceneAppContext = createContext(null); + const sceneAppCache = new Map(); /** diff --git a/packages/scenes/src/components/SceneApp/SceneAppPageView.tsx b/packages/scenes/src/components/SceneApp/SceneAppPageView.tsx index 41555dc97..b9a544786 100644 --- a/packages/scenes/src/components/SceneApp/SceneAppPageView.tsx +++ b/packages/scenes/src/components/SceneApp/SceneAppPageView.tsx @@ -1,6 +1,6 @@ import { NavModelItem, UrlQueryMap } from '@grafana/data'; import { PluginPage } from '@grafana/runtime'; -import React, { useEffect, useLayoutEffect } from 'react'; +import React, { useContext, useEffect, useLayoutEffect } from 'react'; import { RouteComponentProps } from 'react-router-dom'; import { SceneObject } from '../../core/types'; @@ -9,6 +9,7 @@ import { SceneAppPage } from './SceneAppPage'; import { SceneAppDrilldownView, SceneAppPageLike } from './types'; import { getUrlWithAppState, renderSceneComponentWithRouteProps, useAppQueryParams } from './utils'; import { useUrlSync } from '../../services/useUrlSync'; +import { SceneAppContext } from './SceneApp'; export interface Props { page: SceneAppPageLike; @@ -21,8 +22,9 @@ export function SceneAppPageView({ page, routeProps }: Props) { const containerState = containerPage.useState(); const params = useAppQueryParams(); const scene = page.getScene(routeProps.match); + const appContext = useContext(SceneAppContext); const isInitialized = containerState.initializedScene === scene; - const {layout} = page.state; + const { layout } = page.state; useLayoutEffect(() => { // Before rendering scene components, we are making sure the URL sync is enabled for. @@ -36,7 +38,7 @@ export function SceneAppPageView({ page, routeProps }: Props) { return () => containerPage.setState({ initializedScene: undefined }); }, [containerPage]); - const urlSyncInitialized = useUrlSync(containerPage); + const urlSyncInitialized = useUrlSync(containerPage, appContext?.state.urlSyncOptions); if (!isInitialized && !urlSyncInitialized) { return null; diff --git a/packages/scenes/src/components/SceneApp/types.ts b/packages/scenes/src/components/SceneApp/types.ts index cf4140576..054c8c72b 100644 --- a/packages/scenes/src/components/SceneApp/types.ts +++ b/packages/scenes/src/components/SceneApp/types.ts @@ -1,5 +1,5 @@ import { ComponentType } from 'react'; -import { DataRequestEnricher, SceneObject, SceneObjectState } from '../../core/types'; +import { DataRequestEnricher, SceneObject, SceneObjectState, SceneUrlSyncOptions } from '../../core/types'; import { EmbeddedScene } from '../EmbeddedScene'; import { IconName, PageLayoutType } from '@grafana/data'; @@ -14,6 +14,7 @@ export interface SceneAppState extends SceneObjectState { // Array of SceneAppPage objects that are considered app's top level pages pages: SceneAppPageLike[]; name?: string; + urlSyncOptions?: SceneUrlSyncOptions; } export interface SceneAppRoute { @@ -67,7 +68,7 @@ export interface SceneAppPageState extends SceneObjectState { */ getFallbackPage?: () => SceneAppPageLike; - layout?: PageLayoutType + layout?: PageLayoutType; } export interface SceneAppPageLike extends SceneObject, DataRequestEnricher { diff --git a/packages/scenes/src/core/types.ts b/packages/scenes/src/core/types.ts index b20c66b07..a40fa57fd 100644 --- a/packages/scenes/src/core/types.ts +++ b/packages/scenes/src/core/types.ts @@ -278,3 +278,17 @@ export interface SceneDataQuery extends DataQuery { // Opt this query out of time window comparison timeRangeCompare?: boolean; } + +export interface SceneUrlSyncOptions { + /** + * This will update the url to contain all scene url state + * when the scene is initialized. Important for browser history "back" actions. + */ + updateUrlOnInit?: boolean; + /** + * This is only supported by some objects if they implement + * shouldCreateHistoryStep where they can control what changes + * url changes should add a new browser history entry. + */ + createBrowserHistorySteps?: boolean; +} diff --git a/packages/scenes/src/services/UrlSyncContextProvider.ts b/packages/scenes/src/services/UrlSyncContextProvider.ts index 3c70e7a40..ec7c54420 100644 --- a/packages/scenes/src/services/UrlSyncContextProvider.ts +++ b/packages/scenes/src/services/UrlSyncContextProvider.ts @@ -1,8 +1,7 @@ -import { SceneObject } from '../core/types'; -import { UrlSyncManagerOptions } from './UrlSyncManager'; +import { SceneObject, SceneUrlSyncOptions } from '../core/types'; import { useUrlSync } from './useUrlSync'; -export interface UrlSyncContextProviderProps extends UrlSyncManagerOptions { +export interface UrlSyncContextProviderProps extends SceneUrlSyncOptions { scene: SceneObject; children: React.ReactNode; } diff --git a/packages/scenes/src/services/UrlSyncManager.test.ts b/packages/scenes/src/services/UrlSyncManager.test.ts index 5318e8148..ed29e0c63 100644 --- a/packages/scenes/src/services/UrlSyncManager.test.ts +++ b/packages/scenes/src/services/UrlSyncManager.test.ts @@ -113,7 +113,7 @@ describe('UrlSyncManager', () => { obj.setState({ other: 'not synced' }); // Should not update url - expect(locationUpdates.length).toBe(2); + expect(locationUpdates.length).toBe(1); }); }); @@ -124,7 +124,7 @@ describe('UrlSyncManager', () => { children: [new SceneFlexItem({ body: obj })], }); - urlManager = new UrlSyncManager(); + urlManager = new UrlSyncManager({ updateUrlOnInit: true }); urlManager.initSync(scene); expect(locationUpdates.length).toBe(1); @@ -258,7 +258,7 @@ describe('UrlSyncManager', () => { $timeRange: outerTimeRange, }); - urlManager = new UrlSyncManager(); + urlManager = new UrlSyncManager({ updateUrlOnInit: true }); urlManager.initSync(scene); deactivate = scene.activate(); @@ -355,7 +355,7 @@ describe('UrlSyncManager', () => { obj.setState({ other: 'not synced' }); // Should not update url - expect(locationUpdates.length).toBe(2); + expect(locationUpdates.length).toBe(1); // When updating via url updateUrlStateAndSyncState({ array: ['A', 'B', 'C'] }, urlManager); @@ -470,7 +470,7 @@ describe('UrlSyncManager', () => { children: [obj1], }); - urlManager = new UrlSyncManager(); + urlManager = new UrlSyncManager({ updateUrlOnInit: true }); urlManager.initSync(scene1); deactivate = scene1.activate(); diff --git a/packages/scenes/src/services/UrlSyncManager.ts b/packages/scenes/src/services/UrlSyncManager.ts index 04b5dbbeb..de7f145b7 100644 --- a/packages/scenes/src/services/UrlSyncManager.ts +++ b/packages/scenes/src/services/UrlSyncManager.ts @@ -3,7 +3,7 @@ import { Location } from 'history'; import { locationService } from '@grafana/runtime'; import { SceneObjectStateChangedEvent } from '../core/events'; -import { SceneObject, SceneObjectUrlValues } from '../core/types'; +import { SceneObject, SceneObjectUrlValues, SceneUrlSyncOptions } from '../core/types'; import { writeSceneLog } from '../utils/writeSceneLog'; import { Subscription } from 'rxjs'; import { UniqueUrlKeyMapper } from './UniqueUrlKeyMapper'; @@ -18,20 +18,6 @@ export interface UrlSyncManagerLike { handleNewObject(sceneObj: SceneObject): void; } -export interface UrlSyncManagerOptions { - /** - * This will update the url to contain all scene url state - * when the scene is initialized. - */ - updateUrlOnInit?: boolean; - /** - * This is only supported by some objects if they implement - * shouldCreateHistoryStep where they can control what changes - * url changes should add a new browser history entry. - */ - createBrowserHistorySteps?: boolean; -} - /** * Notify the url sync manager of a new object that has been added to the scene * that needs to init state from URL. @@ -46,9 +32,9 @@ export class UrlSyncManager implements UrlSyncManagerLike { private _subs: Subscription | undefined; private _lastLocation: Location | undefined; private _paramsCache = new UrlParamsCache(); - private _options: UrlSyncManagerOptions; + private _options: SceneUrlSyncOptions; - public constructor(_options: UrlSyncManagerOptions = {}) { + public constructor(_options: SceneUrlSyncOptions = {}) { this._options = _options; } @@ -206,7 +192,7 @@ function isUrlStateDifferent(sceneUrlState: SceneObjectUrlValues, currentParams: /** * Creates a new memoized instance of the UrlSyncManager based on options */ -export function useUrlSyncManager(options: UrlSyncManagerOptions): UrlSyncManagerLike { +export function useUrlSyncManager(options: SceneUrlSyncOptions): UrlSyncManagerLike { return useMemo( () => new UrlSyncManager({ diff --git a/packages/scenes/src/services/useUrlSync.ts b/packages/scenes/src/services/useUrlSync.ts index 6a2b098e5..608cc0c0c 100644 --- a/packages/scenes/src/services/useUrlSync.ts +++ b/packages/scenes/src/services/useUrlSync.ts @@ -1,11 +1,11 @@ -import { SceneObject } from '../core/types'; +import { SceneObject, SceneUrlSyncOptions } from '../core/types'; import { useEffect, useState } from 'react'; import { useLocation } from 'react-router-dom'; import { locationService } from '@grafana/runtime'; import { writeSceneLog } from '../utils/writeSceneLog'; -import { UrlSyncManagerOptions, useUrlSyncManager } from './UrlSyncManager'; +import { useUrlSyncManager } from './UrlSyncManager'; -export function useUrlSync(sceneRoot: SceneObject, options: UrlSyncManagerOptions = {}): boolean { +export function useUrlSync(sceneRoot: SceneObject, options: SceneUrlSyncOptions = {}): boolean { const location = useLocation(); const [isInitialized, setIsInitialized] = useState(false); const urlSyncManager = useUrlSyncManager(options); From 485e9e14fb5569faf9d4e98b303eefe569970dc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 28 Aug 2024 12:32:03 +0200 Subject: [PATCH 09/11] Update --- packages/scenes-react/src/contexts/SceneContextProvider.tsx | 6 +++++- packages/scenes/src/core/SceneTimeRange.tsx | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/scenes-react/src/contexts/SceneContextProvider.tsx b/packages/scenes-react/src/contexts/SceneContextProvider.tsx index c21c9c6ee..9858bccff 100644 --- a/packages/scenes-react/src/contexts/SceneContextProvider.tsx +++ b/packages/scenes-react/src/contexts/SceneContextProvider.tsx @@ -72,5 +72,9 @@ export function SceneContextProvider({ children, timeRange, withQueryController } // For root context we wrap the provider in a UrlSyncWrapper that handles the hook that updates state on location changes - return {innerProvider}; + return ( + + {innerProvider} + + ); } diff --git a/packages/scenes/src/core/SceneTimeRange.tsx b/packages/scenes/src/core/SceneTimeRange.tsx index 64ce3bad4..e98b71a75 100644 --- a/packages/scenes/src/core/SceneTimeRange.tsx +++ b/packages/scenes/src/core/SceneTimeRange.tsx @@ -241,7 +241,7 @@ export class SceneTimeRange extends SceneObjectBase impleme this.setState(update); } - public shouldCreateHistoryEntry(values: SceneObjectUrlValues): boolean { + public shouldCreateHistoryStep(values: SceneObjectUrlValues): boolean { return true; } } From 24bf3465d19ec6f1dfca9a6f9bdea3fdd91b954b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Thu, 29 Aug 2024 09:39:48 +0200 Subject: [PATCH 10/11] UrlSync: Variable changes adds browser history steps --- .../scenes/src/services/UrlSyncManager.ts | 2 +- .../components/VariableValueSelect.tsx | 6 +-- .../src/variables/groupby/GroupByVariable.tsx | 5 ++- .../variables/variants/MultiValueVariable.ts | 38 ++++++++++++++++--- 4 files changed, 40 insertions(+), 11 deletions(-) diff --git a/packages/scenes/src/services/UrlSyncManager.ts b/packages/scenes/src/services/UrlSyncManager.ts index de7f145b7..350b4384e 100644 --- a/packages/scenes/src/services/UrlSyncManager.ts +++ b/packages/scenes/src/services/UrlSyncManager.ts @@ -148,9 +148,9 @@ export class UrlSyncManager implements UrlSyncManagerLike { const shouldCreateHistoryEntry = changedObject.urlSync.shouldCreateHistoryStep?.(newUrlState); const shouldReplace = shouldCreateHistoryEntry !== true; - writeSceneLog('UrlSyncManager', 'onStateChange updating URL'); locationService.partial(mappedUpdated, shouldReplace); + writeSceneLog('UrlSyncManager', 'onStateChange updating URL'); /// Mark the location already handled this._lastLocation = locationService.getLocation(); } diff --git a/packages/scenes/src/variables/components/VariableValueSelect.tsx b/packages/scenes/src/variables/components/VariableValueSelect.tsx index 2fbe55b64..2a172bf26 100644 --- a/packages/scenes/src/variables/components/VariableValueSelect.tsx +++ b/packages/scenes/src/variables/components/VariableValueSelect.tsx @@ -71,7 +71,7 @@ export function VariableValueSelect({ model }: SceneComponentProps { - model.changeValueTo(newValue.value!, newValue.label!); + model.changeValueTo(newValue.value!, newValue.label!, true); if (hasCustomValue !== newValue.__isNew__) { setHasCustomValue(newValue.__isNew__); @@ -135,13 +135,13 @@ export function VariableValueSelectMulti({ model }: SceneComponentProps { - model.changeValueTo(uncommittedValue); + model.changeValueTo(uncommittedValue, undefined, true); }} 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 86ec14c26..e48fcc12e 100644 --- a/packages/scenes/src/variables/groupby/GroupByVariable.tsx +++ b/packages/scenes/src/variables/groupby/GroupByVariable.tsx @@ -282,12 +282,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 df84b2383..1b607e354 100644 --- a/packages/scenes/src/variables/variants/MultiValueVariable.ts +++ b/packages/scenes/src/variables/variants/MultiValueVariable.ts @@ -47,7 +47,7 @@ export abstract class MultiValueVariable implements SceneVariable { - protected _urlSync: SceneObjectUrlSyncHandler = new MultiValueUrlSyncHandler(this); + protected _urlSync: MultiValueUrlSyncHandler = new MultiValueUrlSyncHandler(this); /** * Set to true to skip next value validation to maintain the current value even it it's not among the options (ie valid values) @@ -192,7 +192,12 @@ export abstract class MultiValueVariable $__all) then we should let that pass const isAllValueFix = stateUpdate.value === ALL_VARIABLE_VALUE && this.state.text === ALL_VARIABLE_TEXT; - if (this.skipNextValidation && stateUpdate.value !== this.state.value && stateUpdate.text !== this.state.text && !isAllValueFix) { + if ( + this.skipNextValidation && + stateUpdate.value !== this.state.value && + stateUpdate.text !== this.state.text && + !isAllValueFix + ) { stateUpdate.value = this.state.value; stateUpdate.text = this.state.text; } @@ -242,7 +247,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); } @@ -331,7 +347,7 @@ export abstract class MultiValueVariable { - this.updateValueGivenNewOptions(options); + this.updateValueGivenNewOptions(options); }); } @@ -368,6 +384,8 @@ function findOptionMatchingCurrent( export class MultiValueUrlSyncHandler implements SceneObjectUrlSyncHandler { + private _nextChangeShouldAddHistoryStep = false; + public constructor(private _sceneObject: MultiValueVariable) {} private getKey(): string { @@ -428,6 +446,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[]) { From fa16aa0246fda5b1aeeeb1d8d3db962ed635e63a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Thu, 29 Aug 2024 09:40:42 +0200 Subject: [PATCH 11/11] Test --- packages/scenes/src/services/UrlSyncManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/scenes/src/services/UrlSyncManager.ts b/packages/scenes/src/services/UrlSyncManager.ts index 350b4384e..de7f145b7 100644 --- a/packages/scenes/src/services/UrlSyncManager.ts +++ b/packages/scenes/src/services/UrlSyncManager.ts @@ -148,9 +148,9 @@ export class UrlSyncManager implements UrlSyncManagerLike { const shouldCreateHistoryEntry = changedObject.urlSync.shouldCreateHistoryStep?.(newUrlState); const shouldReplace = shouldCreateHistoryEntry !== true; + writeSceneLog('UrlSyncManager', 'onStateChange updating URL'); locationService.partial(mappedUpdated, shouldReplace); - writeSceneLog('UrlSyncManager', 'onStateChange updating URL'); /// Mark the location already handled this._lastLocation = locationService.getLocation(); }