-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Add support for resize and drag handles to BoxAnnotation
#13906
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 resize and drag handles to BoxAnnotation
#13906
Conversation
215d609 to
20bf4f8
Compare
20bf4f8 to
4c117ff
Compare
|
This is tentatively ready. The base functionality is fine, up to known issues with maintaining interactive focus. I don't like the mechanism for overriding styling of handles and the lack of ability to change at least handle dimensions. |
249fea1 to
71b90b1
Compare
71b90b1 to
8e47d41
Compare
hoxbro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have left some comments in the code. Here are a couple of observations:
-
The hit for the corners needs an extra hit radius than x and y only. http://localhost:5777/integration/run?k=handles
screenrecord-2024-06-19_10.39.38.mp4
-
I think it also makes sense to have a
corneroption forresizable.
|
It looks great. The only issue I have noticed so far is that when clicking on a side and dragging, if the cursor moves closer to another handle, the hover styling gets activated on the non-activated handle, even when the original side handle is still active. This just breaks the expectation a bit of the styling being related to the activation, but this is very minor. Codeimport numpy as np
from bokeh.layouts import column
from bokeh.models import ColumnDataSource, RangeTool
from bokeh.plotting import figure, show
from bokeh.io import output_notebook
output_notebook()
n_points = 3000
x_values = np.linspace(0, 100, n_points)
y_values = np.random.randn(n_points).cumsum()
source = ColumnDataSource(data=dict(x=x_values, y=y_values))
detailed_plot = figure(
width=800,
height=300,
tools=["xpan", "xzoom_in", "xzoom_out", "reset", "wheel_zoom"],
toolbar_location="above",
active_scroll="wheel_zoom",
background_fill_color="#efefef",
x_range=(22, 30),
)
detailed_plot.line("x", "y", source=source)
minimap = figure(
width=detailed_plot.width,
height=150,
tools="",
toolbar_location=None,
background_fill_color=detailed_plot.background_fill_color,
title="Drag the middle or edges of the selection box below, or double click to start a new box"
)
minimap.line("x", "y", source=source)
minimap.x_range.range_padding = 0
minimap.ygrid.grid_line_color = None
range_tool = RangeTool(x_range=detailed_plot.x_range, y_range=detailed_plot.y_range, start_gesture='tap')
range_tool.overlay.fill_color = "darkblue"
range_tool.overlay.fill_alpha = 0.3
range_tool.overlay.inverted = True
range_tool.overlay.use_handles = True
range_tool.overlay.handles["all"].hover_fill_color = "orange"
range_tool.overlay.handles["all"].hover_fill_alpha = 0.9
range_tool.overlay.handles["all"].hover_line_alpha = 0
range_tool.overlay.handles["all"].fill_alpha = 0.1
range_tool.overlay.handles["all"].line_alpha = 0.1
minimap.add_tools(range_tool)
show(column(detailed_plot, minimap))video2110899201.mp4 |
2d77c16 to
6fdc791
Compare
6fdc791 to
b0f8c17
Compare
This is a "known" problem with UI event management, quite unrelated to this PR (it's actually mentioned in an earlier comment). Please report this as a bug, I will fix this next time I'm working on UI events. |
Corners will be present when a box is resizable in both dimensions. I presume you would want to have corners also for a single dimension resizing? If so, then I'm not convinced. However, if we want this, then the implementation would have to use left/right/top/bottom handles instead of corners, depending on the configuration. |
This is a variant of the problem mentioned by @droumis. You don't want to have the box hovered when the cursor is between handles, but outside of the box. What you want is hover over handles triggering hover over the box. That requires changes to UI event handling that is outside the scope of this PR. |
I suggested having an option,
Please open an issue then, as you are the best person to specify the changes needed. |
|
Just to note, I don't intend to implement any new features in this PR. Unless there are any other things to address and/or fix in this PR, can we proceed with this PR and get to releasing bokeh 3.5.0.rc1? |
* Add support for resize handles to BoxAnnotation * Add support for view stacking to UI event handling * Allow CompositeRenderer to paint its own child renderers * Add support for intersection kinds * Allow to configure appearance of interaction handles * Add interaction/tools/box_select_handles example * Add release notes * Rename BoxVisuals to AreaVisuals * Make sure computed_renderers are defined * Add visual integration tests * Resolve type/lint errors related to partial objects * Allow to set attrs in HasProps.clone() * Make InteractionHandle's kind viable for And() * Add more visual integration tests * Update object/structure, unit and cross tests * Improve defaults testing of nested structures * Replace InteractionHandles struct with a model * Compare keys as sets in defaults' tests
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |

This PR adds support for resize and drag handles to
BoxAnnotationbehindhandlesproperty. By default the original behavior without handles is preserved. For this to work this PR expands support forCompositeRendererand adds support for renderer stacking to UI event handling of interactive renderers, so that e.g. (pointer) move events can be handled by multiple renderers. Previously on the top-level render was allowed to be interactive.Screencast.from.22.05.2024.16.53.24.webm
The noticeable flickering when interacting with a renderer at its boundary is a pre-existing problem, which solving I may postpone for a further PR.
addresses point 1 from #13646 (comment)