refactor: converge Maestro gesture handling#624
Conversation
thymikee
left a comment
There was a problem hiding this comment.
Review — converge Maestro gesture handling
I traced the directional endpoints through the new shared planner and the Maestro-visible geometry is preserved, which is the thing that matters for compat:
buildSwipeGesturePlan('up')→buildScrollGesturePlan('down', amount 0.6)→ start(centerX, centerY+half), end(centerX, centerY−half). On an 800px frame that's y 640→160 — identical to the oldpoint(50,80)→point(50,20).left/rightline up the same way (320↔80 on a 400px frame), and the Android horizontal content-lane override stays at 65%. So the convergence doesn't move finger paths. Good.pointFromPercentwithout a margin keepstapPointPercentunclamped (the 125% / −10% test), preserving the existing off-screen behavior.
Two cleanups, both minor:
-
clampGestureCoordinate(value, minY, maxY + minY)is hard to read. You're passingmaxY + minYas thesizearg purely so thatsize − marginPx === maxY. It's correct but a reader will misparse it as a bug. A smallclampToRange(value, min, max)helper (or even a comment) at theswipeCoordinatesFromTargetcall sites would be clearer than overloading the margin-based clamp. -
clampGestureCoordinateno longer rounds (the oldclampCoordinatedidMath.round). It's safe today because every caller feeds pre-rounded integers (pointInsideRectcenter +Math.round'dswipeDistance, andpointFromPercentrounds before clamping). But the contract is now "integer in / integer out, fractional in / fractional out," and a future caller passing a fractional value would leak a non-integer coordinate toadb input swipe. Either round insideclampGestureCoordinateor document the integer-input expectation.
Neither blocks merge — the geometry is the important part and it checks out.
Generated by Claude Code
|
Summary
Reuse shared gesture geometry for Maestro directional and percentage swipes, including percent point projection and viewport-based finger swipe planning.
Preserve Android horizontal Maestro content-lane behavior and keep target-relative swipe.label/from distance policy local to Maestro.
Touched files: 6.
Known gaps: target-relative Maestro swipes still keep Maestro-specific element-rect distance semantics because they intentionally differ from viewport scroll planning.
Validation
Focused Maestro/core gesture unit tests passed for runtime swipes, target swipes, percentage taps, and shared scroll/swipe planning.
Provider-backed Android Maestro replay guard passed for fresh selector snapshots and horizontal content-lane swipes.
Typecheck and formatting passed; unrelated formatter churn was restored.