Skip to content
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

feat(cloudwatch): add support for labels on pie graphs (aws#28929). #30287

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kofrasa
Copy link

@kofrasa kofrasa commented May 21, 2024

Add a new "labels" property for graph widgets to show ratios for pie graphs.

Issue #28929

Closes #28929.

Reason for this change

When using a pie graph, the percentage ratios are not shown unless the { "labels": { "visible": true } } property is enabled.

Description of changes

The commits adds a new "labels" property with the type LabelProps on the graph widget.
The LabelProps type contains a single boolean key"visible" which toggles the presence of the label on a pie graph. When set to true, CloudWatch dashboard will render the percentage ratios of each segment of the pie graph.
The README file has been updated to reflect the new property and how it can be used.

Description of how you validated changes

The commit includes unit, integration, and snapshot tests as would be generated by the CDK library.
The change has also been manually verified from the CloudWatch console by enabling and disabling the property.

Checklist


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@github-actions github-actions bot added beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 labels May 21, 2024
@aws-cdk-automation aws-cdk-automation requested a review from a team May 21, 2024 04:17
@kofrasa kofrasa force-pushed the cloudwatch-labels-prop branch 2 times, most recently from fdcd22b to c0d89d9 Compare May 23, 2024 23:40
@aws-cdk-automation aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label May 24, 2024
@kofrasa kofrasa force-pushed the cloudwatch-labels-prop branch 2 times, most recently from 7f1046b to 3a0acf4 Compare May 25, 2024 00:28
@kofrasa kofrasa force-pushed the cloudwatch-labels-prop branch 2 times, most recently from 5bb7e01 to 12d4594 Compare May 28, 2024 17:50
@kofrasa kofrasa force-pushed the cloudwatch-labels-prop branch 2 times, most recently from 1c4f9c9 to 787be13 Compare June 4, 2024 00:03
Copy link
Contributor

@nmussy nmussy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some documentation changes

packages/aws-cdk-lib/aws-cloudwatch/lib/graph.ts Outdated Show resolved Hide resolved
Comment on lines 44 to 46
labels: {
visible: true,
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add test cases for labels: {} and labels: {visible: false}, just to make sure CloudFormation handles them properly?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do, thanks.

@@ -496,6 +496,18 @@ dashboard.addWidgets(new cloudwatch.GraphWidget({
}));
```

When the `view` property is set to `GraphWidgetView.PIE`, the `labels` property can be used to toggle whether the magnitude of a pie segment is shown on the graph as a percentage of the whole.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, I think visible: false can be used for all other widgets types, pie charts just happen to have a different default value?

@aws-cdk-automation aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jun 4, 2024
@kofrasa kofrasa force-pushed the cloudwatch-labels-prop branch 4 times, most recently from 2510a44 to e7319ce Compare June 6, 2024 18:37
@aws-cdk-automation
Copy link
Collaborator

This PR has been in the BUILD FAILING state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

Add a new "labels" property for graph widgets to support toggling visibility status.

=== Issue aws#28929 ===

Closes aws#28929.

=== Reason for this change ===

The labels property that allows for toggling the visibility of labels on graphs is currently missing.

=== Description of changes ===

The commits adds a new `"labels"` property with the type `LabelProps` on the graph widget.
The `LabelProps` type contains a single boolean key`"visible"` which toggles the presence of the label on a pie graph. When set to `true`, CloudWatch dashboard will render the percentage ratios of each segment of the pie graph.
The README file has been updated to reflect the new property and how it can be used.

=== Description of how you validated changes ===

The commit includes unit, integration, and snapshot tests as would be generated by the CDK library.
The change has also been manually verified from the CloudWatch console by enabling and disabling the property.

=== Checklist ===

- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 631f5f0
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

This PR has been in the BUILD FAILING state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(aws-cloudwatch): support labels on pie charts
3 participants