From 1be90425e465878f8f447d4625860fbbe6bfdda6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Fri, 25 Sep 2026 22:22:02 -0400 Subject: [PATCH] T14c, T15 review fixes: Tab steps past merges, empty charts lay out nothing, flat domains widen by size, pointed parts let go of shrunk data, signed waterfall totals, sunburst children take their share, today stays on the timeline --- TASKS.md | 13 ++-- src/charts/calendar.rs | 5 +- src/charts/distribution.rs | 17 ++++- src/charts/flow.rs | 4 + src/charts/funnel.rs | 5 +- src/charts/geometry.rs | 31 ++++++++ src/charts/network.rs | 9 ++- src/charts/parallel.rs | 5 +- src/charts/radar.rs | 3 +- src/charts/relations.rs | 5 +- src/charts/scale.rs | 17 ++++- src/charts/schedule.rs | 7 +- src/charts/stats.rs | 17 ++++- src/charts/sunburst.rs | 23 ++++-- src/charts/targets.rs | 10 ++- src/charts/tests.rs | 146 ++++++++++++++++++++++++++++++++++++- src/charts/tiles.rs | 21 ++++-- src/tables/grid.rs | 22 +++--- src/tables/gridkeys.rs | 57 ++++++++------- src/tables/tests.rs | 18 +++++ 20 files changed, 358 insertions(+), 77 deletions(-) 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" + ); +}