Canvas · Tools: single handles, a wrapping palette, the pen ends with its tool

Handles resize one shape, so a selection of several shows its outline
and no handles; a press there no longer falls through to a marquee.
ToggleGroup wraps its faces when narrow, so the ToolPalette stays in a
280px box. A pen path left open ends when the tool changes: two points
or more go to the owner after the frame, a lone point is dropped, and
the open path paints only while the pen is the tool.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

TASKS.md

@@ -375,3 +375,4 @@ Per-component lines live in `tasks/`. Tags there:
 | T31d | 2 | PASS | |
 | T32a | 1 | FAIL | the plain wheel panned against every scroll box; the handlers panned from the render's viewport, so events within a frame overwrote each other; the fit's cap at one went untested |
 | T32a | 2 | PASS | |
+| T32b | 1 | FAIL | handles drawn for a multi-selection fell through to a marquee; the palette ran past a 280px box; an open pen path outlived the tool |

src/buttons/group.rs

@@ -105,9 +105,10 @@ fn toggle_face(
     let colors = &theme.colors;
     let (text, icon_size) = label_size(size);
     let fg = if on { colors.fg } else { colors.fg_muted };
-    let hover = colors.hover;
+    let (hover, value) = (colors.hover, item.value.clone());
     div()
         .id(id)
+        .debug_selector(move || format!("toggle {value}"))
         .flex()
         .flex_none()
         .items_center()
@@ -194,7 +195,7 @@ impl RenderOnce for ToggleButton {
 
 type OnChange = Rc<dyn Fn(&[SharedString], &mut Window, &mut App)>;
 
-/// Toggles side by side. One is chosen, or several with `multiple`.
+/// Toggles side by side, wrapping onto more lines when narrow. One is chosen, or several with `multiple`.
 #[derive(IntoElement)]
 pub struct ToggleGroup {
     id: ElementId,
@@ -280,6 +281,7 @@ impl RenderOnce for ToggleGroup {
         div()
             .id(self.id)
             .flex()
+            .flex_wrap()
             .items_center()
             .gap_0p5()
             .p_0p5()

src/canvas/marks.rs

@@ -1,4 +1,7 @@
-use gpui::{App, Hsla, IntoElement, ParentElement, Pixels, RenderOnce, Styled, Window, div};
+use gpui::{
+    App, Hsla, InteractiveElement, IntoElement, ParentElement, Pixels, RenderOnce, Styled, Window,
+    div,
+};
 
 use super::{
     edit::Handle,
@@ -8,7 +11,7 @@ use super::{
 use crate::theme::ActiveTheme;
 
 /// A box in view pixels as an absolute element.
-fn placed(frame: &Frame) -> gpui::Div {
+pub(super) fn placed(frame: &Frame) -> gpui::Div {
     div()
         .absolute()
         .left(Pixels::from(frame.x))
@@ -73,7 +76,13 @@ impl RenderOnce for TransformHandles {
         let side = f32::from(theme.canvas().handle.to_pixels(window.rem_size()));
         let squares = handle_boxes(&self.frame, side)
             .into_iter()
-            .map(move |(_, frame)| placed(&frame).bg(surface).border_1().border_color(accent));
+            .map(move |(handle, frame)| {
+                placed(&frame)
+                    .bg(surface)
+                    .border_1()
+                    .border_color(accent)
+                    .debug_selector(move || format!("handle {handle:?}"))
+            });
         div()
             .absolute()
             .inset_0()

src/canvas/tests/mod.rs

@@ -4,14 +4,15 @@ use gpui::{
     div, point, px,
 };
 
-use super::{Frame, InfiniteCanvas, MiniMap, Viewport, ZoomControls};
+use super::{Frame, InfiniteCanvas, MiniMap, Tool, Viewport, ZoomControls};
 use crate::{primitives::FocusNext, theme::Theme};
 
 mod tools;
 
 /// A view that shows one canvas part and keeps the viewports and words it heard.
 struct Stage {
-    part: fn(Entity<Stage>) -> AnyElement,
+    part: fn(&Stage, Entity<Stage>) -> AnyElement,
+    tool: Tool,
     heard: Vec<Viewport>,
     said: Vec<String>,
 }
@@ -21,7 +22,7 @@ impl Render for Stage {
         div()
             .w(px(400.0))
             .h(px(300.0))
-            .child((self.part)(cx.entity()))
+            .child((self.part)(self, cx.entity()))
     }
 }
 
@@ -34,7 +35,7 @@ fn settle(cx: &mut VisualTestContext) {
 }
 
 fn stage(
-    part: fn(Entity<Stage>) -> AnyElement,
+    part: fn(&Stage, Entity<Stage>) -> AnyElement,
     cx: &mut TestAppContext,
 ) -> (Entity<Stage>, &mut VisualTestContext) {
     cx.update(|cx| {
@@ -44,6 +45,7 @@ fn stage(
     });
     let (host, cx) = cx.add_window_view(|_, _| Stage {
         part,
+        tool: Tool::Select,
         heard: Vec::new(),
         said: Vec::new(),
     });
@@ -73,7 +75,7 @@ fn heard(host: &Entity<Stage>, cx: &mut VisualTestContext) -> Vec<Viewport> {
     host.read_with(cx, |stage, _| stage.heard.clone())
 }
 
-fn plane(owner: Entity<Stage>) -> AnyElement {
+fn plane(_: &Stage, owner: Entity<Stage>) -> AnyElement {
     InfiniteCanvas::new("plane", Viewport::new(0.0, 0.0, 1.0))
         .on_viewport(move |view, _, cx| hear(&owner, view, cx))
         .into_any_element()
@@ -122,7 +124,7 @@ fn a_drag_on_empty_space_pans(cx: &mut TestAppContext) {
     );
 }
 
-fn zoom(owner: Entity<Stage>) -> AnyElement {
+fn zoom(_: &Stage, owner: Entity<Stage>) -> AnyElement {
     ZoomControls::new("zoom", 1.0)
         .on_zoom(move |zoom, _, cx| hear(&owner, Viewport::new(0.0, 0.0, zoom), cx))
         .into_any_element()
@@ -146,7 +148,7 @@ fn the_zoom_buttons_step_and_the_percent_resets(cx: &mut TestAppContext) {
     assert_eq!(zooms, [0.75, 1.0, 1.5]);
 }
 
-fn map(owner: Entity<Stage>) -> AnyElement {
+fn map(_: &Stage, owner: Entity<Stage>) -> AnyElement {
     MiniMap::new(
         "map",
         [Frame::new(0.0, 0.0, 400.0, 280.0)],

src/canvas/tests/tools.rs

@@ -1,10 +1,12 @@
 use gpui::{
     AnyElement, Entity, IntoElement, Modifiers, MouseButton, MouseDownEvent, MouseUpEvent,
-    TestAppContext, VisualTestContext, point, px,
+    ParentElement, Styled, TestAppContext, VisualTestContext, div, point, px,
 };
 
 use super::{Stage, said, say, settle, stage};
-use crate::canvas::{Frame, InfiniteCanvas, Shape, ShapeKind, Tool, ToolLayer, Viewport};
+use crate::canvas::{
+    Frame, InfiniteCanvas, Shape, ShapeKind, Tool, ToolLayer, ToolPalette, Viewport,
+};
 
 fn at(x: f32, y: f32) -> gpui::Point<gpui::Pixels> {
     point(px(x), px(y))
@@ -91,14 +93,14 @@ fn layer(tool: Tool, selected: &[&'static str], owner: Entity<Stage>) -> AnyElem
 
 #[gpui::test]
 fn a_shape_tool_draws_what_the_drag_spans(cx: &mut TestAppContext) {
-    let (host, cx) = stage(|owner| layer(Tool::Ellipse, &[], owner), cx);
+    let (host, cx) = stage(|_, owner| layer(Tool::Ellipse, &[], owner), cx);
     drag((110.0, 120.0), (190.0, 180.0), cx);
     assert_eq!(said(&host, cx), ["draw Ellipse 110 120 80 60"]);
 }
 
 #[gpui::test]
 fn a_selected_shape_moves_and_snaps_to_its_neighbour(cx: &mut TestAppContext) {
-    let (host, cx) = stage(|owner| layer(Tool::Select, &["a"], owner), cx);
+    let (host, cx) = stage(|_, owner| layer(Tool::Select, &["a"], owner), cx);
     drag((50.0, 50.0), (147.0, 50.0), cx);
     assert_eq!(
         said(&host, cx),
@@ -109,7 +111,7 @@ fn a_selected_shape_moves_and_snaps_to_its_neighbour(cx: &mut TestAppContext) {
 
 #[gpui::test]
 fn a_drag_on_empty_space_selects_what_it_holds(cx: &mut TestAppContext) {
-    let (host, cx) = stage(|owner| layer(Tool::Select, &[], owner), cx);
+    let (host, cx) = stage(|_, owner| layer(Tool::Select, &[], owner), cx);
     drag((350.0, 150.0), (150.0, -10.0), cx);
     assert_eq!(
         said(&host, cx),
@@ -120,17 +122,89 @@ fn a_drag_on_empty_space_selects_what_it_holds(cx: &mut TestAppContext) {
 
 #[gpui::test]
 fn a_corner_handle_resizes_the_selection(cx: &mut TestAppContext) {
-    let (host, cx) = stage(|owner| layer(Tool::Select, &["a"], owner), cx);
+    let (host, cx) = stage(|_, owner| layer(Tool::Select, &["a"], owner), cx);
     drag((100.0, 100.0), (150.0, 120.0), cx);
     assert_eq!(said(&host, cx), ["resize a 150 120"]);
 }
 
 #[gpui::test]
 fn the_pen_adds_a_point_a_press_and_a_double_press_ends_the_path(cx: &mut TestAppContext) {
-    let (host, cx) = stage(|owner| layer(Tool::Pen, &[], owner), cx);
+    let (host, cx) = stage(|_, owner| layer(Tool::Pen, &[], owner), cx);
     for (x, y) in [(110.0, 150.0), (160.0, 150.0), (160.0, 200.0)] {
         press(x, y, 1, cx);
     }
     press(160.0, 200.0, 2, cx);
     assert_eq!(said(&host, cx), ["draw path of 3 110 150 50 50"]);
 }
+
+#[gpui::test]
+fn a_two_shape_selection_shows_no_handles(cx: &mut TestAppContext) {
+    let (_, cx) = stage(|_, owner| layer(Tool::Select, &["a", "b"], owner), cx);
+    assert!(
+        cx.debug_bounds("handle North").is_none(),
+        "handles resize one shape"
+    );
+    press(150.0, 0.0, 1, cx);
+    assert!(cx.debug_bounds("handle North").is_none());
+}
+
+#[gpui::test]
+fn one_selected_shape_shows_its_handles(cx: &mut TestAppContext) {
+    let (_, cx) = stage(|_, owner| layer(Tool::Select, &["a"], owner), cx);
+    let north = cx
+        .debug_bounds("handle North")
+        .expect("a handle on the top side");
+    assert_eq!(north.center(), at(50.0, 0.0));
+}
+
+fn switch(host: &Entity<Stage>, tool: Tool, cx: &mut VisualTestContext) {
+    host.update(cx, |stage, cx| {
+        stage.tool = tool;
+        cx.notify();
+    });
+    settle(cx);
+}
+
+#[gpui::test]
+fn a_path_left_open_goes_to_the_owner_when_the_tool_changes(cx: &mut TestAppContext) {
+    let (host, cx) = stage(|stage, owner| layer(stage.tool, &[], owner), cx);
+    switch(&host, Tool::Pen, cx);
+    for (x, y) in [(110.0, 150.0), (160.0, 150.0), (160.0, 200.0)] {
+        press(x, y, 1, cx);
+    }
+    switch(&host, Tool::Select, cx);
+    assert_eq!(said(&host, cx), ["draw path of 3 110 150 50 50"]);
+    switch(&host, Tool::Pen, cx);
+    press(120.0, 160.0, 1, cx);
+    switch(&host, Tool::Hand, cx);
+    switch(&host, Tool::Pen, cx);
+    press(130.0, 170.0, 1, cx);
+    press(130.0, 170.0, 2, cx);
+    assert_eq!(
+        said(&host, cx),
+        ["draw path of 3 110 150 50 50"],
+        "a lone point is dropped, so the next path starts afresh"
+    );
+}
+
+fn palette(_: &Stage, _: Entity<Stage>) -> AnyElement {
+    div()
+        .w(px(280.0))
+        .child(ToolPalette::new("palette", Tool::Select))
+        .into_any_element()
+}
+
+#[gpui::test]
+fn the_palette_wraps_inside_a_narrow_box(cx: &mut TestAppContext) {
+    let (_, cx) = stage(palette, cx);
+    for tool in Tool::ALL {
+        let selector = format!("toggle {}", tool.words()).leak();
+        let face = cx.debug_bounds(selector).expect("a face per tool");
+        assert!(
+            face.right() <= px(280.0),
+            "{} ends at {:?}",
+            tool.words(),
+            face.right()
+        );
+    }
+}

src/canvas/tools.rs

@@ -12,7 +12,7 @@ use super::{
         Brush, Gesture, Hand, Handlers, LEAST, OnDraw, OnKeys, OnMove, OnResize, PEN, REACH, Scene,
         commit, path, spanned, stroke_shape, taken,
     },
-    marks::{SelectionBox, SnapIndicator, TransformHandles, handle_boxes},
+    marks::{SelectionBox, SnapIndicator, TransformHandles, handle_boxes, placed},
     plane::{in_view, paint_shape},
     shape::{Shape, ShapeKind},
     view::{Frame, Viewport},
@@ -111,6 +111,10 @@ impl RenderOnce for ToolLayer {
     fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement {
         let hand: Entity<Hand> =
             window.use_keyed_state((self.id.clone(), "hand"), cx, |_, _| Hand::default());
+        if self.tool != Tool::Pen && !hand.read(cx).pen.is_empty() {
+            let points = hand.update(cx, |hand, _| std::mem::take(&mut hand.pen));
+            end_path(points, self.on_draw.clone(), window, cx);
+        }
         if self.tool == Tool::Hand {
             return div().into_any_element();
         }
@@ -187,7 +191,9 @@ impl RenderOnce for ToolLayer {
             Gesture::Stroke { points } if points.len() > 1 => {
                 vec![stroke_shape(points, self.brush.size, self.brush.hue)]
             }
-            _ if pen.len() > 1 => vec![stroke_shape(&pen, PEN, self.brush.hue)],
+            _ if tool == Tool::Pen && pen.len() > 1 => {
+                vec![stroke_shape(&pen, PEN, self.brush.hue)]
+            }
             _ => Vec::new(),
         };
         let marquee = match &gesture {
@@ -197,10 +203,16 @@ impl RenderOnce for ToolLayer {
             ))),
             _ => None,
         };
-        let handles = (tool == Tool::Select && matches!(gesture, Gesture::Idle))
+        let chosen = (tool == Tool::Select && matches!(gesture, Gesture::Idle))
             .then_some(whole)
             .flatten()
-            .map(|frame| TransformHandles::new(in_view(&view, &frame)));
+            .map(|frame| in_view(&view, &frame));
+        let handles = chosen
+            .filter(|_| selected.len() == 1)
+            .map(TransformHandles::new);
+        let outline = chosen
+            .filter(|_| selected.len() > 1)
+            .map(|frame| placed(&frame).border_1().border_color(palette.accent));
         let snap = (!guides.is_empty()).then(|| SnapIndicator::new(guides.clone(), view));
         let point_of = move |at: Point<Pixels>, bounds: Bounds<Pixels>| {
             let local = at - bounds.origin;
@@ -402,7 +414,23 @@ impl RenderOnce for ToolLayer {
             )
             .children(marquee)
             .children(handles)
+            .children(outline)
             .children(snap)
             .into_any_element()
     }
 }
+
+/// Ends a pen path the tool left: two points or more go to the owner after this frame, a lone point is dropped.
+fn end_path(points: Vec<(f32, f32)>, on_draw: Option<OnDraw>, window: &mut Window, cx: &mut App) {
+    if points.len() < 2 {
+        log::info!("tool layer: the pen left a lone point, dropped");
+        return;
+    }
+    window.defer(cx, move |window, cx| {
+        let (kind, frame) = path(&points, PEN);
+        log::info!("tool layer: the pen left a path of {} points", points.len());
+        if let Some(on_draw) = &on_draw {
+            on_draw(kind, frame, window, cx);
+        }
+    });
+}