diff --git a/TASKS.md b/TASKS.md index 54d63a1..e4a4804 100644 --- a/TASKS.md +++ b/TASKS.md @@ -49,8 +49,8 @@ Per-component lines live in `tasks/`. Tags there: - [x] T13a Lists & Trees · Lists - [x] T13b Lists & Trees · Trees - [x] T13c Lists & Trees · Files -- [ ] T14a Tables · Core -- [ ] T14b Tables · Columns & rows +- [x] T14a Tables · Core +- [x] T14b Tables · Columns & rows - [ ] T14c Tables · Grids - [ ] T15a Charts · Cartesian - [ ] T15b Charts · Parts and spreads @@ -204,8 +204,11 @@ Per-component lines live in `tasks/`. Tags there: | T13c | 1 | FAIL | the cursor kept its index through a re-sort, so Enter opened another file | | T13c | 2 | PASS | | | T14a | 1 | FAIL | numbers and words compared out of order in a mixed column, and the sort panicked | +| T14a | 2 | PASS | | | T14b | 1 | FAIL | a removed filter rule left its typing to the next rule, and two sort rows could take one column | +| T14b | 2 | PASS | | | T14c | 1 | FAIL | a shrunk grid kept its cursor outside, long column letters overflowed, presses landed in merged cells, Tab left the grid, and ranges counted text and blanks | -| T15a | 1 | — | pending review | -| T15b | 1 | — | pending review | -| T15c | 1 | — | pending review | +| T14c | 2 | FAIL | Tab stepped into a merge's hidden cells | +| T15a | 1 | FAIL | an empty chart's SVG export panicked, and a huge flat domain looped forever making ticks | +| T15b | 1 | FAIL | a pointed part outlived shrinking data, an empty heatmap made up a cell, a negative total flipped its bar, and a sunburst child filled its parent's arc | +| T15c | 1 | FAIL | eight charts kept a pointed part past their data, and the Gantt today line ran past the timeline | diff --git a/src/charts/calendar.rs b/src/charts/calendar.rs index e378a43..039549e 100644 --- a/src/charts/calendar.rs +++ b/src/charts/calendar.rs @@ -67,7 +67,10 @@ impl RenderOnce for CalendarHeatmap { fn render(mut self, window: &mut Window, cx: &mut App) -> impl IntoElement { let tiles: Entity = window.use_keyed_state((self.id.clone(), "tiles"), cx, |_, _| Tiles::default()); - let (bounds, hover) = (tiles.read(cx).bounds, tiles.read(cx).hover); + let (bounds, hover) = ( + tiles.read(cx).bounds, + tiles.read(cx).pointed(self.counts.len()), + ); let theme = cx.theme(); let (colors, sizes, rem) = (theme.colors.clone(), theme.chart(), window.rem_size()); let pixels = |length: gpui::Rems| f32::from(length.to_pixels(rem)); diff --git a/src/charts/distribution.rs b/src/charts/distribution.rs index df5d02e..97cf066 100644 --- a/src/charts/distribution.rs +++ b/src/charts/distribution.rs @@ -159,7 +159,16 @@ impl WaterfallChart { Step::Down => Ink::Fall, Step::Total => Ink::Series(0), }; - let (drawn, names) = (bars.clone(), labels.clone()); + let running: Rc> = Rc::new( + self.steps + .iter() + .scan(0.0, |sum, (_, change, total)| { + *sum += if *total { 0.0 } else { *change }; + Some(*sum) + }) + .collect(), + ); + let (drawn, names, levels) = (bars.clone(), labels.clone(), running.clone()); let layout: Layout = Rc::new(move |scene: &Scene| { let (low, high) = drawn .iter() @@ -180,7 +189,7 @@ impl WaterfallChart { }, )); if ix + 1 < drawn.len() { - let level = scale.at(if *step == Step::Down { *bottom } else { *top }); + let level = scale.at(levels[ix]); geometry.strokes.push(( Ink::Rule, (start + width, level), @@ -192,9 +201,9 @@ impl WaterfallChart { }); let changes = self.steps; let tips: Tips = Rc::new(move |ix, _| { - let ((label, change, total), (_, top, step)) = (&changes[ix], bars[ix]); + let ((label, change, total), (_, _, step)) = (&changes[ix], bars[ix]); let (name, value) = if *total { - ("Total", compact(top)) + ("Total", compact(running[ix])) } else { ( "Change", diff --git a/src/charts/flow.rs b/src/charts/flow.rs index 09844e6..8793b93 100644 --- a/src/charts/flow.rs +++ b/src/charts/flow.rs @@ -229,6 +229,10 @@ impl RenderOnce for SankeyChart { let flows: Entity = window.use_keyed_state((self.id.clone(), "flows"), cx, |_, _| Flows::default()); let (bounds, hover) = (flows.read(cx).bounds, flows.read(cx).hover); + let hover = hover.filter(|hover| match hover { + Hover::Node(ix) => *ix < self.nodes.len(), + Hover::Link(ix) => *ix < self.links.len(), + }); let theme = cx.theme(); let (colors, sizes, rem) = (theme.colors.clone(), theme.chart(), window.rem_size()); let pixels = |length: gpui::Rems| f32::from(length.to_pixels(rem)); diff --git a/src/charts/funnel.rs b/src/charts/funnel.rs index 95d89ec..4b7afb1 100644 --- a/src/charts/funnel.rs +++ b/src/charts/funnel.rs @@ -87,7 +87,10 @@ impl RenderOnce for FunnelChart { fn render(mut self, window: &mut Window, cx: &mut App) -> impl IntoElement { let flows: Entity = window.use_keyed_state((self.id.clone(), "flows"), cx, |_, _| Tiles::default()); - let (bounds, hover) = (flows.read(cx).bounds, flows.read(cx).hover); + let (bounds, hover) = ( + flows.read(cx).bounds, + flows.read(cx).pointed(self.stages.len()), + ); let theme = cx.theme(); let (colors, sizes, rem) = (theme.colors.clone(), theme.chart(), window.rem_size()); let pixels = |length: gpui::Rems| f32::from(length.to_pixels(rem)); diff --git a/src/charts/geometry.rs b/src/charts/geometry.rs index da18018..355e897 100644 --- a/src/charts/geometry.rs +++ b/src/charts/geometry.rs @@ -104,6 +104,9 @@ pub(crate) struct Categories<'a> { /// Lays out categories and their series inside `frame`: bands along one axis, values along the other. pub(crate) fn categories(chart: &Categories, frame: Rect) -> Geometry { + if chart.labels.is_empty() { + return Geometry::default(); + } let (first, last) = chart.span; let shown: Vec<(usize, &[f64])> = chart .names @@ -388,6 +391,34 @@ mod tests { ); } + #[test] + fn no_categories_lay_out_nothing() { + let hidden = HashSet::new(); + let empty = Categories { + labels: &[], + names: &[], + values: &[], + mark: Mark::Line, + stacked: false, + horizontal: false, + gap: 0.28, + hidden: &hidden, + span: (0, 0), + }; + assert_eq!( + categories( + &empty, + Rect { + x: 0.0, + y: 0.0, + w: 100.0, + h: 100.0 + } + ), + Geometry::default() + ); + } + #[test] fn a_frame_leaves_its_gutters() { let rect = frame((400.0, 200.0), 48.0, 24.0, 8.0); diff --git a/src/charts/network.rs b/src/charts/network.rs index 6f9e6b2..bb426ab 100644 --- a/src/charts/network.rs +++ b/src/charts/network.rs @@ -173,7 +173,10 @@ impl RenderOnce for NetworkGraph { if !settled { window.request_animation_frame(); } - let (bounds, hover) = (graph.read(cx).bounds, graph.read(cx).hover); + let (bounds, hover) = ( + graph.read(cx).bounds, + graph.read(cx).hover.filter(|ix| *ix < self.nodes.len()), + ); let theme = cx.theme(); let (colors, sizes, rem) = (theme.colors.clone(), theme.chart(), window.rem_size()); let pixels = |length: gpui::Rems| f32::from(length.to_pixels(rem)); @@ -275,6 +278,7 @@ impl RenderOnce for NetworkGraph { let offset = position - bounds.origin; (f32::from(offset.x), f32::from(offset.y)) }; + let count = self.nodes.len(); let (moved, pressed, released, left, measured) = ( graph.clone(), graph.clone(), @@ -299,7 +303,8 @@ impl RenderOnce for NetworkGraph { .on_mouse_move(move |event: &MouseMoveEvent, _, cx| { let place = local(event.position, moved.read(cx).bounds); moved.update(cx, |graph, cx| { - if let (Some(ix), Some(MouseButton::Left)) = (graph.drag, event.pressed_button) + if let (Some(ix), Some(MouseButton::Left)) = + (graph.drag.filter(|ix| *ix < count), event.pressed_button) { graph .force diff --git a/src/charts/parallel.rs b/src/charts/parallel.rs index 2763fd1..5d4dacc 100644 --- a/src/charts/parallel.rs +++ b/src/charts/parallel.rs @@ -132,7 +132,10 @@ impl RenderOnce for ParallelCoordinates { fn render(mut self, window: &mut Window, cx: &mut App) -> impl IntoElement { let tiles: Entity = window.use_keyed_state((self.id.clone(), "lines"), cx, |_, _| Tiles::default()); - let (bounds, hover) = (tiles.read(cx).bounds, tiles.read(cx).hover); + let (bounds, hover) = ( + tiles.read(cx).bounds, + tiles.read(cx).pointed(self.records.len()), + ); let theme = cx.theme(); let (colors, sizes, rem) = (theme.colors.clone(), theme.chart(), window.rem_size()); let pixels = |length: gpui::Rems| f32::from(length.to_pixels(rem)); diff --git a/src/charts/radar.rs b/src/charts/radar.rs index 5ce98ad..c7b79ea 100644 --- a/src/charts/radar.rs +++ b/src/charts/radar.rs @@ -110,7 +110,8 @@ impl RenderOnce for RadarChart { } let (bounds, hover, hidden) = { let radar = radar.read(cx); - (radar.bounds, radar.hover, radar.hidden.clone()) + let hover = radar.hover.filter(|spoke| *spoke < self.axes.len()); + (radar.bounds, hover, radar.hidden.clone()) }; let theme = cx.theme(); let (colors, sizes, rem) = (theme.colors.clone(), theme.chart(), window.rem_size()); diff --git a/src/charts/relations.rs b/src/charts/relations.rs index a78cf5b..ee8cc22 100644 --- a/src/charts/relations.rs +++ b/src/charts/relations.rs @@ -154,7 +154,10 @@ impl RenderOnce for ChordDiagram { fn render(mut self, window: &mut Window, cx: &mut App) -> impl IntoElement { let tiles: Entity = window.use_keyed_state((self.id.clone(), "chords"), cx, |_, _| Tiles::default()); - let (bounds, hover) = (tiles.read(cx).bounds, tiles.read(cx).hover); + let (bounds, hover) = ( + tiles.read(cx).bounds, + tiles.read(cx).pointed(self.names.len()), + ); let theme = cx.theme(); let (colors, sizes, rem) = (theme.colors.clone(), theme.chart(), window.rem_size()); let pixels = |length: gpui::Rems| f32::from(length.to_pixels(rem)); diff --git a/src/charts/scale.rs b/src/charts/scale.rs index 0192428..bbc621c 100644 --- a/src/charts/scale.rs +++ b/src/charts/scale.rs @@ -48,14 +48,16 @@ pub(crate) fn nice_step(span: f64, count: usize) -> f64 { /// A domain widened to round steps that hold `low..=high`, and the ticks along it. pub(crate) fn nice(low: f64, high: f64, count: usize) -> ((f64, f64), Vec) { - let (low, high) = if low == high { - (low - 1.0, high + 1.0) + let (low, high) = (low.min(high), low.max(high)); + let (low, high) = if high - low <= high.abs().max(low.abs()) * 1e-12 { + let pad = (high.abs().max(low.abs()) * 0.1).max(1.0); + (low - pad, high + pad) } else { - (low.min(high), low.max(high)) + (low, high) }; let step = nice_step(high - low, count); let (start, end) = ((low / step).floor() * step, (high / step).ceil() * step); - let ticks = (0..) + let ticks = (0..count.max(1) * 4 + 4) .map(|ix| start + step * ix as f64) .take_while(|tick| *tick <= end + step * 1e-9) .map(|tick| if tick.abs() < step * 1e-9 { 0.0 } else { tick }) @@ -137,6 +139,13 @@ mod tests { assert_eq!(domain, (-15.0, 10.0)); assert_eq!(ticks.first(), Some(&-15.0)); assert_eq!(nice(5.0, 5.0, 4).0, (4.0, 6.0), "a single value widens"); + for (low, high) in [(1e30, 1e30), (1e30, 1e30 + 1e14)] { + let ((from, to), ticks) = nice(low, high, 4); + assert!( + from < to && (2..=20).contains(&ticks.len()), + "a huge flat domain widens by its size: {ticks:?}" + ); + } } #[test] diff --git a/src/charts/schedule.rs b/src/charts/schedule.rs index cc6210a..9fa05c7 100644 --- a/src/charts/schedule.rs +++ b/src/charts/schedule.rs @@ -133,7 +133,10 @@ impl RenderOnce for GanttChart { fn render(mut self, window: &mut Window, cx: &mut App) -> impl IntoElement { let tiles: Entity = window.use_keyed_state((self.id.clone(), "rows"), cx, |_, _| Tiles::default()); - let (bounds, hover) = (tiles.read(cx).bounds, tiles.read(cx).hover); + let (bounds, hover) = ( + tiles.read(cx).bounds, + tiles.read(cx).pointed(self.tasks.len()), + ); let theme = cx.theme(); let (colors, sizes, rem) = (theme.colors.clone(), theme.chart(), window.rem_size()); let pixels = |length: gpui::Rems| f32::from(length.to_pixels(rem)); @@ -218,7 +221,7 @@ impl RenderOnce for GanttChart { .today .zip(first) .map(|(today, first)| x_of(days(first, today)) + day / 2.0) - .filter(|x| *x >= names); + .filter(|x| (names..=names + day * span as f32).contains(x)); let tooltip = hover.map(|ix| { let (task, bar) = (&self.tasks[ix], bars[ix]); let dates = format!( diff --git a/src/charts/stats.rs b/src/charts/stats.rs index 2d9930b..f4ec8a4 100644 --- a/src/charts/stats.rs +++ b/src/charts/stats.rs @@ -125,14 +125,14 @@ pub(crate) enum Step { Total, } -/// Each change floating from the running total, bottom and top; a total stands on zero at the sum so far. +/// Each change floating from the running total, bottom and top; a total stands on zero, reaching up or down to the sum so far. pub(crate) fn steps(changes: &[(f64, bool)]) -> Vec<(f64, f64, Step)> { - let mut running = 0.0; + let mut running = 0.0f64; changes .iter() .map(|(value, total)| { if *total { - return (0.0, running, Step::Total); + return (running.min(0.0), running.max(0.0), Step::Total); } let from = running; running += value; @@ -184,6 +184,17 @@ mod tests { ); } + #[test] + fn a_total_below_zero_hangs_down_from_it() { + let bars = steps(&[(-10.0, false), (0.0, true)]); + let (bottom, top, step) = bars[1]; + assert_eq!( + (bottom, top, step), + (-10.0, 0.0, Step::Total), + "the bar runs from the total up to zero" + ); + } + #[test] fn a_waterfall_floats_changes_and_stands_totals_on_zero() { let bars = steps(&[(100.0, false), (-30.0, false), (0.0, true), (20.0, false)]); diff --git a/src/charts/sunburst.rs b/src/charts/sunburst.rs index a59142d..3f8189d 100644 --- a/src/charts/sunburst.rs +++ b/src/charts/sunburst.rs @@ -65,16 +65,15 @@ struct Arc { fn rings(slices: &[Slice]) -> Vec { fn walk( slices: &[Slice], - (depth, start, sweep): (usize, f32, f32), + (depth, start, sweep, whole): (usize, f32, f32, f64), branch: Option, trail: &[SharedString], out: &mut Vec, ) { - let total: f64 = slices.iter().map(Slice::weight).sum(); let mut at = start; for (ix, slice) in slices.iter().enumerate() { - let part = if total > 0.0 { - (slice.weight() / total) as f32 * sweep + let part = if whole > 0.0 { + (slice.weight() / whole) as f32 * sweep } else { 0.0 }; @@ -82,7 +81,7 @@ fn rings(slices: &[Slice]) -> Vec { path.push(slice.name.clone()); walk( &slice.children, - (depth + 1, at, part), + (depth + 1, at, part, slice.weight()), Some(branch), &path, out, @@ -99,7 +98,8 @@ fn rings(slices: &[Slice]) -> Vec { } } let mut out = Vec::new(); - walk(slices, (0, -FRAC_PI_2, TAU), None, &[], &mut out); + let whole = slices.iter().map(Slice::weight).sum(); + walk(slices, (0, -FRAC_PI_2, TAU, whole), None, &[], &mut out); out.sort_by_key(|arc| arc.depth); out } @@ -251,6 +251,17 @@ mod tests { use super::*; + #[test] + fn a_child_takes_only_its_share_of_its_parent() { + let arcs = rings(&[Slice::new("Whole", 100.0).children([Slice::new("Part", 25.0)])]); + let part = arcs.iter().find(|arc| arc.depth == 1).expect("the part"); + assert!( + (part.sweep - TAU / 4.0).abs() < 1e-5, + "a quarter of the ring, got {}", + part.sweep + ); + } + #[test] fn rings_nest_children_inside_their_parent() { let tree = [ diff --git a/src/charts/targets.rs b/src/charts/targets.rs index d14b57f..aec26c2 100644 --- a/src/charts/targets.rs +++ b/src/charts/targets.rs @@ -111,7 +111,10 @@ impl RenderOnce for BulletChart { fn render(mut self, window: &mut Window, cx: &mut App) -> impl IntoElement { let tiles: Entity = window.use_keyed_state((self.id.clone(), "rows"), cx, |_, _| Tiles::default()); - let (bounds, hover) = (tiles.read(cx).bounds, tiles.read(cx).hover); + let (bounds, hover) = ( + tiles.read(cx).bounds, + tiles.read(cx).pointed(self.bullets.len()), + ); let theme = cx.theme(); let (colors, sizes, rem) = (theme.colors.clone(), theme.chart(), window.rem_size()); let pixels = |length: gpui::Rems| f32::from(length.to_pixels(rem)); @@ -318,7 +321,10 @@ impl RenderOnce for ProgressChart { window.request_animation_frame(); } let glided = glided.into_iter().next().expect("one row of shares"); - let (bounds, hover) = (rings.read(cx).bounds, rings.read(cx).hover); + let (bounds, hover) = ( + rings.read(cx).bounds, + rings.read(cx).pointed(self.goals.len()), + ); let theme = cx.theme(); let (colors, sizes, rem) = (theme.colors.clone(), theme.chart(), window.rem_size()); let side = f32::from(bounds.size.width).min(f32::from(bounds.size.height)); diff --git a/src/charts/tests.rs b/src/charts/tests.rs index 8f102f9..3a6b505 100644 --- a/src/charts/tests.rs +++ b/src/charts/tests.rs @@ -1,6 +1,16 @@ -use gpui::TestAppContext; +use std::{cell::Cell, rc::Rc}; -use super::{BarChart, LineChart, Series}; +use gpui::{ + Context, IntoElement, Modifiers, MouseMoveEvent, ParentElement, Render, Styled, TestAppContext, + VisualTestContext, Window, div, point, px, +}; +use jiff::civil::date; + +use super::{ + BarChart, Bullet, BulletChart, CalendarHeatmap, ChordDiagram, FunnelChart, GanttChart, + HeatmapChart, LineChart, NetworkGraph, ParallelCoordinates, ProgressChart, RadarChart, + SankeyChart, Series, Task, Treemap, +}; use crate::theme::Theme; #[gpui::test] @@ -27,3 +37,135 @@ fn a_chart_exports_as_svg_and_csv(cx: &mut TestAppContext) { ); }); } + +/// Every chart that picks a part under the pointer, stacked 200 tall at 400 across, with many parts or few. +struct Picked(Rc>); + +const TALL: f32 = 200.0; + +impl Render for Picked { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + let many = self.0.get(); + let count = if many { 6 } else { 3 }; + let values = |count: usize| { + (0..count) + .map(|ix| 10.0 + ix as f64 * 3.0) + .collect::>() + }; + let names: Vec = (0..count).map(|ix| format!("n{ix}")).collect(); + let tile = |chart: gpui::AnyElement| { + div() + .w(px(400.0)) + .h(px(TALL)) + .overflow_hidden() + .child(chart) + }; + let treemap = names + .iter() + .zip(values(count)) + .fold(Treemap::new("treemap"), |map, (name, value)| { + map.tile(name.clone(), value) + }); + let heatmap = names + .iter() + .fold(HeatmapChart::new("heatmap", names.clone()), |map, name| { + map.row(name.clone(), values(count)) + }); + let radar = RadarChart::new("radar", names.clone()).series(Series::new("a", values(count))); + let funnel = names + .iter() + .zip(values(count).into_iter().rev()) + .fold(FunnelChart::new("funnel"), |funnel, (name, value)| { + funnel.stage(name.clone(), value) + }); + let sankey = (1..count).fold(SankeyChart::new("sankey", names.clone()), |sankey, to| { + sankey.link(0, to, 5.0) + }); + let gantt = (0..count).fold(GanttChart::new("gantt"), |gantt, ix| { + gantt.task(Task::new( + format!("t{ix}"), + date(2026, 3, 1 + ix as i8), + date(2026, 3, 9 + ix as i8), + )) + }); + let network = (1..count).fold( + names + .iter() + .fold(NetworkGraph::new("network"), |graph, name| { + graph.node(name.clone(), 0) + }), + |graph, to| graph.edge(0, to), + ); + let matrix: Vec> = (0..count) + .map(|row| { + (0..count) + .map(|col| if row == col { 0.0 } else { 1.0 + col as f64 }) + .collect() + }) + .collect(); + let chord = ChordDiagram::new("chord", names.clone(), matrix); + let parallel = (0..count).fold( + ParallelCoordinates::new("parallel", ["a", "b"]), + |chart, ix| chart.record(format!("r{ix}"), [ix as f64, (count - ix) as f64]), + ); + let bullets = (0..count).fold(BulletChart::new("bullets"), |chart, ix| { + chart.bullet(Bullet::new(format!("b{ix}"), 5.0 + ix as f64, 8.0)) + }); + let rings = (0..count).fold(ProgressChart::new("rings"), |chart, ix| { + chart.goal(format!("g{ix}"), 3.0 + ix as f64, 10.0) + }); + let days = CalendarHeatmap::new("days", date(2026, 1, 5), values(count * 10)); + div() + .flex() + .flex_col() + .child(tile(treemap.h(px(TALL)).into_any_element())) + .child(tile(heatmap.h(px(TALL)).into_any_element())) + .child(tile(radar.h(px(TALL)).into_any_element())) + .child(tile(funnel.h(px(TALL)).into_any_element())) + .child(tile(sankey.h(px(TALL)).into_any_element())) + .child(tile(gantt.h(px(TALL)).into_any_element())) + .child(tile(network.h(px(TALL)).into_any_element())) + .child(tile(chord.h(px(TALL)).into_any_element())) + .child(tile(parallel.h(px(TALL)).into_any_element())) + .child(tile(bullets.h(px(TALL)).into_any_element())) + .child(tile(rings.h(px(TALL)).into_any_element())) + .child(tile(days.h(px(TALL)).into_any_element())) + } +} + +fn settle(cx: &mut VisualTestContext) { + for _ in 0..3 { + cx.run_until_parked(); + cx.update(|window, _| window.refresh()); + } + cx.run_until_parked(); +} + +#[gpui::test] +fn a_part_under_the_pointer_lets_go_when_its_data_shrinks(cx: &mut TestAppContext) { + cx.update(|cx| { + Theme::init(cx); + Theme::update(cx, |theme| theme.reduced_motion = true); + }); + let many = Rc::new(Cell::new(true)); + let seen = many.clone(); + let (view, cx) = cx.add_window_view(|_, _| Picked(seen)); + cx.simulate_resize(gpui::size(px(400.0), px(TALL * 12.0))); + settle(cx); + for chart in 0..12 { + for (x, y) in [(360.0, 20.0), (200.0, 100.0), (60.0, 170.0)] { + let at = point(px(x), px(TALL * chart as f32 + y)); + cx.simulate_event(MouseMoveEvent { + position: at, + pressed_button: None, + modifiers: Modifiers::none(), + }); + settle(cx); + for full in [false, true] { + many.set(full); + view.update(cx, |_, cx| cx.notify()); + settle(cx); + } + } + } +} diff --git a/src/charts/tiles.rs b/src/charts/tiles.rs index 03fd97f..4b5e775 100644 --- a/src/charts/tiles.rs +++ b/src/charts/tiles.rs @@ -27,6 +27,13 @@ pub(crate) struct Tiles { pub hover: Option, } +impl Tiles { + /// The part under the pointer, if there are still that many parts. + pub(crate) fn pointed(&self, count: usize) -> Option { + self.hover.filter(|ix| *ix < count) + } +} + /// Keeps `tiles` answering the pointer: `find` names the tile at a place in the box. pub(crate) fn tracked( root: Div, @@ -120,7 +127,10 @@ impl RenderOnce for Treemap { fn render(mut self, window: &mut Window, cx: &mut App) -> impl IntoElement { let tiles: Entity = window.use_keyed_state((self.id.clone(), "tiles"), cx, |_, _| Tiles::default()); - let (bounds, hover) = (tiles.read(cx).bounds, tiles.read(cx).hover); + let (bounds, hover) = ( + tiles.read(cx).bounds, + tiles.read(cx).pointed(self.tiles.len()), + ); let theme = cx.theme(); let (colors, sizes, rem) = (theme.colors.clone(), theme.chart(), window.rem_size()); let pixels = |length: gpui::Rems| f32::from(length.to_pixels(rem)); @@ -296,7 +306,8 @@ impl RenderOnce for HeatmapChart { fn render(mut self, window: &mut Window, cx: &mut App) -> impl IntoElement { let tiles: Entity = window.use_keyed_state((self.id.clone(), "tiles"), cx, |_, _| Tiles::default()); - let (bounds, hover) = (tiles.read(cx).bounds, tiles.read(cx).hover); + let cells = self.rows.len() * self.columns.len(); + let (bounds, hover) = (tiles.read(cx).bounds, tiles.read(cx).pointed(cells)); let theme = cx.theme(); let (colors, sizes, rem) = (theme.colors.clone(), theme.chart(), window.rem_size()); let pixels = |length: gpui::Rems| f32::from(length.to_pixels(rem)); @@ -307,8 +318,8 @@ impl RenderOnce for HeatmapChart { pixels(sizes.foot), pixels(sizes.inset), ); - let (columns, rows) = (self.columns.len().max(1), self.rows.len().max(1)); - let (cell_w, cell_h) = (rect.w / columns as f32, rect.h / rows as f32); + let (columns, rows) = (self.columns.len(), self.rows.len()); + let (cell_w, cell_h) = (rect.w / columns.max(1) as f32, rect.h / rows.max(1) as f32); let cell = |row: usize, column: usize| Rect { x: rect.x + cell_w * column as f32, y: rect.y + cell_h * row as f32, @@ -402,7 +413,7 @@ impl RenderOnce for HeatmapChart { (self.id.clone(), "heatmap").into(), &tiles, move |at| { - rect.contains(at).then(|| { + (rect.contains(at) && rows > 0 && columns > 0).then(|| { let (row, column) = ( ((at.1 - rect.y) / cell_h) as usize, ((at.0 - rect.x) / cell_w) as usize, diff --git a/src/tables/grid.rs b/src/tables/grid.rs index 943532e..bfb534f 100644 --- a/src/tables/grid.rs +++ b/src/tables/grid.rs @@ -310,11 +310,9 @@ pub(crate) fn grid(face: Face, base: Div, window: &mut Window, cx: &mut App) -> let (fill_raw, fill_hears) = (face.raw.clone(), face.on_change.clone()); let numbered = face.numbered; let (merges, frozen) = (Rc::new(face.merges.clone()), face.frozen); - let (hovered_merges, stepped, stepped_back) = ( - merges.clone(), - (sheet.clone(), editor.clone()), - (sheet.clone(), editor.clone()), - ); + let hovered_merges = merges.clone(); + let ahead = (sheet.clone(), editor.clone(), merges.clone()); + let behind = (sheet.clone(), editor.clone(), merges.clone()); let keyed = keys(face, metrics, sheet.clone(), editor.clone()); let focus_on_press = focus.clone(); let text_size = theme.text_size(TextSize::Sm); @@ -329,20 +327,22 @@ pub(crate) fn grid(face: Face, base: Div, window: &mut Window, cx: &mut App) -> .border_color(colors.border) .on_key_down(keyed) .capture_action(move |_: &FocusNext, window, cx| { + let (sheet, editor, merges) = &ahead; + let size = (face_rows, face_cols); across( - &stepped.0, - &stepped.1, - (face_cols, false), + (sheet, editor), + (merges, size, false), (metrics, frozen), window, cx, ) }) .capture_action(move |_: &FocusPrev, window, cx| { + let (sheet, editor, merges) = &behind; + let size = (face_rows, face_cols); across( - &stepped_back.0, - &stepped_back.1, - (face_cols, true), + (sheet, editor), + (merges, size, true), (metrics, frozen), window, cx, diff --git a/src/tables/gridkeys.rs b/src/tables/gridkeys.rs index 09932be..9a9aa2c 100644 --- a/src/tables/gridkeys.rs +++ b/src/tables/gridkeys.rs @@ -69,6 +69,29 @@ pub(crate) fn land(merges: &[Merge], (row, col): (usize, usize)) -> (usize, usiz .map_or((row, col), |merge| merge.from) } +/// Where a key moves the cursor from a cell: past the far side of a merge it stands on, and onto the first cell of any merge it reaches. +pub(crate) fn stepped( + merges: &[Merge], + (row, col): (usize, usize), + key: &str, + (rows, cols): (usize, usize), +) -> Option<(usize, usize)> { + let far = merges + .iter() + .find(|merge| merge.from == (row, col)) + .map_or((row, col), |merge| merge.to); + let to = match key { + "up" => (row.saturating_sub(1), col), + "down" | "enter" => ((far.0 + 1).min(rows - 1), col), + "left" => (row, col.saturating_sub(1)), + "right" => (row, (far.1 + 1).min(cols - 1)), + "home" => (row, 0), + "end" => (row, cols - 1), + _ => return None, + }; + Some(land(merges, to)) +} + /// Pulls the cursor, its anchor and a fill back inside a grid of `size`; whether anything moved. pub(crate) fn fit(sheet: &mut Sheet, size: (usize, usize)) -> bool { let last = (size.0.saturating_sub(1), size.1.saturating_sub(1)); @@ -84,26 +107,22 @@ pub(crate) fn fit(sheet: &mut Sheet, size: (usize, usize)) -> bool { /// Tab and Shift-Tab: keep an open edit, then step across, staying inside the grid. pub(crate) fn across( - sheet: &Entity, - editor: &Entity, - (cols, back): (usize, bool), + (sheet, editor): (&Entity, &Entity), + (merges, size, back): (&[Merge], (usize, usize), bool), (metrics, frozen): (Metrics, (usize, usize)), window: &mut Window, cx: &mut App, ) { cx.stop_propagation(); + if size.0 == 0 || size.1 == 0 { + return; + } let (row, col) = sheet.read(cx).cursor; if sheet.read(cx).editing { editor.update(cx, |editing, cx| editing.finish(false, window, cx)); } - let to = ( - row, - if back { - col.saturating_sub(1) - } else { - (col + 1).min(cols.saturating_sub(1)) - }, - ); + let key = if back { "left" } else { "right" }; + let to = stepped(merges, (row, col), key, size).expect("left and right always step"); sheet.update(cx, |sheet, cx| { (sheet.cursor, sheet.anchor) = (to, to); reveal(sheet, metrics, frozen); @@ -221,21 +240,7 @@ pub(crate) fn keys( let stroke = &event.keystroke; let held = &stroke.modifiers; let (row, col) = sheet.read(cx).cursor; - let far = face - .merges - .iter() - .find(|merge| merge.from == (row, col)) - .map_or((row, col), |merge| merge.to); - let to = match stroke.key.as_str() { - "up" => Some((row.saturating_sub(1), col)), - "down" | "enter" => Some(((far.0 + 1).min(rows - 1), col)), - "left" => Some((row, col.saturating_sub(1))), - "right" => Some((row, (far.1 + 1).min(cols - 1))), - "home" => Some((row, 0)), - "end" => Some((row, cols - 1)), - _ => None, - }; - if let Some(to) = to.map(|to| land(&face.merges, to)) { + if let Some(to) = stepped(&face.merges, (row, col), stroke.key.as_str(), (rows, cols)) { cx.stop_propagation(); let stretch = held.shift && stroke.key != "enter"; sheet.update(cx, |sheet, cx| { diff --git a/src/tables/tests.rs b/src/tables/tests.rs index efd64a1..6bfbf0f 100644 --- a/src/tables/tests.rs +++ b/src/tables/tests.rs @@ -426,3 +426,21 @@ fn tab_steps_across_the_grid_and_keeps_an_edit(cx: &mut TestAppContext) { "tab moves right, and keeps an open edit first" ); } + +#[gpui::test] +fn tab_steps_past_a_merge_like_the_arrows(cx: &mut TestAppContext) { + grid_keys(cx); + let heard = Rc::new(RefCell::new(Vec::new())); + let seen = heard.clone(); + let (_, cx) = cx.add_window_view(|_, _| Merged(seen)); + settle(cx); + cx.simulate_click(point(px(146.0), px(42.0)), Modifiers::none()); + settle(cx); + cx.simulate_keystrokes("tab 9 enter"); + settle(cx); + assert_eq!( + *heard.borrow(), + [(0, 3, "9".to_string())], + "tab leaves the merge by its far side" + ); +}