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(Handlebars): formatNumber and group helpers #31261

Merged
merged 2 commits into from
Dec 3, 2024

Conversation

Vitor-Avila
Copy link
Contributor

SUMMARY

This PR adds two new helpers to the Handlebars chart:

  • formatNumber: allows using number.toLocaleString(locale) to format numbers as needed.
  • group (via the handlebars-group-by module): Allows grouping data.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Example using both helpers:
image

TESTING INSTRUCTIONS

  1. Create a Handlebars chart using the cleaned_sales_data dataset.
  2. Use year and deal_size as dimensions and create any metric with the test_metric label.
  3. Use below Handlebars template:
{{#group data by="year"}}
<h3>Year: {{ value }}</h3>
<ul>
{{#each items}}
	<li>{{ deal_size }} deals: {{ formatNumber test_metric "de-DE"}}</li>
{{/each}}
</ul>
{{/group}}

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

Copy link
Member

@kgabryje kgabryje left a comment

Choose a reason for hiding this comment

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

LGTM

@jediwarpraptor
Copy link

This is phenomenal @Vitor-Avila - HUGE! 🎉 🚀 💜

Copy link
Member

@rusackas rusackas left a comment

Choose a reason for hiding this comment

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

Love it!
Could add a test, but it would be a pretty low-likelihood-to-fail test since it would just test that JS does JS stuff.
• We're low on docs for this thing, but this is a reminder that we could really use some, and I'd say we should update them if we had them at all.

@Vitor-Avila
Copy link
Contributor Author

@rusackas 100% agreed. I just took a look at https://superset.apache.org/docs/using-superset/exploring-data, and we don't have a section for Handlebars just yet. I'll see if I can work on a follow up PR covering docs (and perhaps testing).

@Vitor-Avila Vitor-Avila merged commit 77f3764 into master Dec 3, 2024
33 checks passed
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