-
Notifications
You must be signed in to change notification settings - Fork 6
[FEATURE] Selection provider and Item actions #40
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
base: main
Are you sure you want to change the base?
Conversation
249c863 to
9c52f39
Compare
…mers Signed-off-by: Gabriel Bernal <[email protected]>
…erpolation Signed-off-by: Gabriel Bernal <[email protected]>
Signed-off-by: Gabriel Bernal <[email protected]>
9c52f39 to
dbf69c7
Compare
AntoineThebaud
left a comment
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.
Review of the CUE part
| #httpMethod: "GET" | "POST" | "PUT" | "PATCH" | "DELETE" | ||
|
|
||
| #batchMode: "batch" | "individual" | ||
|
|
||
| #contentType: "none" | "json" | "text" | ||
|
|
||
| #baseAction: { | ||
| name: string | ||
| icon?: string | ||
| confirmMessage?: string | ||
| enabled: bool | *true | ||
| bodyTemplate?: string | ||
| batchMode: #batchMode | *"individual" | ||
| } | ||
|
|
||
| #eventAction: { | ||
| #baseAction | ||
| type: "event" | ||
| eventName: string | ||
| } | ||
|
|
||
| #webhookAction: { | ||
| #baseAction | ||
| type: "webhook" | ||
| url: string | ||
| method: #httpMethod | *"POST" | ||
| contentType: #contentType | *"none" | ||
| headers?: [string]: string | ||
| } |
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.
Some defs can be merged up imho:
| #httpMethod: "GET" | "POST" | "PUT" | "PATCH" | "DELETE" | |
| #batchMode: "batch" | "individual" | |
| #contentType: "none" | "json" | "text" | |
| #baseAction: { | |
| name: string | |
| icon?: string | |
| confirmMessage?: string | |
| enabled: bool | *true | |
| bodyTemplate?: string | |
| batchMode: #batchMode | *"individual" | |
| } | |
| #eventAction: { | |
| #baseAction | |
| type: "event" | |
| eventName: string | |
| } | |
| #webhookAction: { | |
| #baseAction | |
| type: "webhook" | |
| url: string | |
| method: #httpMethod | *"POST" | |
| contentType: #contentType | *"none" | |
| headers?: [string]: string | |
| } | |
| #baseAction: { | |
| name: string | |
| icon?: string | |
| confirmMessage?: string | |
| enabled: bool | *true | |
| bodyTemplate?: string | |
| batchMode: "batch" | *"individual" | |
| } | |
| #eventAction: { | |
| #baseAction | |
| type: "event" | |
| eventName: string | |
| } | |
| #webhookAction: { | |
| #baseAction | |
| type: "webhook" | |
| url: string | |
| method: "GET" | *"POST" | "PUT" | "PATCH" | "DELETE" | |
| contentType: *"none" | "json" | "text" | |
| headers?: [string]: string | |
| } |
| #itemAction: | ||
| (#eventAction & {type: "event"}) | | ||
| (#webhookAction & {type: "webhook"}) |
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.
Can be simplified to
| #itemAction: | |
| (#eventAction & {type: "event"}) | | |
| (#webhookAction & {type: "webhook"}) | |
| #itemAction: #eventAction | #webhookAction |
|
|
||
| #actions: { | ||
| enabled: bool | *true | ||
| actionsList: [#itemAction, ...] |
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.
| actionsList: [#itemAction, ...] | |
| actionsList: [...#itemAction] |
[#itemAction, ...] actually means "a list starting by an itemAction followed by any structs"
| displayInHeader?: bool | *true | ||
| displayWithItem?: bool | *true |
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.
Do we really want/need to define default values for these optional attributes?
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.
I think item actions are super interesting for embedded usage, maybe a little less for Perses App.
Idea for the future, add a provider for adding supported custom events, it will be useful for autocomplete (who can know all custom events :p) and remove the "custom event" option if there is no custom event supported (like currently in Perses App).
About the webhook, I did not test it, but can we have some CORS issues due to this? Maybe we want to allow to pass by the proxy too (like datasources) at some point?
| } | ||
| return matches; | ||
| } | ||
| // Re-export all variable interpolation utilities from @perses-dev/components for backwards compatibility |
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.
Any reason(s) to move everything to @perses-dev/components? Circular deps?
Look like more runtime stuff than components to me 👀
And ItemActionsOptionsEditor + SelectionOptionsEditor, I would see them in @perses-dev/components instead.
| /> | ||
|
|
||
| <Stack direction="row" spacing={2}> | ||
| <TextField |
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.
When I tested manually, when I type something, I lose focus after 1 char. Same issue on all fields of this components :/
| ): SelectionState<T, Id> & { hasContext: boolean } { | ||
| const ctx = useContext(SelectionContext); | ||
|
|
||
| useEffect(() => { |
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.
Not fan of this useEffect, but I guess no others choices. This provider is complex to understand, some comments would be welcomed 😅
| {isCollapsed ? <ChevronRight /> : <ChevronDown />} | ||
| </IconButton> | ||
|
|
||
| <Typography variant="subtitle2">{eventAction.name ?? 'Event Action'}</Typography> |
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.


Description
This PR adds a new Actions and selection features. perses/perses#3788. This allows to define actions based on selected data, for example calling an HTTP request with the interpolated data from multiple rows selection.
Two new Providers were added to the main Panel component, yes we have more providers but users don't have to configure these two, plugin developers can just use the hooks without any issue as they are resilient to undefined context:
I split into 3 commits so is easier to review:
Screenshots
Screen.Recording.2026-01-23.at.20.41.34.mov
Checklist
[<catalog_entry>] <commit message>naming convention using one of thefollowing
catalog_entryvalues:FEATURE,ENHANCEMENT,BUGFIX,BREAKINGCHANGE,DOC,IGNORE.UI Changes
See e2e docs for more details. Common issues include: