fix(chart): Restore subheader used in bignumber with trendline #33196

LevisNgigi · 2025-04-21T18:23:30Z

SUMMARY

We lost the subheader in the big number with trendline as we tried to map it to subtitle.This aims at restoring the subheader being used in big number with trendline.
related to this pr

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

BEFORE

AFTER

TESTING INSTRUCTIONS

1)Create or open a chart using Big Number with Trendline chart configured with subheader/comparison suffix such as Wow,MoM etc
2)Confirm that the chart displays the subheader as expected.

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

eschutho · 2025-04-22T21:48:05Z

superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx

@@ -294,6 +343,11 @@ class BigNumberVis extends PureComponent<BigNumberVizProps> {
            {this.renderHeader(
              Math.ceil(headerFontSize * (1 - PROPORTION.TRENDLINE) * height),
            )}
+            {this.rendermetricComparisonSummary(
+              Math.ceil(
+                subheaderFontSize * (1 - PROPORTION.TRENDLINE) * height,


can we rename this to metricComparisonFontSize?

We can,this means the subheader font size in the UI needs updating here well?

eschutho · 2025-04-22T22:26:00Z

...end/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/transformProps.test.ts

+    );
+    expect(result.headerFormatter.format(500)).toBe('$500');
+  });
+});


Really great!

github-actions · 2025-04-23T12:13:03Z

@geido Processing your ephemeral environment request here. Action: up. More information on how to use or configure ephemeral environments

github-actions · 2025-04-23T12:23:32Z

@geido Ephemeral environment spinning up at http://44.247.250.79:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.

Antonio-RiveroMartnez · 2025-04-23T14:11:24Z

Hey! The suffix is working as expected, however, when you set a subtitle, and you remove it, it's not clearing it up:

Screen.Recording.2025-04-23.at.4.09.00.PM.mov

github-actions · 2025-04-23T18:21:49Z

@Antonio-RiveroMartnez Processing your ephemeral environment request here. Action: up. More information on how to use or configure ephemeral environments

github-actions · 2025-04-23T18:33:42Z

@Antonio-RiveroMartnez Ephemeral environment spinning up at http://34.221.106.45:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.

Antonio-RiveroMartnez

Ok, subheader and subtitle working as expected

Screen.Recording.2025-04-23.at.8.42.43.PM.mov

Vitor-Avila · 2025-04-24T03:57:59Z

Performed below manual tests on my end:

Spin up Superset from sha/commit 164a07e2be7854520070a7d845eec449405c4840 (last one before the PR that introduced subtitle - ref).
Created 5 Big Numbers (one for each subheader font size) and added them to a dashboard.
Did the same with the Big Number and Trendline chart type.
Checked out to commit d75ff9e784ca571e8498febb3b492874118312c8 (the one introducing subtitles - ref).
Validated that the subheader text have disappeared for the Big Numbers. Big Numbers with Trendline were still ok.
Created 5 more Big Numbers using the new schema (with subtitle) and added to the same dashboard.
Checked out to commit 41402617970368f0b14ff4e7acc1d271d5bcdd89 (the one that fixes the missing headers - ref). Validated that the Big Numbers are now showing the headers. Big Number with Trendline no longer show the subtitle. Also, the subtitle font sizes would show correct in the dashboard, but when accessing the charts in Explore they would always show up as Small (default value).
Checked out to this PR. Validated that:

Subheaders for Big Numbers with Trendlines are back. ✅
Their font size is showing correctly both in charts and dashboards. ✅
The subtitle for Big Numbers appear duplicated: 🔴

- Accessing the chart in Explore does not show the subtitle content (the subtitle size is also not correct): 🔴

Vitor-Avila · 2025-04-24T04:12:33Z

@LevisNgigi my last comment was longer just to highlight the testing scenarios and flows, but to summarize, these are the remaining issues:

Big Numbers created prior to the subtitle PR show the subheader text duplicated.
Accessing the chart directly in Explore does not carry over the subtitle config (both text and size).

To easily repro this:

Spin up Superset in dev mode.
Check out to commit 164a07e2be7854520070a7d845eec449405c4840.
Create a dashboard including one Big Number for each subheader size available. For example:

4. Checkout to this branch. 5. Refresh the dashboard:

6. Accessing a chart from this dashboard does not show the subtitle config:

Let me know if you have any questions!

LevisNgigi · 2025-04-24T07:44:56Z

@LevisNgigi my last comment was longer just to highlight the testing scenarios and flows, but to summarize, these are the remaining issues:

Big Numbers created prior to the subtitle PR show the subheader text duplicated.

Accessing the chart directly in Explore does not carry over the subtitle config (both text and size).

To easily repro this:

