fix(plugin-chart-echarts): remove erroneous upper bound value

[villebro]

SUMMARY

Currently when selecting "show value" on the chart, the upper bound shows the confidence bound width (400), not the actual value (which is 1.76k). In the tooltip this is fixed by adding the lower bound to the upper bound. However, this is more difficult to implement in the label formatter, as it only receives the single value of the series, while the tooltip formatter receives all series, making it easier to do the addition.

BEFORE

Notice the incorrect upper bound label (400), as opposed to the actual value seen on the tooltip (1.76k):

AFTER

Now the lower and upper bound labels are no longer shown when hovering:

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

• Has associated issue: closes Confidence interval shading in timeseries displays the wrong upper bound value #32428
• 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

[korbit-ai]

I've completed my review and didn't find any issues.

Files scanned

​

File Path	Reviewed
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Need a new review? Comment /korbit-review on 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-review command in a comment at the root of your PR.
• You can ask Korbit to generate a new PR description using the /korbit-generate-pr-description command in any comment on your PR.
• Too many Korbit comments? I can resolve all my comment threads if you use the /korbit-resolve command in any comment on your PR.
• Chat with Korbit on issues we post by tagging @korbit-ai in your reply.
• 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.

Current Korbit Configuration

General Settings

​

Setting	Value
Review Schedule	Automatic excluding drafts
Max Issue Count	10
Automatic PR Descriptions	❌

Issue Categories

​

Category	Enabled
Documentation	✅
Logging	✅
Error Handling	✅
Readability	✅
Design	✅
Performance	✅
Security	✅
Functionality	✅

Feedback and Support

• Tell us what you think of Korbit
• Schedule a call with our team
• Email us @ [email protected]

Note

Korbit Pro is free for open source projects 🎉

Looking to add Korbit to your team? Get started with a free 2 week trial here

[rusackas]

LGTM. I haven't checked if the feature is covered by RTL, but maybe there's an enhancement if so. If it's enzyme, then I wouldn't bother at this time.

[villebro]

LGTM. I haven't checked if the feature is covered by RTL, but maybe there's an enhancement if so. If it's enzyme, then I wouldn't bother at this time.

I don't think this is testable with RTL, as we're just wrapping ECharts in a thin React component (ECharts implements all the drawing stuff at a very low level with ZRender). But working on this gave me a good proper headache, so this plugin is definitely in need of some cleanup at some point. Once we have the new extensions architecture in place, I'd love for us to give this plugin some TLC and rethink how we can do some real RTL-style testing.