Widget max_extent: cap a widget's size during the drag#40
Merged
Conversation
Adds max_extent= to RangeWidget (span width, data units) and RectangleWidget (scalar caps both axes, or a (max_w, max_h) pair), plumbed through add_range_widget / add_rectangle_widget. Default None leaves widgets unbounded. The cap is applied IN the drag handler, not after it: the dragged edge/corner pins and the opposite one stays put. Clamping after the fact moves an edge the user is not holding, which reads as the selection jumping under the cursor — the bug this replaces. Also fixes the band-style RangeWidget being hard to grab by its body. Each edge claimed a fixed +-12 px zone, so a band under ~24 px wide on screen (routine when zoomed out, or when its span is capped) had NO grabbable middle: aiming at the body to translate it caught an edge and resized instead. Each edge now takes at most a third of the band width. Adds window._aplWidgetGeom, a canvas-space geometry readback for Playwright in the same spirit as window._aplTiming. Without it a test cannot locate a 9 px handle on a 2-D panel (the image->canvas mapping depends on the zoom/extent state), so a hard-coded corner silently grabs the body and the resize assertion proves nothing. 16 tests; verified non-vacuous by neutralising the JS clamps and the hit-test fix, which fails 3 of them.
add_rectangle_widget's default size is half the image, so a widget created with
max_extent appeared huge and then snapped down to the cap the first time it was
used — which reads as a bug ("the ROI is always huge and then it clamps down").
The default now honours max_extent per axis. An explicit w/h still wins, and an
uncapped widget is unchanged.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #40 +/- ##
==========================================
+ Coverage 90.38% 90.41% +0.03%
==========================================
Files 39 39
Lines 3994 4008 +14
==========================================
+ Hits 3610 3624 +14
Misses 384 384 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
max_extent=onRangeWidgetandRectangleWidget(plus the matchingadd_*_widgetfactories): a size cap enforced while dragging, so the widget physically stops instead of being clamped after the fact.RangeWidget(max_extent=...)— span width in data unitsRectangleWidget(max_extent=...)— scalar caps both axes, or a(max_w, max_h)pairNoneleaves widgets unbounded; nothing existing changesWhy the cap belongs in the drag
Clamping afterwards moves an edge the user isn't holding (whichever endpoint is numerically smaller), which reads as the selection jumping out from under the cursor. Enforcing it in the drag handler pins the dragged edge/corner and leaves the opposite one alone.
Motivating case: an integrating selector whose span is a number of frames to read, where each extra unit costs real I/O.
Two bug fixes that came out of the same investigation
Narrow bands were impossible to grab by the body. Each edge claimed a fixed ±12 px zone, so a band under ~24 px wide on screen (routine when zoomed out, or when its span is capped) had no grabbable middle — aiming at the body to translate it caught an edge and resized instead. Each edge now takes at most a third of the band width. Wide bands are unaffected.
A capped rectangle was born at half the image, then snapped down to the cap on first use — visibly "huge, then collapses". The default size now honours
max_extentper axis. An explicitw/hstill wins.Testing
21 tests in
test_widget_max_extent.py: Python API, state round-trip, and real browser drags via Playwright.Verified non-vacuous by neutralising the JS clamps and the hit-test fix — 3 tests fail, then pass again on restore.
Full suite: 1781 passed, 58 skipped.
Note on the test hook
window._aplWidgetGeompublishes canvas-space widget geometry, in the same spirit as the existingwindow._aplTiming. Without it a Playwright test cannot locate a 9 px handle on a 2-D panel — the image→canvas mapping depends on zoom/extent state, so a hard-coded corner silently grabs the body and the resize assertion proves nothing. That happened twice while writing these tests.Review notes
max_extentis not enforced on programmaticw.x1 = ...writes — only on drags. Callers that set geometry directly are trusted, matching how the other widget attributes behave.