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 incorrect tooltip value in pie chart #39180

Merged
merged 1 commit into from
Feb 27, 2024

Conversation

EmmadUsmani
Copy link
Contributor

@EmmadUsmani EmmadUsmani commented Feb 26, 2024

Closes #33342
Closes #32430

Description

For pie chart tooltips, we were incorrectly using the .value property of the pie chart slice. We can't use this because it is modified one case to make slices smaller than the minimum percentage setting look bigger.

    if (otherSlice.value > 0) {
      // increase "other" slice so it's barely visible
      if (otherSlice.percentage < OTHER_SLICE_MIN_PERCENTAGE) {
        otherSlice.value = total * OTHER_SLICE_MIN_PERCENTAGE;
      }
      slices.push(otherSlice);
    }

Instead, we should be using slice.displayValue, as was intended

        // Value is used to determine arc size and is modified for very small
        // other slices. We save displayValue for use in tooltips.
        value: row[metricIndex],
        displayValue: row[metricIndex],

Demo

Before After
image Screenshot 2024-02-26 at 1 43 35 PM
image Screenshot 2024-02-26 at 1 46 42 PM

Checklist

  • Tests have been added/updated to cover changes in this PR

Copy link
Contributor Author

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@metabase-bot metabase-bot bot added the .Team/DashViz Dashboard and Viz team label Feb 26, 2024
@EmmadUsmani EmmadUsmani added backport Automatically create PR on current release branch on merge and removed .Team/DashViz Dashboard and Viz team labels Feb 26, 2024 — with Graphite App
@EmmadUsmani EmmadUsmani marked this pull request as ready for review February 26, 2024 21:41
@metabase-bot metabase-bot bot added the visual Run Percy visual testing label Feb 26, 2024
Copy link

Codenotify: Notifying subscribers in CODENOTIFY files for diff d092c30...897eccd.

Notify File(s)
@alxnddr frontend/src/metabase/visualizations/visualizations/PieChart/utils.ts
frontend/src/metabase/visualizations/visualizations/PieChart/utils.unit.spec.ts

@EmmadUsmani EmmadUsmani requested review from alxnddr and a team February 26, 2024 21:47
Copy link

replay-io bot commented Feb 26, 2024

Status Complete ↗︎
Commit 897eccd
Results
⚠️ 3 Flaky
2321 Passed

@EmmadUsmani EmmadUsmani enabled auto-merge (squash) February 26, 2024 22:23
@EmmadUsmani EmmadUsmani removed the visual Run Percy visual testing label Feb 26, 2024 — with Graphite App
@EmmadUsmani EmmadUsmani merged commit c8fb1be into master Feb 27, 2024
205 of 235 checks passed
@EmmadUsmani EmmadUsmani deleted the 02-26-fix_incorrect_tooltip_value_in_pie_chart branch February 27, 2024 00:11
Copy link

@EmmadUsmani Did you forget to add a milestone to the issue for this PR? When and where should I add a milestone?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Automatically create PR on current release branch on merge
Projects
None yet
2 participants