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

Piechart: Implements series override -> hide in area for the legend or tooltip #51297

Merged
merged 7 commits into from
Jun 27, 2022

Conversation

daniellee
Copy link
Contributor

What this PR does / why we need it:

Missing functionality in the pie chart panel for the Series override -> Hide in area.

Shows hiding a country first from the legend and then from the multi tooltip.

Piechart-hide

Which issue(s) this PR fixes:

Fixes #49972

Special notes for your reviewer:

Not sure if there is a way to avoid duplication. There are two places where this series override is implemented already in the PlotLegend and the TooltipPlugin.

I looked at the PlotLegend code for the legend and tried to keep it similar. The tooltip logic is simpler than the TooltipPlugin implementation.

@daniellee daniellee requested review from a team, codeincarnate and oscarkilhed and removed request for a team June 23, 2022 07:38
@daniellee daniellee added the area/panel/piechart Core pie chart visualization label Jun 23, 2022
@grafanabot
Copy link
Contributor

@daniellee daniellee added no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes add to changelog and removed no-changelog Skip including change in changelog/release notes labels Jun 23, 2022
Comment on lines 97 to 102
const hideFromViz = value.field.custom.hideFrom.viz;
const hideFromLegend = value.field.custom?.hideFrom?.legend;

if (hideFromLegend) {
return undefined;
}

Copy link
Member

Choose a reason for hiding this comment

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

Maybe

Suggested change
const hideFromViz = value.field.custom.hideFrom.viz;
const hideFromLegend = value.field.custom?.hideFrom?.legend;
if (hideFromLegend) {
return undefined;
}
const hideFrom = (value.field.custom?.hideFrom ?? {}) as HideSeriesConfig;
if (hideFrom.legend) {
return undefined;
}
const hideFromViz = Boolean(hideFrom.viz)

it has the hideFrom time involved, so a little nicer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is nicer. Couldn't commit your suggestion as it was missing the import for HideSeriesConfig - so added it manually.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually ran into a betterer check for your suggestion - it is adding a type assertion:

image

@ryantxu ryantxu added this to the 9.1.0 milestone Jun 23, 2022
@daniellee daniellee changed the title Piechart: implements series override for hiding a series in a legend or tooltip Piechart: implements series override -> hide in area for legend or tooltip Jun 23, 2022
@daniellee daniellee changed the title Piechart: implements series override -> hide in area for legend or tooltip Piechart: Implements series override -> hide in area for legend or tooltip Jun 23, 2022
@grafanabot
Copy link
Contributor

@daniellee
Copy link
Contributor Author

@ryantxu looks like #50229 broke the main branch. Tons of errors in betterer that are nothing to do with this PR.

@grafanabot
Copy link
Contributor

@grafanabot
Copy link
Contributor

Copy link
Member

@ryantxu ryantxu left a comment

Choose a reason for hiding this comment

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

Looks good! now all the things work :)
2022-06-25_15-02-54 (1)

I still don't get how/why using ... as TypeWeKnow is worse than using the untyped existing any, but that is a much more complicated discussion

@daniellee daniellee changed the title Piechart: Implements series override -> hide in area for legend or tooltip Piechart: Implements series override -> hide in area for the legend or tooltip Jun 27, 2022
@daniellee daniellee merged commit 292b24e into main Jun 27, 2022
@daniellee daniellee deleted the 49972-piechart-hidefrom branch June 27, 2022 13:13
@IevaVasiljeva IevaVasiljeva modified the milestone: 9.1.0-beta1 Aug 2, 2022
@adamxyzxyz
Copy link

hey guys what version is this live in?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pie chart: Can't hide items from legend or tooltip
5 participants