[FEATURE] Add panel-level repeat variable model#32
Conversation
adrianSepiol
force-pushed
the
feat/panel-level-repeat
branch
from
0c5361e to
0e593be
Compare
adrianSepiol
changed the title
adrianSepiol
force-pushed
the
feat/panel-level-repeat
branch
2 times, most recently
from
5289d45 to
73a8a20
Compare
ibakshay
left a comment
ibakshay
left a comment
There was a problem hiding this comment.
Thank you for the PR! Added few comments.
There was a problem hiding this comment.
Pull request overview
Adds a panel-level “repeat variable” model to the dashboard layout spec so an individual grid item/panel can be repeated based on a variable, and exposes the needed layout fields in the panel editor model.
Changes:
- Introduces
repeatVariableas an object on grid items (TS/Go/Java/CUE) with fields likevalue,maxPer, andalignment. - Extends panel editor values/schema to include a
layoutDefinition(width/height + optional repeat variable). - Adds Go JSON unmarshal tests for grid items containing the new repeat variable object.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| ts/src/schema/panel.ts | Adds zod schemas for panel editor layoutDefinition and repeat variable. |
| ts/src/dashboard/panel.ts | Extends exported PanelEditorValues with layoutDefinition. |
| ts/src/dashboard/layout.ts | Adds RepeatVariable model and repeatVariable on GridItemDefinition. |
| java/src/main/java/dev/perses/spec/dashboard/Layout.java | Adds RepeatVariable to GridItem. |
| go/dashboard/layout.go | Adds RepeatVariable structs/types and repeatVariable on GridItem. |
| go/dashboard/layout_test.go | Adds JSON unmarshal test cases for the new repeat variable object. |
| cue/dashboard/layout_go_gen.cue | Updates generated CUE schema to include repeat variable object on grid items. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
adrianSepiol
force-pushed
the
feat/panel-level-repeat
branch
from
73a8a20 to
954d1d9
Compare
adrianSepiol
force-pushed
the
feat/panel-level-repeat
branch
2 times, most recently
from
f88e3d5 to
1da3efd
Compare
Signed-off-by: Adrian Sepiół <a.sepiol@sap.com>
adrianSepiol
force-pushed
the
feat/panel-level-repeat
branch
from
1da3efd to
ea0c4d0
Compare
ibakshay
left a comment
ibakshay
left a comment
There was a problem hiding this comment.
LGTM! @AntoineThebaud Do you also want to have a look?
|
|
||
| export const layoutDefinitionSchema = z.object({ | ||
| repeatVariable: repeatVariableSchema.optional(), | ||
| width: z.number().int().positive(), |
There was a problem hiding this comment.
Panel Editor is only for panel spec, layout is not really handled here (at least currently). Because JSON tab, will only output panel spec, if you do change on layout, nothing will change in JSON. Maybe we need to add two JSON tabs, if we add layout to this editor.
And it's supposing all layouts are using GridItem, that is the case currently. But I don't know if it will always be the case 🤔
After, I don't think it's a better to add a new editor just for layout. And put spec and layout in same editor can be fine.
Ping @Nexucis / @jgbernalp if have strong opinion on this
|
|
||
| #RepeatVariable: { | ||
| value: string @go(Value) | ||
| maxPer?: null | int @go(MaxPer,*int) |
There was a problem hiding this comment.
More link to implem than spec, but what happen if there is two panels next to each other without repeat variable. And you enable it on one with maxPer: 2. Will it do 2 rows?
- 1 panel (without repeat), empty
- panel repeat, panel repeat
There was a problem hiding this comment.
maxPer refers to rows inside grid item. By default the grid item will extend it's width to full grid width when setting repeat panel on given item (inspired by grafana) but after that you can change its width and the panels will be fit into rows based on maxPer variable.
Related to: perses/perses#2936
Model for repeat variable on panel level.