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

Add support for modifiers and allow to auto-activate WheelZoomTool #13815

Merged
merged 7 commits into from
Apr 10, 2024

Conversation

mattpap
Copy link
Contributor

@mattpap mattpap commented Apr 8, 2024

Originally I started working on scroll support in BoxAnnotation per issue #13646, but I realized that I need to resolve the more generic issue of conditional handling of scroll events first, so that bokehjs doesn't interfere by with page scrolling, while allowing some way of easy access to scrolling functionality (like wheel zoom or wheel pan tools provide).

Thus this PR:

  • redesigns how tool event handlers are invoked and allows to indicated from such handlers if an event was handled or not (this has the added benefit of not using expensive signaling infrastructure to dispatch UI events to tool handlers)
  • adds support for modifiers to WheelZoomTool and WheelPanTool
  • adds support to Toolbar for auto-activation of such tools when modifiers are set
  • adds support for a visual indicator that a modifier is required to perform tool's function (e.g. an alert or a plot overlay)

This example shows WheelZoomTool configured with modifiers = dict(ctrl=True), first being scrolled without the modifier and then with:

Screencast.from.09.04.2024.02.22.41.webm

fixes #10439

@jbednar
Copy link
Contributor

jbednar commented Apr 8, 2024

Cool! Can you describe the details of how it "redesigns how tool event handlers are invoked"?

@mattpap
Copy link
Contributor Author

mattpap commented Apr 8, 2024

Cool! Can you describe the details of how it "redesigns how tool event handlers are invoked"?

The previous approach using signals didn't allow for returning a value from handlers (like WheelZoomTool._scroll), so this PR changes that, replacing signals with direct function calls.

bokehjs/src/lib/models/plots/plot_canvas.ts Show resolved Hide resolved
src/bokeh/models/tools.py Show resolved Hide resolved
@mattpap mattpap force-pushed the mattpap/10439_WheelZoomTool_auto_active branch from f20a3f7 to d5ccbf9 Compare April 10, 2024 08:44
Copy link

codecov bot commented Apr 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.64%. Comparing base (e61219b) to head (d5ccbf9).
Report is 1 commits behind head on branch-3.5.

Additional details and impacted files
@@              Coverage Diff               @@
##           branch-3.5   #13815      +/-   ##
==============================================
+ Coverage       91.57%   92.64%   +1.06%     
==============================================
  Files             326      326              
  Lines           20737    20751      +14     
==============================================
+ Hits            18990    19224     +234     
+ Misses           1747     1527     -220     

@mattpap mattpap merged commit 10ff334 into branch-3.5 Apr 10, 2024
27 checks passed
@mattpap mattpap deleted the mattpap/10439_WheelZoomTool_auto_active branch April 10, 2024 09:28
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.

Activate wheel zoom by default with BokehJS
3 participants