Skip to content
This repository has been archived by the owner on Jun 25, 2020. It is now read-only.

fix: fixing tooltip for expanded area chart #134

Merged

Conversation

michellethomas
Copy link
Contributor

@michellethomas michellethomas commented Jul 2, 2019

🐛 Bug Fix

This PR added functionality for more data in the area chart rich tooltip, but those calculations don't make sense for the expanded chart. The expanded chart tooltip already shows % of total values and doesn't have a total line item. generateAreaChartTooltipContent shouldn't be called for expand stacked style.

image

@graceguo-supercat @etr2460

@michellethomas michellethomas requested a review from a team as a code owner July 2, 2019 15:31
@codecov
Copy link

codecov bot commented Jul 2, 2019

Codecov Report

Merging #134 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #134   +/-   ##
=======================================
  Coverage   19.87%   19.87%           
=======================================
  Files           8        8           
  Lines         161      161           
  Branches       10       10           
=======================================
  Hits           32       32           
  Misses        128      128           
  Partials        1        1

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1738572...cb651fa. Read the comment docs.

Copy link
Contributor

@etr2460 etr2460 left a comment

Choose a reason for hiding this comment

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

How does this look when you click one of the items in the legend? In the current behavior, I think the tooltip has additional information when you unselect series from the chart

@michellethomas
Copy link
Contributor Author

expanded:
image

stacked:
image

It's not super necessary to show the 100% on the stacked chart but it's not incorrect so it seems ok.

@etr2460
Copy link
Contributor

etr2460 commented Jul 2, 2019

Sorry, I meant when a subset of series are selected, like this:
image

The percents there provide additional information as the fraction of the selected series that each individual series makes up.

@michellethomas
Copy link
Contributor Author

Oh I see, it just adjusts the %:
image

Copy link
Contributor

@etr2460 etr2460 left a comment

Choose a reason for hiding this comment

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

lgtm

@etr2460 etr2460 merged commit 6a4986e into apache-superset:master Jul 5, 2019
@michellethomas michellethomas deleted the fix_area_expanded_tooltip branch July 8, 2019 18:16
nytai pushed a commit to preset-io/superset-ui-plugins that referenced this pull request Apr 27, 2020
nytai pushed a commit to preset-io/superset-ui-plugins that referenced this pull request Apr 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants