From 1a49d18dd1aed1dbca22f37674a5fcb08524ae48 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Mon, 12 Oct 2026 18:02:02 +0200 Subject: [PATCH] Comments pane: one Show Comments button, and the pane keeps its width (fixes #710) --- crates/ui-egui/src/panes.rs | 34 +++++---- crates/ui-egui/src/ribbon.rs | 3 - crates/ui-egui/tests/comments_pane.rs | 105 ++++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 16 deletions(-) create mode 100644 crates/ui-egui/tests/comments_pane.rs diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index 3100cfe..aaa9bec 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -17,7 +17,10 @@ pub fn show(app: &mut SheetApp, ui: &mut Ui) { } let Some(pane) = app.grid.pane.clone().or_else(|| app.session.comments_pane.then(|| "comments".to_string())) else { return }; let t = Tokens::get(ui.ctx()); - egui::Panel::right("task_pane").default_size(300.0).resizable(true).frame(egui::Frame::NONE.fill(t.ribbon).inner_margin(10)).show(ui, |ui| { + // Capped at half the window so a pane can never cover the sheet, whatever its content asks for. + let max = (ui.available_width() / 2.0).max(300.0); + let panel = egui::Panel::right("task_pane").default_size(300.0).max_size(max).resizable(true); + panel.frame(egui::Frame::NONE.fill(t.ribbon).inner_margin(10)).show(ui, |ui| { ui.horizontal(|ui| { let title = match pane.as_str() { "comments" => "Comments", @@ -62,7 +65,7 @@ fn comments(app: &mut SheetApp, ui: &mut Ui) { .stroke(egui::Stroke::new(1.0, Color32::from_gray(215))) .show(ui, |ui| { ui.set_width(ui.available_width()); - ui.horizontal(|ui| { + ui.horizontal_wrapped(|ui| { if ui.link(egui::RichText::new(&cell).strong()).clicked() { let _ = app.run("selection.set", json!({"cell": cell})); app.grid.ensure_visible = true; @@ -78,18 +81,23 @@ fn comments(app: &mut SheetApp, ui: &mut Ui) { } let key = egui::Id::new(("reply", &cell)); let mut draft: String = ui.ctx().data_mut(|m| m.get_temp(key)).unwrap_or_default(); + // Buttons first, right to left, so the reply field gets exactly the width they leave: + // a field sized by a guess wider than the row pushes the resizable pane wider every frame. ui.horizontal(|ui| { - let r = ui.add(egui::TextEdit::singleline(&mut draft).hint_text("Reply…").desired_width(ui.available_width() - 120.0)); - if (r.lost_focus() && ui.input(|i| i.key_pressed(egui::Key::Enter)) || ui.small_button("Send").clicked()) && !draft.is_empty() { - app.run_or_alert("review.replyComment", json!({"cell": cell, "text": draft})); - draft.clear(); - } - if ui.small_button(if c.resolved { "Reopen" } else { "Resolve" }).clicked() { - app.run_or_alert("review.resolveComment", json!({"cell": cell, "resolved": !c.resolved})); - } - if ui.small_button("Delete").clicked() { - app.run_or_alert("review.deleteComment", json!({"cell": cell})); - } + ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + if ui.small_button("Delete").clicked() { + app.run_or_alert("review.deleteComment", json!({"cell": cell})); + } + if ui.small_button(if c.resolved { "Reopen" } else { "Resolve" }).clicked() { + app.run_or_alert("review.resolveComment", json!({"cell": cell, "resolved": !c.resolved})); + } + let send = ui.small_button("Send").clicked(); + let r = ui.add(egui::TextEdit::singleline(&mut draft).hint_text("Reply…").desired_width(ui.available_width())); + if (r.lost_focus() && ui.input(|i| i.key_pressed(egui::Key::Enter)) || send) && !draft.is_empty() { + app.run_or_alert("review.replyComment", json!({"cell": cell, "text": draft})); + draft.clear(); + } + }); }); ui.ctx().data_mut(|m| m.insert_temp(key, draft)); }); diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index e8291ac..d25dfa1 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -2022,9 +2022,6 @@ fn review(app: &mut SheetApp, ui: &mut Ui) { if __k.clicked() { app.open_dialog("comment", json!({"threaded": true})); } - if big_button(ui, Icon::Comment, "Show\nComments", "Comments pane", false).clicked() { - app.grid.pane = Some("comments".into()); - } let __k = big_button(ui, Icon::Delete, "Delete", "Delete Comment", false); control_keytip(app, ui, &__k, "review.deleteComment"); if __k.clicked() { diff --git a/crates/ui-egui/tests/comments_pane.rs b/crates/ui-egui/tests/comments_pane.rs new file mode 100644 index 0000000..fe61508 --- /dev/null +++ b/crates/ui-egui/tests/comments_pane.rs @@ -0,0 +1,105 @@ +//! The Comments pane: one Show Comments button, and a pane that keeps its width (#710). + +use egui::{Event, Modifiers, PointerButton, Pos2, pos2, vec2}; +use egui_kittest::kittest::Queryable; +use gridcraft_engine::Session; +use gridcraft_ui_egui::SheetApp; +use serde_json::json; + +type Harness = egui_kittest::Harness<'static, SheetApp>; + +const WIDTH: f32 = 1200.0; + +fn harness() -> Harness { + let mut session = Session::new(); + session.new_workbook(); + let mut h = egui_kittest::Harness::builder().with_step_dt(1.0 / 60.0).with_size(vec2(WIDTH, 800.0)).build_ui_state( + |ui, app: &mut SheetApp| { + app.logic(ui.ctx()); + app.ui(ui); + }, + SheetApp::new(session, Default::default()), + ); + h.run_steps(4); + h +} + +fn grid_right(h: &Harness) -> f32 { + h.state().grid.rect.unwrap().right() +} + +/// A comment whose author line and reply row are wide enough to push the pane if they don't wrap. +fn open_pane_on_long_comment(h: &mut Harness) { + let text = "A long comment that should wrap inside the pane instead of pushing it wider. ".repeat(6); + let author = "Someone With A Rather Long Display Name, Finance Department, Head Office"; + h.state_mut().run("review.newComment", json!({"cell": "B2", "text": text, "author": author})).unwrap(); + h.state_mut().run("review.showComments", json!({"on": true})).unwrap(); + h.run_steps(3); +} + +fn drag(h: &mut Harness, from: Pos2, to: Pos2) { + let press = |pos, pressed| Event::PointerButton { pos, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE }; + h.input_mut().events.extend([Event::PointerMoved(from), press(from, true)]); + h.step(); + for i in 1..=6 { + let t = i as f32 / 6.0; + h.input_mut().events.push(Event::PointerMoved(from + (to - from) * t)); + h.step(); + } + h.input_mut().events.push(press(to, false)); + h.run_steps(3); +} + +#[test] +fn the_pane_does_not_grow_on_its_own_and_keeps_a_manual_resize() { + let mut h = harness(); + // Between the grid and the right edge of the window, before any pane is open. + let gutter = WIDTH - grid_right(&h); + open_pane_on_long_comment(&mut h); + let opened = grid_right(&h); + assert!(WIDTH - opened < WIDTH / 2.0, "the pane opens at its default width, got {}", WIDTH - opened); + h.run_steps(60); + assert_eq!(grid_right(&h), opened, "the pane must not widen itself frame after frame"); + + // Drag the pane's left edge 150px to the left: the new width stays. + let edge = pos2(opened + gutter, 450.0); + drag(&mut h, edge, edge - vec2(150.0, 0.0)); + let resized = grid_right(&h); + // The grab point is a few pixels off the exact edge, so allow some slack on the 150px. + assert!(opened - resized > 120.0, "the drag widens the pane, grid edge {opened} -> {resized}"); + h.run_steps(60); + assert_eq!(grid_right(&h), resized, "a manual resize must hold"); + + // Dragged almost to the window's left edge, the pane stops at half the window. + let edge = pos2(resized + gutter, 450.0); + drag(&mut h, edge, pos2(60.0, 450.0)); + assert!(grid_right(&h) > WIDTH / 2.0 - gutter - 30.0, "the pane is capped at half the window, grid edge {}", grid_right(&h)); +} + +/// How many separate ribbon buttons show `tip` when hovered, scanning the big-button row. +fn buttons_with_tip(h: &mut Harness, tip: &str) -> usize { + h.ctx.all_styles_mut(|s| s.interaction.tooltip_delay = 0.0); + let (mut count, mut inside) = (0, false); + let mut x = 2.0; + while x < WIDTH { + h.input_mut().events.push(Event::PointerMoved(pos2(x, 120.0))); + h.run_steps(6); + let shown = h.query_all_by_value(tip).next().is_some(); + if shown && !inside { + count += 1; + } + inside = shown; + x += 8.0; + } + count +} + +#[test] +fn review_has_a_single_show_comments_button() { + let mut h = harness(); + h.state_mut().ui.ribbon_tab = "Review".into(); + h.run_steps(2); + // The scan has to find a known single button, or a count of one below would prove nothing. + assert_eq!(buttons_with_tip(&mut h, "New Comment"), 1); + assert_eq!(buttons_with_tip(&mut h, "Comments pane"), 1); +}