-
Notifications
You must be signed in to change notification settings - Fork 6
feat: replace recharts with Chart component in deployment metrics #896
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
Conversation
… github.com:devtron-labs/devtron-fe-common-lib into feat/deployment-metrics-charts
1. allow only one dataset for area chart 2. add support for referenceLines 3. add time scale support 4. change grid lines and axis labels color
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.
Pull Request Overview
This PR enhances the Charts component by replacing recharts with Chart.js and improves the Table component by migrating from useAsync to useQuery for better data fetching. The changes modernize the chart capabilities with support for time-based axes, reference lines, and improved performance configurations.
- Replace recharts with Chart.js implementation for better chart rendering
- Migrate Table component from useAsync to useQuery for improved data fetching
- Add time-based axis support and reference lines to charts
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
src/Shared/Components/Table/types.ts | Updates type definitions for Table component, removing APIOptions import and adding RowsResultType |
src/Shared/Components/Table/InternalTable.tsx | Migrates from useAsync to useQuery for data fetching |
src/Shared/Components/Charts/utils.tsx | Adds time axis support, reference lines, and performance optimizations |
src/Shared/Components/Charts/types.ts | Extends chart types with time axis and reference line configurations |
src/Shared/Components/Charts/plugins.ts | Refactors plugins to support reference lines instead of average/separator lines |
src/Shared/Components/Charts/index.ts | Exports new ReferenceLineConfigType |
src/Shared/Components/Charts/constants.ts | Removes hardcoded legend config and adds axis label colors |
src/Shared/Components/Charts/Chart.component.tsx | Major refactor to support time scales and reference lines |
package.json | Adds chartjs-adapter-moment dependency and version bump |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Pls look into copilot's review as well
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.
Pull Request Overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/Shared/Components/Charts/utils.tsx:297
- The
beginAtZero: true
property was removed from the line/area chart configuration but retained for stackedBar. This inconsistency could lead to unexpected chart behavior. Consider documenting why this property is needed for stacked bars but not for line/area charts.
beginAtZero: true,
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist