From 62650090a62e20e5a5a6467e64e4de489650e795 Mon Sep 17 00:00:00 2001 From: Bogdan Matei Date: Fri, 6 Dec 2024 11:37:27 +0200 Subject: [PATCH] Remove complexity --- packages/scenes/src/core/SceneScopesBridge.ts | 17 +++++++---------- .../variables/adhoc/AdHocFilterRenderer.tsx | 10 ++++------ .../variables/adhoc/AdHocFiltersVariable.tsx | 19 ------------------- .../src/variables/groupby/GroupByVariable.tsx | 18 +----------------- 4 files changed, 12 insertions(+), 52 deletions(-) diff --git a/packages/scenes/src/core/SceneScopesBridge.ts b/packages/scenes/src/core/SceneScopesBridge.ts index d727a262a..122fa5042 100644 --- a/packages/scenes/src/core/SceneScopesBridge.ts +++ b/packages/scenes/src/core/SceneScopesBridge.ts @@ -1,16 +1,15 @@ import { isEqual } from 'lodash'; +import { useEffect } from 'react'; import { BehaviorSubject, filter, map, Observable, pairwise, Unsubscribable } from 'rxjs'; import { Scope } from '@grafana/data'; import { SceneObjectBase } from './SceneObjectBase'; -import { SceneComponentProps, SceneObjectState, SceneObjectUrlValues, SceneObjectWithUrlSync } from './types'; +import { SceneComponentProps, SceneObjectUrlValues, SceneObjectWithUrlSync } from './types'; import { ScopesContextValue, useScopes } from './ScopesContext'; import { SceneObjectUrlSyncConfig } from '../services/SceneObjectUrlSyncConfig'; -export interface SceneScopesBridgeState extends SceneObjectState {} - -export class SceneScopesBridge extends SceneObjectBase implements SceneObjectWithUrlSync { +export class SceneScopesBridge extends SceneObjectBase implements SceneObjectWithUrlSync { static Component = SceneScopesBridgeRenderer; protected _urlSync = new SceneObjectUrlSyncConfig(this, { keys: ['scopes'] }); @@ -21,10 +20,6 @@ export class SceneScopesBridge extends SceneObjectBase i private _pendingScopes: string[] | null = null; - public constructor(state: SceneScopesBridgeState) { - super(state); - } - public getUrlState(): SceneObjectUrlValues { return { scopes: this._pendingScopes ?? (this.context?.state.value ?? []).map((scope) => scope.metadata.name), @@ -113,7 +108,7 @@ export class SceneScopesBridge extends SceneObjectBase i this._contextSubject.next(newContext); if (shouldUpdate) { - setTimeout(() => this.forceRender()); + this.forceRender(); } } } @@ -130,7 +125,9 @@ export class SceneScopesBridge extends SceneObjectBase i function SceneScopesBridgeRenderer({ model }: SceneComponentProps) { const context = useScopes(); - model.updateContext(context); + useEffect(() => { + model.updateContext(context); + }, [context, model]); return null; } diff --git a/packages/scenes/src/variables/adhoc/AdHocFilterRenderer.tsx b/packages/scenes/src/variables/adhoc/AdHocFilterRenderer.tsx index 6832a44b3..be94f3daf 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFilterRenderer.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFilterRenderer.tsx @@ -28,8 +28,6 @@ const filterNoOp = () => true; export function AdHocFilterRenderer({ filter, model }: Props) { const styles = useStyles2(getStyles); - const { _isScopesLoading } = model.useState(); - const [keys, setKeys] = useState([]); const [values, setValues] = useState([]); const [isKeysLoading, setIsKeysLoading] = useState(false); @@ -141,8 +139,8 @@ export function AdHocFilterRenderer({ filter, model }: Props) { // there's a bug in react-select where the menu doesn't recalculate its position when the options are loaded asynchronously // see https://github.com/grafana/grafana/issues/63558 // instead, we explicitly control the menu visibility and prevent showing it until the options have fully loaded - isOpen={isValuesOpen && !isValuesLoading && !_isScopesLoading} - isLoading={isValuesLoading || _isScopesLoading} + isOpen={isValuesOpen && !isValuesLoading} + isLoading={isValuesLoading} openMenuOnFocus={true} onOpenMenu={async () => { setIsValuesLoading(true); @@ -189,8 +187,8 @@ export function AdHocFilterRenderer({ filter, model }: Props) { // there's a bug in react-select where the menu doesn't recalculate its position when the options are loaded asynchronously // see https://github.com/grafana/grafana/issues/63558 // instead, we explicitly control the menu visibility and prevent showing it until the options have fully loaded - isOpen={isKeysOpen && !isKeysLoading && !_isScopesLoading} - isLoading={isKeysLoading || _isScopesLoading} + isOpen={isKeysOpen && !isKeysLoading} + isLoading={isKeysLoading} onOpenMenu={async () => { setIsKeysOpen(true); setIsKeysLoading(true); diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx index b3684d802..00da5991e 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx @@ -104,11 +104,6 @@ export interface AdHocFiltersVariableState extends SceneVariableState { * @internal state of the new filter being added */ _wip?: AdHocFilterWithLabels; - - /** - * @internal flag for keeping track of scopes loading state - */ - _isScopesLoading?: boolean; } export type AdHocVariableExpressionBuilderFn = (filters: AdHocFilterWithLabels[]) => string; @@ -200,12 +195,6 @@ export class AdHocFiltersVariable private _activationHandler = () => { this._scopesBridge = sceneGraph.getScopesBridge(this); - - if (this._scopesBridge) { - this._subs.add( - this._scopesBridge.subscribeToIsLoading((isLoading) => this.setState({ _isScopesLoading: isLoading })) - ); - } }; public setState(update: Partial): void { @@ -302,10 +291,6 @@ export class AdHocFiltersVariable * Get possible keys given current filters. Do not call from plugins directly */ public async _getKeys(currentKey: string | null): Promise>> { - if (this._scopesBridge?.getIsLoading()) { - return []; - } - const override = await this.state.getTagKeysProvider?.(this, currentKey); if (override && override.replace) { @@ -353,10 +338,6 @@ export class AdHocFiltersVariable * Get possible key values for a specific key given current filters. Do not call from plugins directly */ public async _getValuesFor(filter: AdHocFilterWithLabels): Promise>> { - if (this._scopesBridge?.getIsLoading()) { - return []; - } - const override = await this.state.getTagValuesProvider?.(this, filter); if (override && override.replace) { diff --git a/packages/scenes/src/variables/groupby/GroupByVariable.tsx b/packages/scenes/src/variables/groupby/GroupByVariable.tsx index 76e93afed..1a9be8e19 100644 --- a/packages/scenes/src/variables/groupby/GroupByVariable.tsx +++ b/packages/scenes/src/variables/groupby/GroupByVariable.tsx @@ -51,11 +51,6 @@ export interface GroupByVariableState extends MultiValueVariableState { * Return replace: false if you want to combine the results with the default lookup */ getTagKeysProvider?: getTagKeysProvider; - - /** - * @internal flag for keeping track of scopes loading state - */ - _isScopesLoading?: boolean; } export type getTagKeysProvider = ( @@ -157,12 +152,6 @@ export class GroupByVariable extends MultiValueVariable { allActiveGroupByVariables.add(this); - if (this._scopesBridge) { - this._subs.add( - this._scopesBridge.subscribeToIsLoading((isLoading) => this.setState({ _isScopesLoading: isLoading })) - ); - } - return () => allActiveGroupByVariables.delete(this); }); } @@ -171,10 +160,6 @@ export class GroupByVariable extends MultiValueVariable { * Get possible keys given current filters. Do not call from plugins directly */ public _getKeys = async (ds: DataSourceApi) => { - if (this._scopesBridge?.getIsLoading()) { - return []; - } - // TODO: provide current dimensions? const override = await this.state.getTagKeysProvider?.(this, null); @@ -235,7 +220,6 @@ export function GroupByVariableRenderer({ model }: SceneComponentProps>>(() => { @@ -303,7 +287,7 @@ export function GroupByVariableRenderer({ model }: SceneComponentProps {