-
Notifications
You must be signed in to change notification settings - Fork 11.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DashboardScene: Fixing major row repeat issues #87539
Changes from 1 commit
daa9d05
7c67900
4b258a0
b5e7364
10bd609
a2fac61
6e5a743
2e06d79
9fa50e7
5564da3
341df56
bdd13c2
8b0b65a
ba76261
b252f02
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,14 +2,7 @@ import { css } from '@emotion/css'; | |
import React from 'react'; | ||
|
||
import { GrafanaTheme2 } from '@grafana/data'; | ||
import { | ||
SceneComponentProps, | ||
SceneGridLayout, | ||
SceneGridRow, | ||
SceneObjectBase, | ||
SceneObjectState, | ||
VizPanel, | ||
} from '@grafana/scenes'; | ||
import { SceneComponentProps, SceneGridRow, SceneObjectBase, SceneObjectState, VizPanel } from '@grafana/scenes'; | ||
import { Icon, TextLink, useStyles2 } from '@grafana/ui'; | ||
import appEvents from 'app/core/app_events'; | ||
import { SHARED_DASHBOARD_QUERY } from 'app/plugins/datasource/dashboard'; | ||
|
@@ -25,31 +18,6 @@ import { RowOptionsButton } from './RowOptionsButton'; | |
export interface RowActionsState extends SceneObjectState {} | ||
|
||
export class RowActions extends SceneObjectBase<RowActionsState> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An interesting thing i found when testing is that repeated row has row actions available. This means that the repeated row can be repeated further down. It was possible in the previous arch as well, tho it resulted in no effect. I think we should just disable the repeated row's RowActions, otherwise it's confusing and the results are confusing. WDYT? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, your rigtht, it should not have any options action even There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @torkelo - i've just pushed a commit that handles that |
||
private updateLayout(rowClone: SceneGridRow): void { | ||
const row = this.getParent(); | ||
|
||
const layout = this.getDashboard().state.body; | ||
|
||
if (!(layout instanceof SceneGridLayout)) { | ||
throw new Error('Layout is not a SceneGridLayout'); | ||
} | ||
|
||
// remove the repeated rows | ||
const children = layout.state.children.filter((child) => !child.state.key?.startsWith(`${row.state.key}-clone-`)); | ||
|
||
// get the index to replace later | ||
const index = children.indexOf(row); | ||
|
||
if (index === -1) { | ||
throw new Error('Parent row not found in layout children'); | ||
} | ||
|
||
// replace the row with the clone | ||
layout.setState({ | ||
children: [...children.slice(0, index), rowClone, ...children.slice(index + 1)], | ||
}); | ||
} | ||
|
||
public getParent(): SceneGridRow { | ||
if (!(this.parent instanceof SceneGridRow)) { | ||
throw new Error('RowActions must have a SceneGridRow parent'); | ||
|
@@ -64,39 +32,29 @@ export class RowActions extends SceneObjectBase<RowActionsState> { | |
|
||
public onUpdate = (title: string, repeat?: string | null): void => { | ||
const row = this.getParent(); | ||
let repeatBehavior: RowRepeaterBehavior | undefined; | ||
|
||
// return early if there is no repeat | ||
if (!repeat) { | ||
const clone = row.clone(); | ||
|
||
// remove the row repeater behaviour, leave the rest | ||
clone.setState({ | ||
title, | ||
$behaviors: row.state.$behaviors?.filter((b) => !(b instanceof RowRepeaterBehavior)) ?? [], | ||
}); | ||
|
||
this.updateLayout(clone); | ||
|
||
return; | ||
if (row.state.$behaviors) { | ||
for (let b of row.state.$behaviors) { | ||
if (b instanceof RowRepeaterBehavior) { | ||
repeatBehavior = b; | ||
} | ||
} | ||
} | ||
|
||
const children = row.state.children.map((child) => child.clone()); | ||
|
||
const newBehaviour = new RowRepeaterBehavior({ | ||
variableName: repeat, | ||
sources: children, | ||
}); | ||
|
||
// get rest of behaviors except the old row repeater, if any, and push new one | ||
const behaviors = row.state.$behaviors?.filter((b) => !(b instanceof RowRepeaterBehavior)) ?? []; | ||
behaviors.push(newBehaviour); | ||
|
||
row.setState({ | ||
title, | ||
$behaviors: behaviors, | ||
}); | ||
if (repeat && !repeatBehavior) { | ||
const repeatBehavior = new RowRepeaterBehavior({ variableName: repeat }); | ||
row.setState({ $behaviors: [...(row.state.$behaviors ?? []), repeatBehavior] }); | ||
// Temp, needs fix in scenes lib | ||
repeatBehavior.activate(); | ||
} else if (repeatBehavior) { | ||
repeatBehavior.removeRepeatClonesFromLayout(); | ||
row.setState({ $behaviors: row.state.$behaviors!.filter((b) => b !== repeatBehavior) }); | ||
} | ||
|
||
newBehaviour.activate(); | ||
if (title !== row.state.title) { | ||
row.setState({ title }); | ||
} | ||
}; | ||
|
||
public onDelete = () => { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is unrelated change, and is only here for readability. I have numerous times been very confused trying to find where panel changes are committed (thinking this function only handled repeat options)