Spin up Superset in dev mode.

Check out to commit 164a07e2be7854520070a7d845eec449405c4840.

Create a dashboard including one Big Number for each subheader size available. For example:

4. Checkout to this branch. 5. Refresh the dashboard: 6. Accessing a chart from this dashboard does not show the subtitle config:
Let me know if you have any questions!

@Vitor-Avila Thanks for the thorough testing and concise feedback with repro steps.The steps were super helpful! I’ve fixed the subtitle duplication issue for Big Number charts. However, the issue where legacy charts (those created with subheader only) don’t show the subtitle content or correct size in Explore is still pending. Mapping the control to subheader brings back the value in explore and configs are restored, but it prevents the subtitle from being fully clearable, which isn’t ideal.Since subheader is also used as the comparison suffix in Big Number with Trendline, a simple migration isn’t straightforward I think.So wondering the best way forward.

Vitor-Avila · 2025-04-24T12:32:09Z

@LevisNgigi glad to help and excited we're making progress here! 🙌

hmm could we make the migration dynamic based on the viz type? If it's a Big Number migrate subheader to subtitle and subheaderFontSize to subtitleFontSize, if Big Number with Trendline don't do it? I believe we can differentiate the two via their params, right? Ideally this migration could happen on the fly by the frontend.

Let me know if that makes sense/is possible -- I'm not super close to our frontend logic so it's totally possible that this wouldn't work/isn't possible.

Vitor-Avila · 2025-04-25T02:45:01Z

superset-frontend/src/explore/components/ExploreViewContainer/index.jsx

+  if (
+    form_data.viz_type === 'big_number_total' &&
+    slice?.form_data?.subheader &&
+    (!controls.subtitle?.value || controls.subtitle.value === '')
+  ) {
+    controls.subtitle = {
+      ...controls.subtitle,
+      value: slice.form_data.subheader,
+    };
+  }
+  if (
+    form_data.viz_type === 'big_number_total' &&
+    slice?.form_data?.subheader_font_size &&
+    (!controls.subtitle_font_size?.value ||
+      controls.subtitle_font_size.value === '')
+  ) {
+    controls.subtitle_font_size = {
+      ...controls.subtitle_font_size,
+      value: slice.form_data.subheader_font_size,
+    };
+  }


I'm suggesting this change for two reasons:

It moves the second change (controls.subtitle_font_size = slice.form_data.subheader_font_size) as part of the first if block. This is mostly because if the chart has a subheader config and does not have a subtitle config, then subheader_font_size should be the source of truth. For safety, just checking if slice?.form_data?.subheader_font_size exists.

In the current implementation, !controls.subtitle_font_size?.value is evaluated. I believe controls.subtitle_font_size always gets a default value, so that second if would never really execute and instead the old subheader ends up with the default size.

Suggested change

if (

form_data.viz_type === 'big_number_total' &&

slice?.form_data?.subheader &&

(!controls.subtitle?.value || controls.subtitle.value === '')

) {

controls.subtitle = {

...controls.subtitle,

value: slice.form_data.subheader,

};

}

if (

form_data.viz_type === 'big_number_total' &&

slice?.form_data?.subheader_font_size &&

(!controls.subtitle_font_size?.value ||

controls.subtitle_font_size.value === '')

) {

controls.subtitle_font_size = {

...controls.subtitle_font_size,

value: slice.form_data.subheader_font_size,

};

}

if (

form_data.viz_type === 'big_number_total' &&

slice?.form_data?.subheader &&

(!controls.subtitle?.value || controls.subtitle.value === '')

) {

controls.subtitle = {

...controls.subtitle,

value: slice.form_data.subheader,

};

if (slice?.form_data?.subheader_font_size) {

controls.subtitle_font_size = {

...controls.subtitle_font_size,

value: slice.form_data.subheader_font_size,

};

}

}

Vitor-Avila · 2025-04-25T02:50:24Z

@LevisNgigi thank you so much for taking the time to implement these fixes! 🙏 I tested the flow again and:

Big Numbers with Trendline seem to be working properly. ✅
Existing Big Numbers are no longer showing both subheader and subtitle in the dashboard view. Just one is displayed. ✅
Accessing these Big Numbers in Explore shows a single entry as well. ✅
Accessing these Big Numbers in Explore defaults the subtitle size to the default value (Small) as opposed to respecting the old value from subheader. 🔴

I suggested a potential fix for this remaining issue, but I didn't want to commit it as I want your thoughts on it first. If you're comfortable with it, feel free to push it to this branch and I can do one final round of tests. 🙌

Vitor-Avila

LGTM!

LevisNgigi and others added 30 commits December 12, 2024 18:24

(refactor)Fix saved query access for SQL Lab users

0702ac1

Merge remote-tracking branch 'upstream/master'

9125aba

Merge branch 'apache:master' into master

9de7b1b

Merge branch 'apache:master' into master

e630b87

Merge branch 'apache:master' into master

824f097

Merge branch 'apache:master' into master

5751c81

Merge branch 'apache:master' into master

fd6b083

Merge branch 'apache:master' into master

e827801

Merge branch 'apache:master' into master

c3ce0c5

Merge branch 'apache:master' into master

7731fb7

Merge branch 'apache:master' into master

f23d780

feat:add subheaders to big number charts

6345323

feat:add subheaders to big number charts

dddd052

fix:test

5a58ac0

fix:small nits

5fcee79

fix:small nits

48644d7

fix:small nits

f401002

feat:add subtitle option to bignumber charts

c888455

feat:add subtitle option to bignumber charts

563119a

feat:add display icon functionality

da4c820

feat:add rename metric in bignumberperiodoverperiod

5f56622

fix:fix test to align with removed Main label

0f07309

fix:small nit

8157869

fix:small nit

14e9c01

fix:apply changes from feedback

1f72a29

fix:apply changes from feedback

489d748

fix:remove use of prefix

5db8d28

fix:remove use of prefix

310fd80

fix:remove use of prefix

3428660

fix:restore Main

56676c4

eschutho reviewed Apr 22, 2025

View reviewed changes

LevisNgigi added 2 commits April 23, 2025 10:50

fix:fix test?

c4b9ac6

fix:fix test?

aca484d

geido added testenv-up and removed testenv-up labels Apr 23, 2025

fix:fix deletion bug

811193e

Antonio-RiveroMartnez added testenv-up and removed testenv-up labels Apr 23, 2025

Antonio-RiveroMartnez approved these changes Apr 23, 2025

View reviewed changes

LevisNgigi mentioned this pull request Apr 23, 2025

feat: add metric name for big number chart types #33013 #33099

Closed

4 tasks

LevisNgigi added 4 commits April 24, 2025 09:59

fix:fix duplicate subheader

8723fdf

Merge branch 'add-subheaders' into restore-subheader

9d1e586

fix:fix duplicate subheader

a685590

fix:fix duplicate subheader

b885476

fix:fix explore big number

f5aefe4

Vitor-Avila reviewed Apr 25, 2025

View reviewed changes

fix:fix explore font size

46c4704

Vitor-Avila approved these changes Apr 25, 2025

View reviewed changes

Vitor-Avila merged commit 6a586fe into apache:master Apr 25, 2025
54 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(chart): Restore subheader used in bignumber with trendline #33196

fix(chart): Restore subheader used in bignumber with trendline #33196

LevisNgigi commented Apr 21, 2025 •

edited

Loading

eschutho Apr 22, 2025

LevisNgigi Apr 23, 2025

eschutho Apr 22, 2025

github-actions bot commented Apr 23, 2025

github-actions bot commented Apr 23, 2025

Antonio-RiveroMartnez commented Apr 23, 2025

github-actions bot commented Apr 23, 2025

github-actions bot commented Apr 23, 2025

Antonio-RiveroMartnez left a comment

Vitor-Avila commented Apr 24, 2025

Vitor-Avila commented Apr 24, 2025

LevisNgigi commented Apr 24, 2025

Vitor-Avila commented Apr 24, 2025

Vitor-Avila Apr 25, 2025

Vitor-Avila commented Apr 25, 2025

Vitor-Avila left a comment

fix(chart): Restore subheader used in bignumber with trendline #33196

fix(chart): Restore subheader used in bignumber with trendline #33196

Conversation

LevisNgigi commented Apr 21, 2025 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

eschutho Apr 22, 2025

Choose a reason for hiding this comment

LevisNgigi Apr 23, 2025

Choose a reason for hiding this comment

eschutho Apr 22, 2025

Choose a reason for hiding this comment

github-actions bot commented Apr 23, 2025

github-actions bot commented Apr 23, 2025

Antonio-RiveroMartnez commented Apr 23, 2025

github-actions bot commented Apr 23, 2025

github-actions bot commented Apr 23, 2025

Antonio-RiveroMartnez left a comment

Choose a reason for hiding this comment

Vitor-Avila commented Apr 24, 2025

Vitor-Avila commented Apr 24, 2025

LevisNgigi commented Apr 24, 2025

Vitor-Avila commented Apr 24, 2025

Vitor-Avila Apr 25, 2025

Choose a reason for hiding this comment

Vitor-Avila commented Apr 25, 2025

Vitor-Avila left a comment

Choose a reason for hiding this comment

LevisNgigi commented Apr 21, 2025 •

edited

Loading