diff --git a/TASKS.md b/TASKS.md index 639043d..117893c 100644 --- a/TASKS.md +++ b/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 | diff --git a/src/buttons/group.rs b/src/buttons/group.rs index 079a6f6..67448f8 100644 --- a/src/buttons/group.rs +++ b/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; -/// 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() diff --git a/src/canvas/marks.rs b/src/canvas/marks.rs index 661729c..cdd25e1 100644 --- a/src/canvas/marks.rs +++ b/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() diff --git a/src/canvas/tests/mod.rs b/src/canvas/tests/mod.rs index 517d7ab..b1832f2 100644 --- a/src/canvas/tests/mod.rs +++ b/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) -> AnyElement, + part: fn(&Stage, Entity) -> AnyElement, + tool: Tool, heard: Vec, said: Vec, } @@ -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) -> AnyElement, + part: fn(&Stage, Entity) -> AnyElement, cx: &mut TestAppContext, ) -> (Entity, &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, cx: &mut VisualTestContext) -> Vec { host.read_with(cx, |stage, _| stage.heard.clone()) } -fn plane(owner: Entity) -> AnyElement { +fn plane(_: &Stage, owner: Entity) -> 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) -> AnyElement { +fn zoom(_: &Stage, owner: Entity) -> 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) -> AnyElement { +fn map(_: &Stage, owner: Entity) -> AnyElement { MiniMap::new( "map", [Frame::new(0.0, 0.0, 400.0, 280.0)], diff --git a/src/canvas/tests/tools.rs b/src/canvas/tests/tools.rs index 4b185ca..2016fd3 100644 --- a/src/canvas/tests/tools.rs +++ b/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 { point(px(x), px(y)) @@ -91,14 +93,14 @@ fn layer(tool: Tool, selected: &[&'static str], owner: Entity) -> 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, 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) -> 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() + ); + } +} diff --git a/src/canvas/tools.rs b/src/canvas/tools.rs index 95028ce..0a3c072 100644 --- a/src/canvas/tools.rs +++ b/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 = 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, bounds: Bounds| { 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, 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); + } + }); +}