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

Pie Chart Legend hover behavior inconsistent #20142

Closed
SuberFu opened this issue Nov 15, 2023 · 7 comments · Fixed by #20299
Closed

Pie Chart Legend hover behavior inconsistent #20142

SuberFu opened this issue Nov 15, 2023 · 7 comments · Fixed by #20299

Comments

@SuberFu
Copy link

SuberFu commented Nov 15, 2023

Expected behaviour

Hover on pie element does the following.
Gray everything out and highlight hovered element.
Same happens to the legend item.

Actual behaviour

Hover on pie element has different behavior than hover on legend element.
Hover on legend element also cause grayout of element entries and the active element doesn't "ungray" out.

Live demo with steps to reproduce

Link

Product version

Highchart 11.2.0

Affected browser(s)

Chrome

@highsoft-bot highsoft-bot added this to To do in Development-Flow via automation Nov 15, 2023
@bm64
Copy link
Member

bm64 commented Nov 16, 2023

Hi @SuberFu,

I'm unable to replicate the hover behavior issue you described. Could you please provide two screenshots showing the issue? One showing the hover effect on the pie chart, and another on the legend.

@TorsteinHonsi
Copy link
Collaborator

I'm unable to replicate the hover behavior issue you described.

I think the problem is the sliced point, Samsung. It behaves differently.

  • When hovering any point, including the Samsung slice, the Samsung slice stays fully opaque.
  • When hovering any legend item, including the Samsung legend item, the Samsung slice stays semi-transparent.

The expectation is that sliced points behave the same as other points when it comes to the inactive state.

In non-styled mode it behaves slightly different, but also not as expected: https://jsfiddle.net/highcharts/ba4qh0je/ . The Samsung slice stays fully opaque regardless of whether we are hovering the slices or legends.

@jakubjanuchta
Copy link
Contributor

After thorough investigation, it looks like in point.setState, we don't set sliced point's state to inactive when hovering on a different point because we don't want to change state of a selected point when current state is different than select, which I think makes sense for other series but not for pie.

if (
// already has this state
(state === point.state && !move) ||
// selected points don't respond to hover
(point.selected && state !== 'select') ||
// series' state options is disabled
(stateOptions.enabled === false) ||
// general point marker's state options is disabled
(state && (
stateDisabled ||
(normalDisabled &&
(markerStateOptions as any).enabled === false)
)) ||
// individual point marker's state options is disabled
(
state &&
pointMarker.states &&
(pointMarker.states as any)[state] &&
(pointMarker.states as any)[state].enabled === false
) // #1610
) {
return;
}

We can change the point's state to inactive for pie's sliced point, but than we lose the select state options which I think doesn't make sense, however we could update the point with previous select and current inactive states merged together.

What do you think @TorsteinHonsi?

@TorsteinHonsi
Copy link
Collaborator

Tough call @jakubjanuchta ...

The demo works perfectly if we remove line 1407 altogether. But that's just luck, because we haven't applied any visual overrides for the select state. If we do the same with the column series, it has a specific visual style for the selected state. https://jsfiddle.net/highcharts/edf30vm2/ . And removing line 1407 will eradicate those once it is hovered.

I think it is a bug that we can't set states.select.borderColor etc. on a pie series. I don't know why it is different from the column. Probably because we associate selected pie slices with the sliced property.

I think your proposal of merging the select and inactive states is worth a try. It makes sense that a point can be both selected and inactive. But we must make sure that when the point is unhovered, it reverts back to the select state, not normal. The column series is a good test for that.

@jakubjanuchta
Copy link
Contributor

Tough call @jakubjanuchta ...

The demo works perfectly if we remove line 1407 altogether. But that's just luck, because we haven't applied any visual overrides for the select state. If we do the same with the column series, it has a specific visual style for the selected state. https://jsfiddle.net/highcharts/edf30vm2/ . And removing line 1407 will eradicate those once it is hovered.

I think it is a bug that we can't set states.select.borderColor etc. on a pie series. I don't know why it is different from the column. Probably because we associate selected pie slices with the sliced property.

I think your proposal of merging the select and inactive states is worth a try. It makes sense that a point can be both selected and inactive. But we must make sure that when the point is unhovered, it reverts back to the select state, not normal. The column series is a good test for that.

@TorsteinHonsi

I've got one more thought. Actually, the behavior for the pie series and for the column is exactly the same, see:
https://jsfiddle.net/BlackLabel/d1wkjpvq/

If we select a column point and we set states.select.color to 'red', it will stay red and fully opaque, no matter if other points hovered. Even if we set states.hover.color to 'green' and hover the selected point, it will stay red. And that's exactly the same what happens for pie series.

Now I'm not sure, should we still change this behavior for the pie?

@TorsteinHonsi
Copy link
Collaborator

@jakubjanuchta No, I don't think we should change that behaviour.

But there's this think with the inactive state - the column doesn't have inactive states for the other points...

@jakubjanuchta
Copy link
Contributor

@jakubjanuchta No, I don't think we should change that behaviour.

I agree, thanks!

But there's this think with the inactive state - the column doesn't have inactive states for the other points...

@TorsteinHonsi
I'm not sure what do you mean by that, could you please explain it?

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

Successfully merging a pull request may close this issue.

6 participants