upcoming: [DI-20501] - Fix Demo feedbacks and missed changes across ACLP by venkymano-akamai · Pull Request #10851 · linode/manager

venkymano-akamai · 2024-08-29T07:56:52Z

Description 📝

Fixed demo feedbacks and missed changes across ACLP

Changes 🔄

Fixed Dashboard Select placeholder from Select a Dashboard to Select Dashboard
Fixed striking functionality interchangeably while clicking native legend as well as legend table row.
Enhanced Region Selection component to honor the placeholder prop being passed.
Pass fill option to line graph only if the chart type is area.
Pass queried resource option for choosing default selections as well on the CloudPulseCustomSelect component
Integrate manual refresh functionality
Fix color for close icon in the CloudPulseResourcesSelection component
Update labels in time duration from Past to Last

Target release date 🗓️

02-09-2024

Preview 📷

Include a screenshot or screen recording of the change

💡 Use <video src="" /> tag when including recordings in table.

Before	After

How to test 🧪

Enable mock service as some API endpoints are not available.
Click on the Monitor tab
You will be able to see the options and fixes
For global manual refresh, once you see the widgets rendered, click the refresh icon, you can see again the widgets are loaded.

As an Author I have considered 🤔

Check all that apply

venkymano-akamai · 2024-08-29T08:47:35Z

packages/manager/src/components/LineGraph/LineGraph.tsx

        display: _nativeLegend,
+        onClick: (e, legendItem) => {
+          if (legendItem) {
+            handleLegendClick(legendItem.datasetIndex!);


FYI, this will strikeout the legend as well on nativelegend click

venkymano-akamai · 2024-08-29T08:48:27Z

packages/manager/src/features/CloudPulse/Utils/CloudPulseWidgetUtils.ts

        const dimension = {
          backgroundColor: color,
-          borderColor: '',
+          borderColor: color,


FYI - pass border color for graphs

venkymano-akamai · 2024-08-29T08:48:42Z

packages/manager/src/features/CloudPulse/Utils/CloudPulseWidgetUtils.ts

-          borderColor: '',
+          borderColor: color,
          data: seriesDataFormatter(transformedData.values, start, end),
+          fill: widgetChartType === 'area' ? true : false,


FYI - pass fill based on chart type

venkymano-akamai · 2024-08-29T08:53:13Z

packages/manager/src/components/LineGraph/LineGraph.tsx

        display: _nativeLegend,
+        onClick: (_e, legendItem) => {
+          if (legendItem && legendItem.datasetIndex) {
+            handleLegendClick(legendItem.datasetIndex); // when we click on native legend, also call the handle legend click function


FYI - Even on clicking the native legend, make use of handle legend click functionality

venkymano-akamai · 2024-08-29T10:20:01Z

packages/manager/src/features/CloudPulse/shared/CloudPulseTimeRangeSelect.tsx


  if (label === PAST_7_DAYS) {
-    return { unit: 'day', value: 7 };
+    return { unit: 'days', value: 7 };


The metrics API understands days instead of day going forward

packages/manager/src/features/CloudPulse/Overview/GlobalFilters.tsx

bnussman-akamai · 2024-08-29T13:22:10Z

packages/manager/src/features/CloudPulse/Utils/CloudPulseWidgetUtils.ts

-          borderColor: '',
+          borderColor: color,
          data: seriesDataFormatter(transformedData.values, start, end),
+          fill: widgetChartType === 'area' ? true : false,


Suggested change

fill: widgetChartType === 'area' ? true : false,

fill: widgetChartType === 'area',

@bnussman-akamai , fixed this comment, please check if it is good now

jaalah-akamai · 2024-08-30T03:22:57Z

packages/manager/src/features/CloudPulse/shared/CloudPulseCustomSelect.tsx

+              ? options
+              : queriedResources
+              ? queriedResources
+              : [],


Nit: Would options || queriedResources || [] give us the same here?

Yes, this will work as well, thanks for the simplification point, have addressed this. Please check if it is good now

github-actions · 2024-08-30T16:01:39Z

Coverage Report: ❌
Base Coverage: 86.15%
Current Coverage: 86.14%

…backs_bug_fix

coliu-akamai · 2024-09-03T13:25:36Z

packages/manager/src/features/CloudPulse/Dashboard/CloudPulseDashboard.tsx

+            subtitle={subtitle}
+            title=""


Should the subtitle be made the title instead?

Note: leaving the title blank affects accessibility - when I tested using voiceover on placeholders, the title gets read but the subtitle doesn't

coliu-akamai · 2024-09-03T13:27:15Z

packages/manager/src/features/CloudPulse/Utils/CloudPulseWidgetUtils.ts

+    resources.find((resourceObj) => String(resourceObj.id) === id)?.label ??
+    id ??
+    ''


nitpick: for readability, we could switch to having

const resource = resources.find((resourceObj) => String(resourceObj.id) === id);
(could name the const resourceObj instead)

outside of the return statement, and then here, just have

return resource?.label ?? id ?? '';

venkymano-akamai · 2024-09-03T14:24:32Z

@coliu-akamai , can you please check if it is good now

@bnussman-akamai , @jaalah-akamai feel free to look if it is good

coliu-akamai · 2024-09-03T16:19:05Z

This could just be a me problem, but I'm getting an error when trying to render all the graphs:

I have the CloudPulse flag and the MSW on - is there anything else I need to do?

confirmed fix asynchronously!

jaalah-akamai

Thanks for making changes

coliu-akamai

✅ Fixed Dashboard Select placeholder from Select a Dashboard to Select Dashboard
✅ Fixed striking functionality interchangeably while clicking native legend as well as legend table row.
✅ Enhanced Region Selection component to honor the placeholder prop being passed.
✅ Integrate manual refresh functionality
✅ Fix color for close icon in the CloudPulseResourcesSelection component
✅ Update labels in time duration from Past to Last

Quick note for global refresh - seems like part of the title flickers. I'm unsure if this is an issue or not/or if it's already known, but here's the video in case:

Screen.Recording.2024-09-03.at.1.23.49.PM.mov

Had a couple of other minor nitpicky comments below, but they're non-blocking. Thanks for the changes!

coliu-akamai · 2024-09-03T17:03:53Z

packages/manager/src/features/CloudPulse/Widget/CloudPulseWidget.tsx

          <CloudPulseLineGraph
            error={
-              status === 'error'
+              status === 'error' && error?.[0]?.reason !== jweTokenExpiryError // show the error only if the error is not related to token expiration


nitpick: now that error?.[0]?.reason is being used several times here and at lines 356, we could move it to a constant for better readability. Somewhere above, we could have something like const metricsError = error?.[0]?.reason; and then use metricsError here (potentially use a different name that better encompasses what the error is for)

coliu-akamai · 2024-09-03T17:09:36Z

packages/manager/src/components/LineGraph/LineGraph.tsx

      legend: {
        display: _nativeLegend,
+        onClick: (_e, legendItem) => {
+          if (legendItem && legendItem.datasetIndex!==undefined) {


Can we put spaces around the !==
(eslint complains otherwise)

…backs_bug_fix

venkymano-akamai · 2024-09-03T17:46:57Z

@coliu-akamai , Thanks for the approval, i have fixed those minors. Also the flickering is due to preferences, for which we have a separate PR that is also under review

venkymano-akamai added 4 commits August 29, 2024 08:22

upcoming: [DI-20348] - Fixing Feedbacks

7a57e5e

upcoming: [DI-20348] - Fixing empty dependency array

7d27173

upcoming: [DI-20348] - Defer placeholder from props

fae1d22

upcoming: [DI-20357] - Code Refactoring for bug fixes

62bae37

venkymano-akamai requested a review from a team as a code owner August 29, 2024 07:56

venkymano-akamai requested review from AzureLatte and jdamore-linode and removed request for a team August 29, 2024 07:56

upcoming: [DI-20501] - Add change set

b751a67

venkymano-akamai commented Aug 29, 2024

View reviewed changes

upcoming: [DI-20501] - Remove non nullish assertion

a1d1fca

venkymano-akamai commented Aug 29, 2024

View reviewed changes

venkymano-akamai added 3 commits August 29, 2024 14:53

upcoming: [DI-20501] - Getting started to Docs

c65bb73

upcoming: [DI-20501] - changing to themes

208746b

upcoming: [DI-20501] - changing from day to days

84b7f6f

venkymano-akamai commented Aug 29, 2024

View reviewed changes

venkymano-akamai changed the title ~~upcoming: [DI-20501] - Fix Demo feedbacks and small bugs across ACLP~~ upcoming: [DI-20501] - Fix Demo feedbacks and missed changes across ACLP Aug 29, 2024

upcoming: [DI-20501] - update changeset

a87dcca

nikhagra-akamai suggested changes Aug 29, 2024

View reviewed changes

packages/manager/src/features/CloudPulse/Overview/GlobalFilters.tsx Outdated Show resolved Hide resolved

nikhagra-akamai approved these changes Aug 29, 2024

View reviewed changes

upcoming: [DI-20501] - timestamp to use REFRESH constant

0a1f891

jaalah-akamai added the Cloud Pulse label Aug 29, 2024

jaalah-akamai assigned venkymano-akamai Aug 29, 2024

venkymano-akamai added 2 commits August 29, 2024 18:14

upcoming: [DI-20501] - full undefined checks

c4dee73

upcoming: [DI-20501] - ESlint changes

b020042

bnussman-akamai reviewed Aug 29, 2024

View reviewed changes

upcoming: [DI-20501] - PR comments

55fa719

venkymano-akamai requested a review from bnussman-akamai August 29, 2024 13:38

upcoming: [DI-20501] - UT failure fixes

7f92b02

jaalah-akamai reviewed Aug 30, 2024

View reviewed changes

upcoming: [DI-20501] - Loading icon while jwetoken fetch happening

c3f0e4c

venkymano-akamai requested a review from jaalah-akamai August 30, 2024 03:57

Merge branch 'develop' of https://github.com/linode/manager into feed…

ee4b6e5

…backs_bug_fix

kmuddapo approved these changes Sep 2, 2024

View reviewed changes

venkymano-akamai added 5 commits September 2, 2024 11:37

upcoming: [DI-20501] - Enable option chain checks

9247d76

upcoming: [DI-20501] - ESlint issue fixes

0dc9914

upcoming: [DI-20501] - is entity for placeholder

e14ec9b

upcoming: [DI-20501] - Enable beta for cloudpulse endpoints

9636269

upcoming: [DI-20501] - Add api-v4 changeset

517f64d

coliu-akamai reviewed Sep 3, 2024

View reviewed changes

upcoming: [DI-20501] - PR comments

f338d29

venkymano-akamai requested a review from coliu-akamai September 3, 2024 14:24

jaalah-akamai approved these changes Sep 3, 2024

View reviewed changes

jaalah-akamai added the Add'tl Approval Needed Waiting on another approval! label Sep 3, 2024

coliu-akamai approved these changes Sep 3, 2024

View reviewed changes

coliu-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Sep 3, 2024

venkymano-akamai added 2 commits September 3, 2024 23:14

upcoming: [DI-20501] - Code simplification

8d10526

Merge branch 'develop' of https://github.com/linode/manager into feed…

3a41b89

…backs_bug_fix

jaalah-akamai merged commit f787b09 into linode:develop Sep 3, 2024

This was referenced Sep 13, 2024

Release v1.128.0 - release → staging #10935

Merged

Release v1.128.0 - staging → master #10946

Merged

	fill: widgetChartType === 'area' ? true : false,
	fill: widgetChartType === 'area',

Conversation

venkymano-akamai commented Aug 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description 📝

Changes 🔄

Target release date 🗓️

Preview 📷

How to test 🧪

As an Author I have considered 🤔

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

venkymano-akamai Aug 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

venkymano-akamai commented Sep 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coliu-akamai commented Sep 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jaalah-akamai left a comment

Choose a reason for hiding this comment

Uh oh!

coliu-akamai left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

venkymano-akamai commented Sep 3, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

venkymano-akamai commented Aug 29, 2024 •

edited

Loading

venkymano-akamai Aug 30, 2024 •

edited

Loading

github-actions bot commented Aug 30, 2024 •

edited

Loading

venkymano-akamai commented Sep 3, 2024 •

edited

Loading

coliu-akamai commented Sep 3, 2024 •

edited

Loading

coliu-akamai left a comment •

edited

Loading