From dc35aa2e36395e6339cf8885583840460548b652 Mon Sep 17 00:00:00 2001 From: Greg Pomerantz Date: Tue, 18 Aug 2026 08:08:51 -0400 Subject: [PATCH] Redesign selection handles (teardrop + 48dp grab); place menu above selection - Handles are now teardrops (stem + filled circle) like the native Android selector: 20dp circle normally, 28dp while dragging. - The grab region is a 48dp box around the circle centre, independent of the visual size; the old 16dp target was not grabbable by finger. - The copy/cut/paste menu is placed ABOVE the selected line (native behaviour), flipping below only when there is no room above. The menu is drawn last (top of the z-order) and Gio routes a touch to the topmost op whose clip contains it, so a below-placed menu covered the handles' grab boxes and silently stole every handle-drag press. - Regressions: menu above-placement arithmetic, first-line flip-below, and the existing tracking/clamp tests updated for the new policy. - On-device verified: handle drags work with the menu up (previously dead lower grab region), first-line handles still grabbable from the uncovered top strip, menu taps and tap-to-clear unchanged. - doc/development_plan.md section 15 records the z-order/placement contract. --- doc/development_plan.md | 28 ++++++++ internal/editor/selection_menu_track_test.go | 46 +++++++++++- internal/editor/state.go | 29 +++++--- internal/ui/render.go | 75 +++++++++++++++----- 4 files changed, 150 insertions(+), 28 deletions(-) diff --git a/doc/development_plan.md b/doc/development_plan.md index 5a3935e..063789e 100644 --- a/doc/development_plan.md +++ b/doc/development_plan.md @@ -906,3 +906,31 @@ status bar and clickable; bottom bar flush with the keyboard top (view spans 128..1517 with the IME shown); BACK dismisses the keyboard and re-tap re-raises it without a re-show loop; typing saves; the selection menu tracks small scrolls and hides when the word leaves the viewport. + +## 15. Selection handles: teardrop shape, grab box, and menu placement (2026-08-18) + +**Handles.** The selection/caret handles are teardrops (a stem from the +caret point to a filled circle, like the native Android selector). The +circle is 20dp normal and 28dp while its drag is active. The GRAB region +is deliberately much larger than the visual: a 48dp box centred on the +circle centre. The visual size and the touch target are independent — +small visuals with 48dp targets is the native behaviour, and the earlier +16dp target was not grabbable by finger. + +**Menu placement contract: above first, below on overflow.** The +copy/cut/paste menu is placed ABOVE the selected line (like the native +Android selection toolbar), flipping below only when there is no room +above the window, and clamped to the window edges. This is not a matter +of taste: the handles hang off the line's BOTTOM edge, and the menu is +drawn last, so it sits on top of the z-order — Gio routes a touch to the +topmost op whose clip contains it. A menu placed below the line therefore +covers the handles' grab boxes and silently steals every touch meant for +a handle drag (a drag can only grab once it has received the PRESS; a +press captured by the menu's click never reaches the drag, no matter how +far the finger then moves). Verified on device: with the menu up, drags +from the previously-dead lower grab region work once the menu is above; +in the first-line case (menu flipped below) the handles are still +grabbable from the uncovered top strip of their boxes. + +Regressions: `internal/editor/selection_menu_track_test.go` (tracking + +above-placement arithmetic + first-line flip). diff --git a/internal/editor/selection_menu_track_test.go b/internal/editor/selection_menu_track_test.go index 3e65cf3..d119c21 100644 --- a/internal/editor/selection_menu_track_test.go +++ b/internal/editor/selection_menu_track_test.go @@ -69,11 +69,11 @@ func TestSelectionMenu_FollowsTextAcrossScroll(t *testing.T) { t.Fatal("menu not shown for a visible selection") } y0 := e.MenuRect.Y - // The menu sits below the anchor's line: lineTop(line 5) = reg.Y + 5*lh + // The menu sits ABOVE the anchor's line: lineTop(line 5) = reg.Y + 5*lh // (reg.Y = 10+52, the anchor's visual line index is 5). - wantY0 := ui.Dp(62) + ui.Dp(5*lh) + EffectiveLineHeight() + 8 + wantY0 := ui.Dp(62) + ui.Dp(5*lh) - menuH - 8 if !dpeq(y0, wantY0) { - t.Fatalf("initial menu Y = %v, want %v (below the anchor line)", y0, wantY0) + t.Fatalf("initial menu Y = %v, want %v (above the anchor line)", y0, wantY0) } // Scroll down by exactly 2 lines and lay out again. @@ -117,3 +117,43 @@ func TestSelectionMenu_ClampsWhenTextLeavesView(t *testing.T) { t.Fatalf("menu Y = %v, want 8 (pinned to the window top when the text is above the viewport)", got) } } + +// TestSelectionMenu_FlipsBelowOnFirstLine checks the edge where there is no +// room above the anchor (selection on the first line): the menu must flip +// below the line rather than clamping to the window top and sitting over the +// anchor's own text. +func TestSelectionMenu_FlipsBelowOnFirstLine(t *testing.T) { + lh := menuTrackState(t) + + TheState.Editor.Buffer = "aa\nbb\n" + SetSelection(0, 2) // "aa" on line 0 + + gl := ui.GlyphLayout{LineHeight: EffectiveLineHeight()} + const ascent float64 = 14 + lines := 2 + for i := 0; i < lines; i++ { + for c := 0; c < 2; c++ { + gl.ByteOffsets = append(gl.ByteOffsets, i*3+c) + gl.X = append(gl.X, ui.Dp(10+10*c)) + gl.Y = append(gl.Y, ui.Dp(ascent+float64(i)*lh)) + gl.Advance = append(gl.Advance, 10) + } + } + TheState.Editor.GlyphLayout = gl + TheState.LastLineY = ui.Dp(float64(lines) * lh) + TheState.Editor.IMEWindowStartByte = 0 + TheState.ScrollOffset = 0 + + EditorLayout(ui.Dp(1000), ui.Dp(2000), false) + showSelectionMenu() + e := &TheState.Editor + if !e.MenuVisible { + t.Fatal("menu not shown") + } + // Line 0 top = reg.Y = 62; above would be 62-52-8 = 2 < 8, so the menu + // flips below: 62 + lh + 8. + wantY := ui.Dp(62) + EffectiveLineHeight() + 8 + if !dpeq(e.MenuRect.Y, wantY) { + t.Fatalf("menu Y = %v, want %v (flipped below the first line)", e.MenuRect.Y, wantY) + } +} diff --git a/internal/editor/state.go b/internal/editor/state.go index 60f0f23..8054fc8 100644 --- a/internal/editor/state.go +++ b/internal/editor/state.go @@ -763,8 +763,12 @@ func hideSelectionMenu() { e.MenuRect = ui.Region{} } -// positionSelectionMenu places the copy/cut/paste menu below the visual line -// containing the anchor (selection end or caret), clamped to the window. It +// positionSelectionMenu places the copy/cut/paste menu above the visual line +// containing the anchor (selection end or caret) — like the native Android +// selection toolbar — falling back to below when there is no room above. +// Above matters: the selection handles hang off the line's bottom edge and +// their 48dp grab boxes would sit under a below-placed menu; the menu is +// drawn last (top of the z-order) and would steal those touches. It // is called from showSelectionMenu and from EditorLayout on every frame // while the menu is visible, so the menu tracks the selected text when the // user scrolls: the menu is anchored to the TEXT (an absolute buffer offset @@ -797,9 +801,18 @@ func positionSelectionMenu(e *EditorState) bool { if winW > 0 && mx+float64(menuW) > winW-8 { mx = winW - float64(menuW) - 8 } - my := lineTop + float64(EffectiveLineHeight()) + 8 // below the line - if winH > 0 && my+float64(menuH) > winH-8 { - my = lineTop - float64(menuH) - 8 // flip above the line + // Prefer ABOVE the line, like the native Android selection toolbar. + // Placing it below would sit over the selection handles' grab region + // (the handles hang off the line's bottom edge), and the menu is drawn + // last (top of the z-order) so it would steal the touches meant for the + // handles. Above keeps the handles fully grabbable. Flip below only when + // there is no room above. + my := lineTop - float64(menuH) - 8 + if my < 8 { + my = lineTop + float64(EffectiveLineHeight()) + 8 // flip below the line + if winH > 0 && my+float64(menuH) > winH-8 { + my = winH - float64(menuH) - 8 // clamp to window bottom + } } if my < 8 { my = 8 @@ -808,9 +821,9 @@ func positionSelectionMenu(e *EditorState) bool { return true } -// showSelectionMenu recomputes the menu items and positions the menu below -// the line containing the selection end (or caret). Copy and Cut are -// offered only while a selection is active; Paste always. +// showSelectionMenu recomputes the menu items and positions the menu above +// the line containing the selection end (or caret), falling back to below. +// Copy and Cut are offered only while a selection is active; Paste always. func showSelectionMenu() { e := &TheState.Editor if e.TooLarge || len(e.GlyphLayout.ByteOffsets) == 0 { diff --git a/internal/ui/render.go b/internal/ui/render.go index 1794e5e..7d0048c 100644 --- a/internal/ui/render.go +++ b/internal/ui/render.go @@ -120,18 +120,23 @@ type Renderer struct { selDragsOn [4]bool selDragEmitting [4]bool selDragHandler func(any) + // selDraggingWhich is which handle drag (0-3) is currently in flight + // (-1 = none), set by CheckGestures and read by drawWrappedText to + // enlarge the grabbed handle, as the framework does while dragging. + selDraggingWhich int } // New creates a new Renderer. func New(th Theme, shp *text.Shaper) *Renderer { r := &Renderer{ - theme: th, - shp: shp, - icons: make(map[string]image.Image), - clicks: make(map[string]*clickReg), - Keys: make(map[string]keyReg), - scrolls: make(map[string]scrollReg), - gioEditors: make(map[string]*widget.Editor), + theme: th, + shp: shp, + icons: make(map[string]image.Image), + clicks: make(map[string]*clickReg), + Keys: make(map[string]keyReg), + scrolls: make(map[string]scrollReg), + gioEditors: make(map[string]*widget.Editor), + selDraggingWhich: -1, } r.loadIcons() return r @@ -350,6 +355,7 @@ func (r *Renderer) CheckGestures(q input.Source, m unit.Metric) []InputEvent { switch e.Kind { case pointer.Drag: r.selDragEmitting[which] = true + r.selDraggingWhich = which events = append(events, InputEvent{ Handler: r.selDragHandler, Data: SelectionDragEvent{Which: which, X: r.toDp(Px(e.Position.X)), Y: r.toDp(Px(e.Position.Y))}, @@ -362,6 +368,9 @@ func (r *Renderer) CheckGestures(q input.Source, m unit.Metric) []InputEvent { }) } r.selDragEmitting[which] = false + if r.selDraggingWhich == which { + r.selDraggingWhich = -1 + } } } } @@ -820,10 +829,20 @@ func (r *Renderer) drawWrappedText(gtx layout.Context, str string, reg Region, w n := len(layout.X) - 1 return reg.X + layout.X[n] + layout.Advance[n], reg.Y - scrollOffset + layout.Y[n] - ascent } + // The visual handle (drawHandle) is a ~20dp teardrop centred at + // (hx, hy+lineH+handleRadius); the GRAB region is a 48dp box around + // that centre — Android's own handles are small but their touch + // targets are not (framework slop + 48dp minimum touch target), and + // 16dp was far too small to grab reliably by finger. + const handleRadius = Dp(10) registerDrag := func(d *gesture.Drag, hx, hy Dp) { + cy := hy + lineH + handleRadius + const grab = Dp(24) // 48dp box + minx, miny := int(r.toPx(hx-grab)), int(r.toPx(cy-grab)) + maxx, maxy := int(r.toPx(hx+grab)), int(r.toPx(cy+grab)) hc := clip.Rect{ - Min: image.Point{X: int(r.toPx(hx - 8)), Y: int(r.toPx(hy - 6))}, - Max: image.Point{X: int(r.toPx(hx + 8)), Y: int(r.toPx(hy + lineH + 6))}, + Min: image.Point{X: minx, Y: miny}, + Max: image.Point{X: maxx, Y: maxy}, }.Push(gtx.Ops) d.Add(gtx.Ops) hc.Pop() @@ -879,14 +898,14 @@ func (r *Renderer) drawWrappedText(gtx layout.Context, str string, reg Region, w bc.Pop() r.selDragsOn[2] = true } - r.drawHandle(gtx, sx, sy, lineH) - r.drawHandle(gtx, ex, ey, lineH) + r.drawHandle(gtx, sx, sy, lineH, r.selDraggingWhich == 0) + r.drawHandle(gtx, ex, ey, lineH, r.selDraggingWhich == 1) } else { // Caret drag (long press on blank space): a single handle on the caret. cx, cy := handleAt(cursorPos) registerDrag(&r.selDragCaret, cx, cy) r.selDragsOn[3] = true - r.drawHandle(gtx, cx, cy, lineH) + r.drawHandle(gtx, cx, cy, lineH, r.selDraggingWhich == 3) } } @@ -899,12 +918,34 @@ func (r *Renderer) drawWrappedText(gtx layout.Context, str string, reg Region, w } } -// drawHandle draws a selection handle: a short vertical stem with a filled -// square foot at the line's bottom (v1 approximation of Android's circle). -func (r *Renderer) drawHandle(gtx layout.Context, x, y, lineH Dp) { +// drawHandle draws a selection handle mimicking the native Android +// teardrop: a filled circle below the line with a short stem reaching up +// toward the line, in the system selection blue. (x, y) is the insertion +// point at the top of the line, as returned by handleAt. While the handle +// is being dragged it is drawn enlarged, as the framework does. +func (r *Renderer) drawHandle(gtx layout.Context, x, y, lineH Dp, dragging bool) { col := Color{R: 51, G: 153, B: 255, A: 255} - r.drawBg(gtx, Region{X: x - 1, Y: y, W: 2, H: lineH}, col) - r.drawBg(gtx, Region{X: x - 5, Y: y + lineH - 2, W: 10, H: 10}, col) + radius, stemW, stemLen := Dp(10), Dp(3), Dp(12) + if dragging { + radius, stemW, stemLen = Dp(13), Dp(4), Dp(15) + } + // Stem: from just below the line's bottom up into the line, meeting the + // top of the circle (2dp overlap avoids a seam between the two shapes). + circleTop := y + lineH + r.drawBg(gtx, Region{X: x - stemW/2, Y: circleTop - stemLen, W: stemW, H: stemLen + 2}, col) + r.drawCircle(gtx, x, circleTop+radius, radius, col) +} + +// drawCircle draws a filled circle of radius r centred at (cx, cy), as a +// square RRect clip with all corner radii at half the side. +func (r *Renderer) drawCircle(gtx layout.Context, cx, cy, rad Dp, col Color) { + rr := clip.UniformRRect(image.Rectangle{ + Min: image.Point{X: int(r.toPx(cx - rad)), Y: int(r.toPx(cy - rad))}, + Max: image.Point{X: int(r.toPx(cx + rad)), Y: int(r.toPx(cy + rad))}, + }, int(r.toPx(rad))).Push(gtx.Ops) + paint.ColorOp{Color: color.NRGBA{R: col.R, G: col.G, B: col.B, A: col.A}}.Add(gtx.Ops) + paint.PaintOp{}.Add(gtx.Ops) + rr.Pop() } func (r *Renderer) drawPng(gtx layout.Context, img image.Image, reg Region, width, height Dp) {