-
Notifications
You must be signed in to change notification settings - Fork 9
Components For visualisations and Sidebar #53
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: monitoring-dashboard
Are you sure you want to change the base?
Components For visualisations and Sidebar #53
Conversation
|
@psankhe28 is attempting to deploy a commit to the UCI Team on Vercel. A member of the Team first needs to authorize it. |
| import React, { useEffect, useRef } from "react"; | ||
| import Chart from "chart.js/auto"; | ||
| import useWindowSize from "../../hooks/useWindow"; | ||
|
|
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 you move this function to some util function
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.
It's a hook so I have kept it in the hooks folder. Should I change?
src/components/visualisation/pie.tsx
Outdated
| const { height } = useWindowSize(); | ||
|
|
||
| useEffect(() => { | ||
| // Generate dynamic background colors for each segment based on labels |
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.
since it's reading data from props,so there are chances that when you reload or during initiation time data may be undefined which will break the condition data.labels.map and return error.So I suggest you to use optional chaining like data?.labels?.map
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 have updated it in the recent push.
| data: { | ||
| labels: data.labels, | ||
| datasets: [ | ||
| { |
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.
here also add optional chaining
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 have updated it in the recent push.
src/components/visualisation/pie.tsx
Outdated
|
|
||
| return ( | ||
| <div | ||
| style={{ |
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.
move inline styles to a style class
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 have updated it in the recent push.
In visualization: Bar, Line and Pie Chart.
For sidebar, added submenus.