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

Persistent and Multiple Clicking and Unclicking Behaviour #1852

Closed
chriddyp opened this issue Jul 4, 2017 · 35 comments

Comments

Projects
None yet
6 participants
@chriddyp
Copy link
Member

commented Jul 4, 2017

Very closely related to Persistent, stateful hover, click, and selection properties and Customized Click, Hover, and Selection Styles or Traces is the ability for users to select one or more points through clicking.

Under the hood, this might just be a new "select" mode (like "Box Select" or "Lasso Select") that allows users to select and de-select points through clicking. Once clicked, the style of the clicked points would remain persistent.

From a UI perspective, the user would be informed that points are clickable and unclickable through:

  • cursor: pointer
  • customized hover on clickable points like bigger points or an outline or a tooltip
  • A persistent or hover-activated "x" for unclicking points.

See this tableau dashboard for an example: https://public.tableau.com/profile/joe3537#!/vizhome/WorldBankIndicator/WorldData

image

Related to Persistent, stateful hover, click, and selection properties, users should be able to programatically specify which points are selected in order to initialize their graphs with pre-selected data.


cc @alexcjohnson @etpinard @monfera @jackparmer @charleyferrari

@chriddyp

This comment has been minimized.

Copy link
Member Author

commented Mar 1, 2018

A few requests for this from the Dash community (https://community.plot.ly/t/highlight-clicked-marker-in-scattermapbox-graph/8315, https://community.plot.ly/t/is-there-a-way-to-clear-clickdata/8708).

If you are using Dash in your company or organization and can fund the development of this feature, please reach out.

@alexcjohnson

This comment has been minimized.

Copy link
Contributor

commented Apr 30, 2018

A few notes after discussing this offline with @chriddyp

  • Using click to create or alter selections need not be mutually exclusive with lasso or box select; those are set by layout.dragmode because the property you're controlling is "what happens when the user drags on the plot." Here the question is "what happens when the user clicks on the plot" and it could be really nice to, for example, draw a lasso to start your selection but then click individual points to add or remove them from the selection.
  • I don't think we can make click-to-select the default behavior. A lot of existing apps rely on the click event to trigger something other than selection. Should we make a new top-level attribute layout.clickmode or does this need to be a trace-level attribute? The new behavior could be clickmode: 'select', and later we can add clickmode: 'hover' to trigger persistent hover labels, what do we call the existing behavior, clickmode: 'event'? Do we still emit plotly_click events in these other clickmode settings?
  • Multiple selections and deselection: my gut reaction is to have a regular click make a totally new selection, so deselection would be just a matter of clicking a blank spot on the plot (perhaps also when only one point is selected, clicking it again would deselect?), and use shift-click to add/remove points. Is this discoverable enough? Perhaps we add a notifier about it on your first click-to-select?
  • Does click-to-select exactly mirror hover data, like the click event data does right now, so that in "compare" mode a single click could add one point from each trace? Or do we do something like in the spikelines feature where after picking the hover data, we filter again to find the best single point and that's the one referenced for selection?
@etpinard

This comment has been minimized.

Copy link
Member

commented May 1, 2018

nice to, for example, draw a lasso to start your selection but then click individual points to add or remove them from the selection.

yeah that would be really nice.

I don't think we can make click-to-select the default behavior

agreed.

Should we make a new top-level attribute layout.clickmode or does this need to be a trace-level attribute?

I like a top-level layout.clickmode and perhaps down the road have a clickinfo attributes in traces with values 'none' and 'skip' similar to hoverinfo to control how a given trace contribute to the click data.

The new behavior could be clickmode: 'select', and later we can add clickmode: 'hover' to trigger persistent hover labels, what do we call the existing behavior, clickmode: 'event'? Do we still emit plotly_click events in these other clickmode settings?

I guess we could make clickmode a flaglist so that users could set e.g. clickmode: 'select+event'. That said, are we ever going to add support for persistent selection via hover? For example via layout.hovermode: 'select'? (see last comment for more on this thought).

Multiple selections and deselection: my gut reaction is to have a regular click make a totally new selection, so deselection would be just a matter of clicking a blank spot on the plot

Agreed.

and use shift-click to add/remove points. Is this discoverable enough? Perhaps we add a notifier about it on your first click-to-select?

👍 for shift clicks!

Does click-to-select exactly mirror hover data, like the click event data does right now, so that in "compare" mode a single click could add one point from each trace? Or do we do something like in the spikelines feature where after picking the hover data, we filter again to find the best single point and that's the one referenced for selection?

I guess this should be configurable in a similar way to hovermode: 'x' | 'y' | 'closest' which perhaps means we need two new click* attributes: one similar to dragmode which tells which interaction to perform and one similar to hovermode which tells which info the interaction should brings up.

@alexcjohnson

This comment has been minimized.

Copy link
Contributor

commented May 1, 2018

I guess we could make clickmode a flaglist so that users could set e.g. clickmode: 'select+event'.

I like that - so users who already implemented hacks to get a similar result (@cpsievert) or switching between select and some other click behavior could set clickmode: 'select' and not worry about their other event handlers conflicting with the new behavior.

That said, are we ever going to add support for persistent selection via hover? For example via layout.hovermode: 'select'?

Yeah possibly - for immediate cross-plot brushing like https://rd3.now.sh/ perhaps. Doesn't seem to me like the two conflict though, we could add that later and people might even want to use the two together, so you can persistently select some points then add the transient hover selection to it.

perhaps means we need two new click* attributes: one similar to dragmode which tells which interaction to perform and one similar to hovermode which tells which info the interaction should brings up.

Alternatively, don't implement a second attribute, have selection and event data always match hover data, but make hovermode default to 'closest' when you're using the new clickmode values.

I started describing how a second `click* attribute would look and it got really complicated (below), so ^^ is my vote, I think it's both clearer for users (it's obvious what exactly you're clicking on) and easier to implement!

  • clickdata: ('all'|'closest') perhaps? Where 'all' means "all the points showing hover data" and 'closest' is just the closest one (but I think hovermode: 'closest', clickdata: 'x' would be confusing, I don't think we allow that - so clickdata would be irrelevant if hovermode is already 'closest')? And this controls both the event data and the selection data? For backward compatibility it seems like with clickmode: 'event', clickdata should default to 'all', but for clickmode: 'select' I feel like clickdata should default to 'closest' AND we should find some way to indicate with the hover tooltip which point is the closest (thicker border? bold text?)
@etpinard

This comment has been minimized.

Copy link
Member

commented May 23, 2018

I started describing how a second `click* attribute would look and it got really complicated (below),

I had a feeling adding a second click attribute would made things confusion. Thanks for confirming that feeling I had. Much of the spikes behavior is tied down to hovermode as well, so this is not untraveled territory for our good friend hovermode. So yeah, a single clickmode sounds 👌 to me.

Maybe the problem with hovermode isn't what it does, but how it's named: something more general like interactionmode might be better.

but make hovermode default to 'closest' when you're using the new clickmode values.

Oof, that might be a little too magical here for my taste. How bad (for most applications) would clickmode: 'select' and hovermode: 'x' be?

@alexcjohnson alexcjohnson referenced this issue May 23, 2018

Open

v2.0.0 wishlist #420

0 of 15 tasks complete
@alexcjohnson

This comment has been minimized.

Copy link
Contributor

commented May 23, 2018

Oof, that might be a little too magical here for my taste. How bad (for most applications) would clickmode: 'select' and hovermode: 'x' be?

Per #420 (comment) we're thinking of changing the default hovermode to always be 'closest', so it's not too much of a stretch to say new features could force that behavior before v2. But I'd also be OK with just adding something to the docs for clickmode - like "clickmode: 'select' with hovermode: 'x' can be confusing, consider explicitly setting hovermode: 'closest' when using this feature"

@etpinard

This comment has been minimized.

Copy link
Member

commented May 23, 2018

we're thinking of changing the default hovermode to always be 'closest', so it's not too much of a stretch to say new features could force that behavior before v2.

Now that #420 (comment) is been added to our v2 checklist, 👍 for making clickmode: 'select' change the hovermode default. Note that we should make that clear in the clickmode and/or hovermode attribute description.

@rmoestl

This comment has been minimized.

Copy link
Contributor

commented Jun 15, 2018

This is just the documentation of a team internal discussion. It was about the questions if users should be able to deselect a data point that has already been selected by a box or lasso select.

TL;DR is to make sure that if a user deselects a data point in an existing lasso or box area, this point should stay deselected even if another box or lasso selection take place.

Here's a screen recording of the current misbehavior in current development state. Watch out for what happens when the last lasso select takes place.

plotlyjs-1852-deselect-by-click

Other options would have been

  1. to not allow to deselect a data point already selected through box or lasso
  2. to purge all box an lasso polygons when a use selects by click, but keep all previously selected points selected
@rmoestl

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2018

@alexcjohnson @etpinard I'd like to show the solution I came up with to cover the edge cases regarding the interplay of box-lasso-select and click-select and ask for your feedback before moving on.

(I published the current changes in my own fork, see below)

Current box-lasso-select approach
Before click-select, box-lasso-select was implemented as follows: when the user adds to a selection by holding the Shift key, cartesian/select.js passes the current and all previously drawn polygons to a trace's selectPoints method. The selectPoints method is supposed to do the following: go through each data point of the trace and re-evaluate if it is contained in any polygon. To summarize, the whole calcData was re-evaluated every time.

Edge case interplay box-lasso-select vs. click-select
When looking at the edge case in the previous comment, there could be the case that by click-deselecting a point already selected through a polygon before, a call of selectPoints would re-select this point again. There was no way to distinguish between a polygon drawn before and a freshly drawn polygon. Because when a user draws a new polygon over this click-deselected point, his intention is obviously to re-select the point.

Approaches
To solve the issue I was thinking through various approaches (with input from Alex):

  1. box-and-lasso-select not allowed to select a click-deselected point. Would have required to remember the ways a point was selected and deselected trace-internally. Plus would have required a way for selectPoints to differentiate between an existing polygon and a new own that is currently created throughout a drag operation.
  2. click-deselect in a polygon creates a little substract polygon, possibly hidden (HT Alex). Alex mentioned in a chat that this way multiple points at the same location would get selected, which is probably not the user's intention.
  3. stop evaluating all polygons and instead only evaluate the currently drawn polygon. Keep the previous polygon visually, but no longer reevaluate each calcData item against each previous polygon. The result of selectPoints would no longer overwrite the last selection state, but would update it. The only obstacle would be to recognize when a point was selected but deselected again within a single drag operation.

Rationale for approach 3
I was comparing the options based on maintainability and decided to went with option 3. To make this happen, I decoupled checking if a point is contained in a polygon from setting its selection state. That means that the selection interface of a trace changed from having a selectPoints method to have the following four functions:

  • getPointsIn(searchInfo, polygon), returns the point indices contained in the passed polygon
  • selectPoints(searchInfo, pointsIds), updates trace-internal state on calcData and produces an array of selection points
  • deselectPoints(searchInfo, pointsIds), the opposite of selectPoints
  • clearSelection, clears selection state entirely

The logic of whether a point is going to be selected or not has definitely moved from the trace modules towards the calling code in cartesian/select.js.

Please see the current changes in https://github.com/rmoestl/plotly.js/commits/1852-persistent-click. For your convenience I temporarily committed my testing mock named 1852_select-by-clicking.

@nicolaskruchten

This comment has been minimized.

Copy link
Member

commented Jun 21, 2018

I actually think that option 2 is nice, especially if coupled with the inverse: click-to-select creates a tiny non-subtract polygon, such that all data at a given location is selected/deselected.

@rmoestl

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2018

On the one hand, that'd be a nice feature. On the other hand, a user would not have the ability to select just a single data point. And users can already select all data at one location by drawing a little polygon over the respective area.

@nicolaskruchten

This comment has been minimized.

Copy link
Member

commented Jun 22, 2018

@alexcjohnson

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2018

I think approach 3, as you've done it, is the right one - there may be other ways to select points that DO differentiate multiple points at the same coordinates, for example on a SPLOM you select points in one pane where they have different coordinates, then you modify that selection in a pane where they overlay. Working only with polygons would not be able to handle this.

I can certainly see the appeal of having click-to-select pick all points at the current location instead of just the top one - while you're right that users can already do this with a tiny polygon, there's no way to select just an occluded point either way, so this seems like a limitation as much as a feature. But I think we should solve this as a separate issue in concert with a multi-overlapping-point hovermode, that perhaps makes hover labels like (4.2, 5.3) - 13 points so you know from the tooltip what your click is going to select.

@rmoestl

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2018

Yes, I think as long as we don't have multi-overlapping-point hovermode, it could be confusing for users to be presented with a hover label of one point and find multiple selected after a click.

Any feedback on the interface change I described, Alex?

@alexcjohnson

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2018

Any feedback on the interface change I described, Alex?

Totally agreed on breaking up the functionality that way, and getPointsIn looks great. I just wonder if we can reduce the footprint a bit for selectPoints, deselectPoints, and clearSelection, into one item, something like:

toggleSelected(searchInfo, selected, pointIds)
// @param {boolean} selected: are these points to be selected (true) or deselected (false)
// @param {array[integer]} pointIds: the points to modify - omit to modify all points in the trace.
//     ie clearSelection is toggleSelection(searchInfo, false)
@rmoestl

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2018

Yes, sure we can.

@etpinard

This comment has been minimized.

Copy link
Member

commented Jun 27, 2018

Referencing #720 the multi-overlapping-point hovermode issue.


Splitting the "which pts are in this polygon" logic from "tag these pts as selected" makes a ton of sense, and should improve performance too. So, 👍 from me 🎉

I agree with @alexcjohnson , we should aim for two trace _module methods, not four.

Now, before going any further, @rmoestl you should try your new internal API on a non-array-of-objects-calcdata trace module e.g. scattergl. I think things should work out, but yeah we should make sure not to assume too many things about gd.calcdata.

@rmoestl

This comment has been minimized.

Copy link
Contributor

commented Jun 28, 2018

Okay, I see scattergl is also controlled by cartesian plot. It'll be the next trace type I'm going to integrate.

@rmoestl

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2018

Status: I've adapted scattergl and histogram to implement the new selection interface. Had to tweak the calling code in cartesian/select.js due to inconsistencies in hoverdata (binNumber instead of pointNumber) and the way scattergl determines to render in selection-mode or non-selection-mode.

Question about box trace: working on box trace type, I found out that there's no way to detect if an element in gd._hoverdata is indeed a point or a box. There can be cases when no point is hovered but only boxes. Like this:
image
Even though an eventdata element represents a box, it gets a pointNumber property. But it is not the index of a data point but something different. There's no other property that would allow to differentiate between box or point.

Would it be okay to introduce a flag called notSelectable: true for hoverdata elements? Only if notSelectable is set to true, that element won't be selectable.

@alexcjohnson

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2018

Tricky question... I’m not sure “selectable” is going to serve us well, or at the very least it’s likely to change over time as we figure out meaningful ways to select more kinds of objects. Can we just annotate each item as to what it represents, and have the module decide what to do with it?

Re: pointNumber vs binNumber - the idea is pointNumber should refer to an index in the data arrays, and if the item doesn’t refer to one index it should use a different name (such as binNumber) but if applicable it may include an array pointNumbers. Sounds like you’ve found some places we don’t follow this convention, perhaps now is the time to fix these!

@rmoestl

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2018

Not sure what you mean by

and have the module decide what to do with it?

Let the trace module detect which point has been clicked (similarly to getPointsIn capable to find out which points are in a passed polygon) and let the base plot (e.g. cartesian) call that?

@etpinard

This comment has been minimized.

Copy link
Member

commented Jul 4, 2018

Sounds like you’ve found some places we don’t follow this convention, perhaps now is the time to fix these!

Absolutely, hovering on a box should not include pointNumber in the hoverdata. Same for violin traces. Fixing this would IMHO trigger a breaking change though.

Can we just annotate each item as to what it represents, and have the module decide what to do with it

I think what @alexcjohnson means is (1) add below this line pointData2.hoverOnBox = true and then (2) in getPointsIn filter out point data items with hoverOnBox: true.

@rmoestl

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2018

I think what @alexcjohnson means is (1) add below this line pointData2.hoverOnBox = true and then (2) in getPointsIn filter out point data items with hoverOnBox: true.

Mhm, I see. Until now my approach was to let the base plot (i.e. cartesian) figure out the clicked point based on the assumption that hoverdata is relatively homogeneous across traces.

The proposed solution suggests to make detecting which point has been clicked a concern of the traces themselves. What about having a hybrid approach: for traces that expose a function with a certain name, e.g. getClickedPtsFor(searchInfo, hoverData), delegate the detection mechanism to the trace, else do it in cartisian (and the other base plots respectively). A similar approach is already being done in makeEventData

out = trace._module.eventData(out, pt, trace, cd, pointNumber);
and seems to me as a good trade-off between keeping things DRY and be flexible on the traces side of things.

@etpinard

This comment has been minimized.

Copy link
Member

commented Jul 4, 2018

Apart from boxes and violins, traces with fill: 'toself' and hoveron: 'fills' also feature "special" event data.

Now,

on the assumption that hoverdata is relatively homogeneous across traces.

this would be very nice, unfortunately we're not there yet. But I agree with you: in theory, we should be able to figure out the clicked point based on gd._hoverdata only w/o having to resort to _module.

So, I don't think we should add another method on the module objects (e.g. getClickedPtsFor), adding a few lines of (hopefully temporary) logic in cartesian/select.js should suffice.

@rmoestl

This comment has been minimized.

Copy link
Contributor

commented Jul 5, 2018

Okay, sounds reasonable to me as well. We're putting in temporary logic in one place and weed out parts of it as soon as hoverdata has been harmonized for a special-case trace type. Eventually the detection logic will become clean.

rmoestl added a commit that referenced this issue Jul 17, 2018

Introduce change of trace selection interface [1852]
- Reason: new approach is needed to support proper
  interplay of polygon and click select. See
  #1852
  for a discussion.
- New approach separates the two concerns
  'what was selected' and 'set selected state of
  data points', that were previously handled
  by `selectPoints`, into multiple functions.
- Also the polygons now longer implicitly
  carry selection state in cartesian/select.js
  but are only visual cues anymore.
- Introduce a new Lib function for calculating
  the set difference of two arrays.
@rmoestl

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2018

Is it correct that one needs to hold both Shift and Alt to subtract from an existing (box or lasso) selection? If true, it seems odd that

drag([[219, 143], [219, 183]], {altKey: true});
is working. On current master, when testing manually I can't subtract from a selection just holding the Alt key.
CC @etpinard

@etpinard

This comment has been minimized.

Copy link
Member

commented Jul 18, 2018

Is it correct that one needs to hold both Shift and Alt to subtract from an existing (box or lasso) selection?

That's correct.

You're right this test looks odd. I think if you just Alt+click the browser doesn't register clicks.

Does that test fail if we set {altKey: true, shiftKey: true} instead?

@rmoestl

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2018

Nope, test does not fail with {altKey: true, shiftKey: true}.
With my changes made to support select-on-click though, test fails with {altKey: true} and passes with {altKey: true, shiftKey: true}.

rmoestl added a commit that referenced this issue Jul 18, 2018

Introduce change of trace selection interface [1852]
- Reason: new approach is needed to support proper
  interplay of polygon and click select. See
  #1852
  for a discussion.
- New approach separates the two concerns
  'what was selected' and 'set selected state of
  data points', that were previously handled
  by `selectPoints`, into multiple functions.
- Also the polygons now longer implicitly
  carry selection state in cartesian/select.js
  but are only visual cues anymore.
- Introduce a new Lib function for calculating
  the set difference of two arrays.
@etpinard

This comment has been minimized.

Copy link
Member

commented Jul 18, 2018

With my changes made to support select-on-click though, test fails with {altKey: true} and passes with {altKey: true, shiftKey: true}.

That sounds fine to me!

@rmoestl

This comment has been minimized.

Copy link
Contributor

commented Jul 19, 2018

Is box/lasso select supposed to be supported for scattergeo? (Sorry for asking, wasn't able to find anything about it at https://plot.ly/javascript/)

I was able to fix failing geo_point-selection image test (mock json has selectedpoints set) but when I use box or lasso drag mode the selection doesn't change even when I'm on current master w/o my changes.

@etpinard

This comment has been minimized.

Copy link
Member

commented Jul 19, 2018

Is box/lasso select supposed to be supported for scattergeo?

Looks like they're broken at the moment.

@etpinard

This comment has been minimized.

Copy link
Member

commented Jul 19, 2018

... on master that is.

@etpinard

This comment has been minimized.

Copy link
Member

commented Jul 19, 2018

I'll come up with a fix ASAP.

@etpinard

This comment has been minimized.

Copy link
Member

commented Sep 12, 2018

Looking at this issue again, I think @rmoestl's remarkable #2944 effectively closed this thing.

@chriddyp ok to close this issue?

@etpinard

This comment has been minimized.

Copy link
Member

commented Sep 14, 2018

Closed in #2944

@etpinard etpinard closed this Sep 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.