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

Tap tool default mode should select+unselect, but not append #13831

Open
bryevdv opened this issue Apr 17, 2024 · 6 comments
Open

Tap tool default mode should select+unselect, but not append #13831

bryevdv opened this issue Apr 17, 2024 · 6 comments

Comments

@bryevdv
Copy link
Member

bryevdv commented Apr 17, 2024

Original ref: https://discourse.bokeh.org/t/taptool-on-a-pie-chart-reopens-all-previous-taps/11448/6

In Bokeh 3.4.x the tap tool seems to be in permanent "append" mode, i.e. it always does what I would only expect to see if shift was added to a tap.

This can be seen in the color_scatter example in in the gallery. Here is the result of just randomly clicking many places with no shift or other modifier added:

Screenshot 2024-04-17 at 10 35 46

This does not occur with the 3.3.x version of the gallery example -- each new tap replaces the previous selection.

cc @bokeh/dev I have triaged discussion for now but I believe this is a bug/regression (and merits a 3.4.2 release).

@bryevdv bryevdv changed the title Tap Tap tool permanently in append mode Apr 17, 2024
@mattpap
Copy link
Contributor

mattpap commented Apr 18, 2024

Since bokeh 3.4 the default for tap tool is xor mode. It may appear to look like the tool is in append mode, but in append mode the user can only add selected data points, but not remove them, which the main benefit of xor mode. We switched to xor default, because users wanted to be able to deselect points and especially toggle data points.

The case from discourse the prompted this discussion silently assumes replace mode, which now requires explicit specification. The best way, that works with all modes, would be to keep track of selected data points and perform actions only for newly selected data points. Currently there is no built-in functionality for this.

@bryevdv
Copy link
Member Author

bryevdv commented Apr 18, 2024

cc @bokeh/dev I think this change should be re-visited. It certainly makes sense to offer this mode for some niche cases, but I don't think it makes sense as the default (and it clearly defies some user's expectations as well as seen on the discourse). This is especially jarring in plots like pie or bar charts where there are only a handful of elements to select. This also puts one tool on different footing than all other tools which is not good for simple documentation. Many users have used tap selection as part of a larger UI, e.g. to trigger some other drill down or other UI update and this change breaks all of those usages.

@bryevdv
Copy link
Member Author

bryevdv commented Apr 18, 2024

FWIW I would be ok with this default behavior:

  • tapping new points replaces the existing selection (previous behavior)
  • tapping an already selected point unselects it (new behavior)

i.e. it is very specifically the "additive" nature of this xor-mode, where tapping a new point adds to an existing selection, that is the problem

@philippjfr
Copy link
Contributor

I agree with that proposal. Let's allow deselect but not adding by default.

@mattpap
Copy link
Contributor

mattpap commented Apr 18, 2024

Do we want this behavior to be implemented in place of replace mode (just for tap tool) or should I introduce a new mode?

@bryevdv
Copy link
Member Author

bryevdv commented Apr 18, 2024

I actually don't know what all the current modes are to say, where are they enumerated/described?

Well, I guess they are listed here

SelectionMode = Enumeration(replace, append, intersect, subtract, xor)

I guess it's a little weird, I don't think one set of selection modes really neatly covers all the tools in the same way. i.e the re-select to un-select that can make sense for a tap is nearly impossible to imagine being useful on a lasso tool. I think I lean towards: new mode, e.g. replace-foo with that being the default for tap. That's in the interest of not having "replace" mean multiple things, but also being able to say "a replace mode" is the default for all tools, which is less awkward to explain. But I am interested in others' thoughts.

@bryevdv bryevdv added this to the 3.5 milestone Apr 19, 2024
@bryevdv bryevdv changed the title Tap tool permanently in append mode Tap tool default mode should select+unselect, but not append Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants