search

grafana / scenes

Build Grafana dashboards directly in your Grafana app plugins.
https://grafana.com/developers/scenes
Apache License 2.0
133 stars 20 forks source link

Add method to get ancestor for localValueVariable #699

Closed mdvictor closed 5 months ago

mdvictor commented 5 months ago

In certain scenarios (e.g.: https://github.com/grafana/grafana/issues/84915) where we repeat a panel under a repeated row using the same variable, the DashboardGridItem._performRepeat looks up and finds the variable in the grid row, since they have the same name, and since that is a LocalValueVariable it cannot perform the repeat with it.

With this method we can get the parent variable of the LocalValueVariable and use that to repeat.

Needed by https://github.com/grafana/grafana/pull/86425

📦 Published PR as canary version: 4.11.1--canary.699.8720696583.0
:sparkles: Test out this PR locally via: ```bash npm install @grafana/scenes@4.11.1--canary.699.8720696583.0 # or yarn add @grafana/scenes@4.11.1--canary.699.8720696583.0 ```
torkelo commented 5 months ago

@mdvictor what would be the point of this scenario?

I do not think we should support it.

If the row is repeated by variable A, and you repeat a panel inside that row by the same variable it should only see the local scoped value.

That is how repeating works.

mdvictor commented 5 months ago

@torkelo I'm also wondering if this should be supported. It might be an edge case to use the same var for both panels and rows.

My only argument for supporting this would be parity with the old arch, where you can have a dashboard with repeated panels inside repeated rows under the same variable and they will both update when that variable changes.

In the new arch the repeated panels will not update because the type is LocalValueVariable and it only accepts MultiValueVariable.

Do we not want to support this specific implementation or the scenario at all?

torkelo commented 5 months ago

@mdvictor i would say that is a bug in the old architecture,

The query in a panel would get the local value, so the panel repeat should also

mdvictor commented 5 months ago

@torkelo Just making sure I understand right: We do not want to support the scenario of having a repeated row and a repeated panel under it using the same variable? Or we do but using the local value instead of searching up the three?

torkelo commented 5 months ago

Just making sure I understand right: We do not want to support the scenario of having a repeated row and a repeated panel under it using the same variable?

@mdvictor , yea this scenarios does not make sense as the variable will only have single value under the row