Conversation
Doc updates for architecture overview references Co-authored-by: rajsite <1588923+rajsite@users.noreply.github.com> Signed-off-by: Ahalya Radhakrishnan <ahalya.radhakrishnan@ni.com>
Signed-off-by: Ahalya Radhakrishnan <ahalya.radhakrishnan@ni.com>
Signed-off-by: Ahalya Radhakrishnan <ahalya.radhakrishnan@ni.com>
…a/feat/grafana/add-custom-refresh-event
Signed-off-by: Ahalya Radhakrishnan <ahalya.radhakrishnan@ni.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a custom event NIRefreshDashboardEvent that enables panels to programmatically trigger dashboard refreshes. The event is subscribed to in DashboardPageProxy to ensure consistent behavior across both old and new dashboard implementations.
Changes:
- Added
NIRefreshDashboardEventclass definition in the events type file - Implemented event subscription in
DashboardPageProxythat callsrefreshTimeModel()when triggered - Added comprehensive test coverage for the event subscription lifecycle
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| public/app/types/events.ts | Defines the new NIRefreshDashboardEvent class extending BusEventBase |
| public/app/features/dashboard/containers/DashboardPageProxy.tsx | Subscribes to the refresh event in a useEffect hook with proper cleanup |
| public/app/features/dashboard/containers/DashboardPageProxy.test.tsx | Adds four comprehensive test cases covering subscription, event handling, cleanup, and feature toggle independence |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
public/app/features/dashboard/containers/DashboardPageProxy.tsx
Outdated
Show resolved
Hide resolved
public/app/features/dashboard/containers/DashboardPageProxy.test.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Ahalya Radhakrishnan <ahalya.radhakrishnan@ni.com>
Signed-off-by: Ahalya Radhakrishnan <ahalya.radhakrishnan@ni.com>
public/app/features/dashboard/containers/DashboardPageProxy.test.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Ahalya Radhakrishnan <ahalya.radhakrishnan@ni.com>
|
Note to reviewers: The pipeline failures/queued states are not specific to this PR. I've verified that almost all pipeline builds in this repository are currently experiencing similar issues. |
m-akinc
left a comment
There was a problem hiding this comment.
Rubber-stamping. I would raise concerns about adding more custom code to Grafana, but it seems like this has already been considered.
* Doc updates for architecture overview (#66) Doc updates for architecture overview references Co-authored-by: rajsite <1588923+rajsite@users.noreply.github.com> Signed-off-by: Ahalya Radhakrishnan <ahalya.radhakrishnan@ni.com> * moved logic to proxy page Signed-off-by: Ahalya Radhakrishnan <ahalya.radhakrishnan@ni.com> * add test for dashboard page proxy component Signed-off-by: Ahalya Radhakrishnan <ahalya.radhakrishnan@ni.com> * remove changes made in time series panel Signed-off-by: Ahalya Radhakrishnan <ahalya.radhakrishnan@ni.com> * updated test Signed-off-by: Ahalya Radhakrishnan <ahalya.radhakrishnan@ni.com> * resolve nit comments Signed-off-by: Ahalya Radhakrishnan <ahalya.radhakrishnan@ni.com> * add code comment and organize tests Signed-off-by: Ahalya Radhakrishnan <ahalya.radhakrishnan@ni.com> --------- Signed-off-by: Ahalya Radhakrishnan <ahalya.radhakrishnan@ni.com> Co-authored-by: Milan Raj <rajsite@users.noreply.github.com> Co-authored-by: rajsite <1588923+rajsite@users.noreply.github.com>
What is this feature?
User Story 3467495: NI Grafana Fork | Add ability to trigger a dashboard refresh using appEvents
This PR adds a new
NIRefreshDashboardEventthat allows panels to trigger a dashboard refresh.Workaround for grafana#109587
HLD and discussion for change.
Why do we need this feature?
When panels need to refresh the dashboard, they need a way to communicate this action to the dashboard container.
The original HLD proposed subscribing to the custom event in DashboardPage. However, this implementation subscribes to the event in DashboardPageProxy instead, ensuring the event handler works consistently regardless of which dashboard implementation is rendered.
This change is required for the feature Feature 3189931: Support for high resolution zoom. For more technical details, refer this document.
Who is this feature for?
Dashboard panel users who need to trigger a refresh within their panels.
Which issue(s) does this PR fix?:
NA
Fixes
NA
Special notes for your reviewer:
The pipeline failures/queued states are not specific to this PR. I've verified that almost all pipeline builds in this repository are currently experiencing similar issues.
Please check that: