feat: add metric name for big number chart types#33013
feat: add metric name for big number chart types#33013fardin-developer wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
| Incorrect Boolean Type Declaration ▹ view | ✅ Fix detected |
Files scanned
| File Path | Reviewed |
|---|---|
| superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberPeriodOverPeriod/types.ts | ✅ |
| superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/types.ts | ✅ |
| superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/sharedControls.ts | ✅ |
| superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/transformProps.ts | ✅ |
| superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberPeriodOverPeriod/utils.ts | ✅ |
| superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/controlPanel.ts | ✅ |
| superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberPeriodOverPeriod/controlPanel.ts | ✅ |
| superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberPeriodOverPeriod/transformProps.ts | ✅ |
| superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberPeriodOverPeriod/PopKPI.tsx | ✅ |
| superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Need a new review? Comment
/korbit-reviewon this PR and I'll review your latest changes.Korbit Guide: Usage and Customization
Interacting with Korbit
- You can manually ask Korbit to review your PR using the
/korbit-reviewcommand in a comment at the root of your PR.- You can ask Korbit to generate a new PR description using the
/korbit-generate-pr-descriptioncommand in any comment on your PR.- Too many Korbit comments? I can resolve all my comment threads if you use the
/korbit-resolvecommand in any comment on your PR.- On any given comment that Korbit raises on your pull request, you can have a discussion with Korbit by replying to the comment.
- Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.
Customizing Korbit
- Check out our docs on how you can make Korbit work best for you and your team.
- Customize Korbit for your organization through the Korbit Console.
Feedback and Support
|
@kgabryje Processing your ephemeral environment request here. Action: up. More information on how to use or configure ephemeral environments |
|
@kgabryje Ephemeral environment spinning up at http://34.214.166.129:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup. |
There was a problem hiding this comment.
Would it be better to use the theme.typography.sizes?
There was a problem hiding this comment.
Should be weird if existing charts start showing the metric all of the sudden, wouldn't be better to default to false?
There was a problem hiding this comment.
+1 for this. It should be either false by default (preferable I guess? and low effort solution) or we need a migration that sets that to false in existing charts
There was a problem hiding this comment.
Should be weird if existing charts start showing the metric all of the sudden, wouldn't be better to default to false?
Fixed
There was a problem hiding this comment.
Same comment about default from above
There was a problem hiding this comment.
Changed to false
There was a problem hiding this comment.
Curious what's the header here? the big number? and probably we could use the same sizes as the header or subheader?
There was a problem hiding this comment.
Yes, the header font size is used for displaying the big number. I’ve unified the font sizes for both the metric name and subheader (comparison) into a single array:
const sharedFontSizes = [16, 20, 26, 32, 40];
There was a problem hiding this comment.
Not fault of this PR but we could DRY these utils a bit since all these methods are pretty much the same but reading from different arrays: metricNameFontSizes, headerFontSizes etc.
There was a problem hiding this comment.
Thank you, I've DRYed it up, let me know if it looks good!
There was a problem hiding this comment.
Could we DRY this to render any info? because this is the same as how you're rendering kickerFontSize
There was a problem hiding this comment.
Yup, I DRY’d it into a renderWithFontSize(rendererFn, fontSize) helper method that calculates the height based on the font size and optional trendline, and renders the desired content using the passed function.
|
Do we want this feature in Big Number with Trendline as well? I think the feature wasn't implemented there |
|
@kasiazjc to review ui changes. |
Thank you @kgabryje. I have added metric name for Big Number with Trendline as well. |
…gNumberWithTrendline
364b15b to
43bb4a1
Compare
|
Created a new PR |
…#33099) Co-authored-by: Fardin Mustaque <fardinmustaque@Fardins-Mac-mini.local>
…#33099) Co-authored-by: Fardin Mustaque <fardinmustaque@Fardins-Mac-mini.local>

Fixes: #32985
SUMMARY
This PR introduces support for displaying the Metric name in Big Number chart types, including Big Number and Big Number with Time Comparison. The metric name appears above the main number, styled consistently with existing elements, and includes customization options in the control panel.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
CHANGES
BEFORE/AFTER SCREENSHOTS
Before
After
Screen.Recording.2025-04-07.at.12.32.34.AM.mov
ADDITIONAL NOTES
CHECKLIST