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

Drilldowns Don't Respect allowPointSelect: true #5113

Closed
markmsmith opened this Issue Mar 10, 2016 · 3 comments

Comments

Projects
None yet
2 participants
@markmsmith

markmsmith commented Mar 10, 2016

If you enable drilldowns for a map but also enable allowPointSelect, I would expect to be able to select and multi-select values by holding down shift, cmd etc and not drilldown. However, the drilldown's click handler doesn't check for modifier keys and it runs before click's the defaultFunction (which DOES check for the modifiers), so clicking always drills down. I'd also expect when drilling down to NOT fire select events, but that seems to happen since the default function isn't cancelled by the click handler. Lastly, when you drill back up, any points selected at the previous level from before you drilled down display with with their 'select' state styling, but reset to normal after triggering a mouseout by moving your cursor over the selected point and back out.

Demo fiddle: http://jsfiddle.net/markmsmith/x6v8Lpnz/

The check for the click event's modifier keys should be here and when drilling down it should return false to prevent incorrect selections (after issue #5112 is fixed). I've worked around the phantom select styling on drill-up by just clearing all selections when the 'drilldown' event is fired, but it would be nice to have it either clear them automatically on drill down (preferred) or have the option to preserve selections.

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Mar 14, 2016

Collaborator

Thanks for your input!

The check for the click event's modifier keys should be here

What type of check do you have in mind here? Hard coding modifier keys is probably not a good idea, since allowPointSelect also runs without a modifier key...

Collaborator

TorsteinHonsi commented Mar 14, 2016

Thanks for your input!

The check for the click event's modifier keys should be here

What type of check do you have in mind here? Hard coding modifier keys is probably not a good idea, since allowPointSelect also runs without a modifier key...

@markmsmith

This comment has been minimized.

Show comment
Hide comment
@markmsmith

markmsmith Mar 16, 2016

I was thinking that just checking for the modifier keys would be fine, since it didn't make sense to me to still select when drilling down, and hard coding them is what the default click handler does.
I know that's a divergence from allowPointSelect behavior when you don't have drilldown enabled, but otherwise there wouldn't be a way to drilldown without selecting when you do have it turned on. I mean, I guess you could require some different modifier to be held to NOT select when drilling down, but that seems even weirder.

We're basically telling the user, if you want to have allowPointSelect AND drilldown turned on for a Point at the same time, you need to hold a modifier to do selects, since otherwise we can't know whether you intended a click to do drilldown or select. This wouldn't affect points that don't have a drilldown specified, which can still be selected without modifier keys in the usual way.

markmsmith commented Mar 16, 2016

I was thinking that just checking for the modifier keys would be fine, since it didn't make sense to me to still select when drilling down, and hard coding them is what the default click handler does.
I know that's a divergence from allowPointSelect behavior when you don't have drilldown enabled, but otherwise there wouldn't be a way to drilldown without selecting when you do have it turned on. I mean, I guess you could require some different modifier to be held to NOT select when drilling down, but that seems even weirder.

We're basically telling the user, if you want to have allowPointSelect AND drilldown turned on for a Point at the same time, you need to hold a modifier to do selects, since otherwise we can't know whether you intended a click to do drilldown or select. This wouldn't affect points that don't have a drilldown specified, which can still be selected without modifier keys in the usual way.

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Mar 17, 2016

Collaborator

Okay, it's fixed now, please see http://jsfiddle.net/highcharts/x6v8Lpnz/2/.

What I did was to add the originalEvent to the drilldown event, so that you can cancel when a modifier key is pressed. In addition, in the demo I set a flag on the point that is drilling, so that you can cancel the select action from its event.

Collaborator

TorsteinHonsi commented Mar 17, 2016

Okay, it's fixed now, please see http://jsfiddle.net/highcharts/x6v8Lpnz/2/.

What I did was to add the originalEvent to the drilldown event, so that you can cancel when a modifier key is pressed. In addition, in the demo I set a flag on the point that is drilling, so that you can cancel the select action from its event.

@TorsteinHonsi TorsteinHonsi added Enhancement and removed Undecided labels Mar 17, 2016

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