upcoming: [DI-19818] - Added line graph to CloudPulse widgets for different metrics#10710
Conversation
|
Coverage Report: ✅ |
packages/manager/src/features/CloudPulse/Utils/CloudPulseWidgetColorPalette.ts
Outdated
Show resolved
Hide resolved
packages/manager/src/features/CloudPulse/Widget/CloudPulseWidget.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/features/CloudPulse/Widget/CloudPulseWidget.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/features/CloudPulse/Widget/CloudPulseWidget.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/features/CloudPulse/Widget/CloudPulseWidget.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/features/CloudPulse/Widget/CloudPulseWidget.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/features/CloudPulse/Widget/CloudPulseWidget.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/features/CloudPulse/Widget/CloudPulseWidget.tsx
Outdated
Show resolved
Hide resolved
carrillo-erik
left a comment
There was a problem hiding this comment.
Left some initial feedback to be addressed. I'll continue to review but this should give you a starting point. Reach out if you have questions.
Sure thanks for comments, I'll addressed them and also put bigger logic into a util file and reduce the component size |
packages/manager/src/features/CloudPulse/Utils/CloudPulseWidgetUtils.ts
Outdated
Show resolved
Hide resolved
packages/manager/src/features/CloudPulse/Widget/CloudPulseWidget.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/features/CloudPulse/Widget/CloudPulseWidget.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/features/CloudPulse/Widget/components/CloudPulseLineGraph.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/features/CloudPulse/shared/CloudPulseTimeRangeSelect.tsx
Outdated
Show resolved
Hide resolved
|
There seems to be a lot of jankiness when you change values for the charts, I'm not sure if that's because we're dealing with mocks or if it's due to multiple re-renderings. It's just something to keep an eye on as development progresses 👍 |
|
Approving pending changes request from Banks |
ddd9ce1 to
ab0a446
Compare
|
Looks like you have a test failure for |
|
@carrillo-erik this should be ready for another pass 👍 |
@jaalah-akamai it seems some other test is failing as it doesn't belong to ACLP |
packages/manager/src/features/CloudPulse/Widget/CloudPulseWidget.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/features/CloudPulse/Widget/components/CloudPulseLineGraph.tsx
Outdated
Show resolved
Hide resolved
|
I highly recommend that the CloudPulse use |
We have already discussed regarding this & for now we can move with chart.js for our beta release. |
|
@nikhagra Could you merge in the latest changes from |
40821c1 to
acd515f
Compare
@jdamore-linode updated from latest dev branch |
|
I'm aware of that Slack discussion, but I don't agree with the outcome. It's just creating more work for us later. @nikhagra |
Its in the radar already post discussion on the slack , we will be migrating it post beta asap. Hope this shouldn't be a showstopper for this PR and beta which we have a very tight-timeline of 2 more weeks only for dev integrations with service owners. Thanks!! |
|
Thanks!! @bnussman-akamai for the approvals. Can this request be merged now please? |
|
@jaalah-akamai @jdamore-linode @bnussman-akamai I've marked marked all the conversations as resolved but still it says merging blocked because of requested changes. Could you please check and merge |
Description 📝
Line graph component is added in cloud pulse widget to showcase data for different metrics.
Changes 🔄
List any change relevant to the reviewer.
Target release date 🗓️
2nd August 2024
Preview 📷
Include a screenshot or screen recording of the change
💡 Use
<video src="" />tag when including recordings in table.How to test 🧪
As an Author I have considered 🤔
Check all that apply