Skip to content

Conversation

@shahrokni
Copy link
Contributor

@shahrokni shahrokni commented Jan 8, 2026

Relates to perses/perses#3483

Description 🖊️

This change renders the Table Cell Content accordingly. If data link exists, the cell content will be rendered as a link.

Embedded Variables ⚠️

Link embedded variables should be translated by the plugin, because Shared/Components have no access to the variable context.

Embedded values from neighboring cells ⚠️

There is an ongoing discussion about emending values from neighboring cells.
As it was discussed with @AntoineThebaud a link may have a reference to a neighboring cell as a parameter. This is completely feasible. However, I decided to discuss some details with @AntoineThebaud before the actual implementation.

image

Checklist

  • Pull request has a descriptive title and context useful to a reviewer.
  • Pull request title follows the [<catalog_entry>] <commit message> naming convention using one of the
    following catalog_entry values: FEATURE, ENHANCEMENT, BUGFIX, BREAKINGCHANGE, DOC,IGNORE.
  • All commits have DCO signoffs.

UI Changes

  • Changes that impact the UI include screenshots and/or screencasts of the relevant changes.
  • Code follows the UI guidelines.
  • E2E tests are stable and unlikely to be flaky.
    See e2e docs for more details. Common issues include:
    • Is the data inconsistent? You need to mock API requests.
    • Does the time change? You need to use consistent time values or mock time utilities.
    • Does it have loading states? You need to wait for loading to complete.

@shahrokni shahrokni requested a review from a team as a code owner January 8, 2026 13:41
@shahrokni shahrokni requested review from AntoineThebaud and jgbernalp and removed request for a team January 8, 2026 13:41
@shahrokni shahrokni marked this pull request as draft January 8, 2026 13:59
Signed-off-by: Seyed Mahmoud SHAHROKNI <[email protected]>

Signed-off-by: Mahmoud Shahrokni <[email protected]>

Signed-off-by: Mahmoud Shahrokni <[email protected]>
@shahrokni shahrokni marked this pull request as ready for review January 8, 2026 14:11
// Column link settings
// The URL could be set to a static link or could be constructed dynamically
// The URL may include reference to the variables or neighboring cells in the row
export interface DataLink {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall, but we should adjust this interface with the comments from perses/plugins#488 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I answered the comment.
I understand the concern there. I am just wondering, if we should mix this kind of refactor with this feature.
Could you check and see if my answer makes sense? (I know that at some points we need to refactor the interface)

If a refactor is a must, should we move the new suggested DataLink interface under perses/ui/core?
DataLink is not bound to any component and is something general. I see no directory which could fit in Shared repo.

Copy link
Contributor

@AntoineThebaud AntoineThebaud Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FE counterpart of the BE interface shared in above comment is already in the core https://github.com/perses/perses/blob/main/ui/core/src/model/panels.ts#L17-L23

For now located in "panels" because added with the Panel links feature but we should soon have the Dashboard links that will also need the very same interface.

Still, e.g in the case of the Table panel links we shouldn't have the name attribute - as there's a single link to configure and not an array of them - so we should still have sth like this on Table plugin or shared side I guess:

type DataLink = Omit<Link, 'name'>;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants