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

fix(plugin-chart-echarts): show zero value in tooltip #21296

Merged
merged 1 commit into from
Sep 1, 2022

Conversation

villebro
Copy link
Member

@villebro villebro commented Sep 1, 2022

SUMMARY

Currently only truthy values are shown on the tooltip of the ECharts plugin, causing zeros to be removed.

The PR fixes the bug and adds unit tests for both extracting the series and formatting (logic of both have been updated). In addition the tests are refactored to the preferred single test format (the refactoring changes can best be seen with "Hide whitespace changes").

AFTER

Now zero is shown on the tooltip; see the series SUM(0):
image

BEFORE

Previously only truthy values were shown, i.e. excluding zero:
image

TESTING INSTRUCTIONS

  1. Create a chart using the ECharts plugin
  2. Create a series that contains a zero value
  3. Check that the value show up on the tooltip

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

Copy link
Member

@stephenLYZ stephenLYZ left a comment

Choose a reason for hiding this comment

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

LGTM!

@codecov
Copy link

codecov bot commented Sep 1, 2022

Codecov Report

Merging #21296 (749e137) into master (034ee1c) will decrease coverage by 0.01%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master   #21296      +/-   ##
==========================================
- Coverage   66.43%   66.42%   -0.02%     
==========================================
  Files        1784     1784              
  Lines       68185    68185              
  Branches     7265     7264       -1     
==========================================
- Hits        45298    45291       -7     
- Misses      21018    21024       +6     
- Partials     1869     1870       +1     
Flag Coverage Δ
javascript 52.47% <50.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...plugins/plugin-chart-echarts/src/utils/forecast.ts 93.22% <50.00%> (ø)
superset-frontend/src/featureFlags.ts 28.57% <0.00%> (-71.43%) ⬇️
...rontend/src/components/ErrorMessage/ErrorAlert.tsx 89.47% <0.00%> (-0.78%) ⬇️
superset-frontend/src/SqlLab/actions/sqlLab.js 61.05% <0.00%> (-0.33%) ⬇️
...c/views/CRUD/data/database/DatabaseModal/index.tsx 32.03% <0.00%> (-0.17%) ⬇️
...d/src/SqlLab/components/SaveDatasetModal/index.tsx 52.87% <0.00%> (ø)
...onents/ErrorMessage/ErrorMessageWithStackTrace.tsx 60.00% <0.00%> (ø)
...-frontend/src/components/FilterableTable/index.tsx 72.63% <0.00%> (+0.29%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@villebro villebro merged commit 1aeb8fd into apache:master Sep 1, 2022
@villebro villebro deleted the villebro/echarts-nil-tooltip branch September 1, 2022 17:47
@villebro villebro added the v2.0 label Sep 9, 2022
eschutho pushed a commit that referenced this pull request Sep 20, 2022
Co-authored-by: Ville Brofeldt <ville.brofeldt@apple.com>
(cherry picked from commit 1aeb8fd)
AAfghahi pushed a commit that referenced this pull request Oct 6, 2022
Co-authored-by: Ville Brofeldt <ville.brofeldt@apple.com>
(cherry picked from commit 1aeb8fd)
Fahrenheit35 pushed a commit to Fahrenheit35/superset that referenced this pull request Nov 11, 2022
Co-authored-by: Ville Brofeldt <ville.brofeldt@apple.com>
(cherry picked from commit 1aeb8fd)
@mistercrunch mistercrunch added 🍒 2.0.1 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/L v2.0 v2.0.1 🍒 2.0.1 🚢 2.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants