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

[datadog_dashboard] Add support for sunburst widget request style #2089

Merged
merged 8 commits into from
Sep 1, 2023

Conversation

matt-miller-ddog
Copy link
Contributor

Adding the a missing style property to sunburst widget requests. Corresponding Go client PR: DataDog/datadog-api-client-go#2158.

@matt-miller-ddog matt-miller-ddog requested review from a team as code owners August 31, 2023 17:09
@nkzou nkzou changed the title Add support for sunburst widget request style [datadog_dashboard] Add support for sunburst widget request style Aug 31, 2023
@matt-miller-ddog matt-miller-ddog requested a review from a team as a code owner August 31, 2023 17:15
@@ -5926,6 +5926,16 @@ func getSunburstRequestSchema() map[string]*schema.Schema {
// "query" and "formula" go together
"query": getFormulaQuerySchema(),
"formula": getFormulaSchema(),
// Settings specific to Sunburst requests
"style": {
Description: "Define style for the widget's request.",
Copy link
Contributor Author

@matt-miller-ddog matt-miller-ddog Aug 31, 2023

Choose a reason for hiding this comment

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

FYI, I flipped the wording compared to how Heatmaps describe this. (I think it makes more sense this way, but let me know if I should change it.)

Copy link

@woodb woodb left a comment

Choose a reason for hiding this comment

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

lgtm 👍 One minor comment on the docs link 🙏


Optional:

- `palette` (String) A color palette to apply to the widget. The available options are available at: https://docs.datadoghq.com/dashboards/widgets/timeseries/#appearance.
Copy link

Choose a reason for hiding this comment

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

question: Is it intentional that we're linking to the timeseries docs here? If these docs are generated then we may need to fix it upstream

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like most of the widget request styles share the same schema from getWidgetRequestStyle() line 9192 of resource_datadog_dashboard.go. Is there a more generic list of styles that could be linked to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't really want to rock the boat here; but, offhand, https://docs.datadoghq.com/dashboards/guide/widget_colors/ looks like a better page; at the very least we should replace #appearance with #color so it links right.

I'll run this by Dataviz and see what they advise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update: it's on their radar. They're going to do a review of color docs, as I think there are a few more stale things beyond just this.


Optional:

- `palette` (String) A color palette to apply to the widget. The available options are available at: https://docs.datadoghq.com/dashboards/widgets/timeseries/#appearance.
Copy link

Choose a reason for hiding this comment

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

question: Is it intentional that we're linking to the timeseries docs here? If these docs are generated then we may need to fix it upstream

Copy link

@cswatt cswatt left a comment

Choose a reason for hiding this comment

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

approved for docs

@nkzou nkzou merged commit 95e7746 into master Sep 1, 2023
8 checks passed
@nkzou nkzou deleted the matt.miller/sunburst-request-style branch September 1, 2023 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants