-
Notifications
You must be signed in to change notification settings - Fork 14
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
Charts Without Grafana #2049
Charts Without Grafana #2049
Conversation
532f165
to
73f0065
Compare
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.
LGTM
I saw a couple improvements but we can implement those in subsequent PRs
], | ||
})); | ||
|
||
console.log('zoom level', chartJsInstance.getZoomLevel()); |
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 we can use actions for these instead of console logging
66ea310
to
24dfda5
Compare
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.
LGTM, I basically looked at at the structure and it looks good, I will take some time the following weeks to play with it locally and review it in deeper detail 👍
Left some very pedantic nitpicks if you want to address them 😅
assets/js/common/TimeSeriesLineChart/TimeSeriesLineChart.test.jsx
Outdated
Show resolved
Hide resolved
assets/js/common/TimeSeriesLineChart/TimeSeriesLineChart.test.jsx
Outdated
Show resolved
Hide resolved
d630d0b
to
d7cb229
Compare
Description
Replace Grafana charts using a bespoke implementation with Chart.js.
Grafana is completely removed, the backend will expose chart data through a dedicated endpoint.
The chart data is retrieved through prometheus.
How was this tested?
Automated tests