diff --git a/apps/gridcraft/src/main.rs b/apps/gridcraft/src/main.rs index 8093205..4fbac1e 100644 --- a/apps/gridcraft/src/main.rs +++ b/apps/gridcraft/src/main.rs @@ -135,10 +135,22 @@ fn save_prefs(app: &SheetApp) { } } +/// Read the system clipboard through the shared handle, creating it on first use. +fn read_clipboard( + handle: &RefCell>, + read: impl FnOnce(&mut arboard::Clipboard) -> Result, +) -> Option { + let mut clipboard = handle.try_borrow_mut().ok()?; + if clipboard.is_none() { + *clipboard = arboard::Clipboard::new().ok(); + } + read(clipboard.as_mut()?).ok() +} + fn services() -> Services { // Keep the owner alive: X11/Wayland serve clipboard data from this handle. let clipboard: Rc>> = Rc::default(); - let paste_clipboard = clipboard.clone(); + let (html_clipboard, text_clipboard) = (clipboard.clone(), clipboard.clone()); Services { copy_html: Some(Box::new(move |html, text| { let mut clipboard = clipboard.borrow_mut(); @@ -147,13 +159,8 @@ fn services() -> Services { } clipboard.as_mut().ok_or("clipboard unavailable")?.set_html(html, Some(text)).map_err(|e| e.to_string()) })), - paste_html: Some(Box::new(move || { - let mut clipboard = paste_clipboard.try_borrow_mut().ok()?; - if clipboard.is_none() { - *clipboard = arboard::Clipboard::new().ok(); - } - clipboard.as_mut()?.get().html().ok() - })), + paste_html: Some(Box::new(move || read_clipboard(&html_clipboard, |clipboard| clipboard.get().html()))), + paste_text: Some(Box::new(move || read_clipboard(&text_clipboard, |clipboard| clipboard.get_text()))), pick_open: Some(Box::new(|| { rfd::FileDialog::new() .add_filter("Spreadsheets", &["xlsx", "xlsm", "xls", "xlsb", "ods", "csv", "tsv", "txt", "json"]) diff --git a/apps/gridcraft/src/native_menu.rs b/apps/gridcraft/src/native_menu.rs index b127543..f4b6e80 100644 --- a/apps/gridcraft/src/native_menu.rs +++ b/apps/gridcraft/src/native_menu.rs @@ -298,6 +298,8 @@ impl NativeMenu { app.open_dialog(name, params); } else if id == "edit.copy" || id == "edit.cut" { app.copy_to_clipboard(ctx, &id); + } else if id == "edit.paste" { + app.paste_from_clipboard(); } else { app.run_or_alert(&id, json!({})); } diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index d0eef85..ece47cd 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -84,7 +84,6 @@ pub struct GridState { pub last_border: String, pub last_fill: String, pub last_font_color: String, - pub system_clipboard: Option, pub trace: Option, pub renaming_tab: Option, pub rename_text: String, @@ -2651,6 +2650,7 @@ fn context_menu(app: &mut SheetApp, ui: &mut egui::Ui) { "edit.copy" | "edit.cut" => { app.copy_to_clipboard(ui.ctx(), id); } + "edit.paste" => app.paste_from_clipboard(), other => app.run_or_alert(other, json!({})), } } @@ -2893,6 +2893,7 @@ fn header_menu(app: &mut SheetApp, ui: &mut egui::Ui, was_open: bool) { "edit.copy" | "edit.cut" => { app.copy_to_clipboard(ui.ctx(), id); } + "edit.paste" => app.paste_from_clipboard(), other => app.run_or_alert(other, json!({})), } } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 0b19b25..8655089 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -179,6 +179,10 @@ pub struct Services { /// source spreadsheet recorded as numbers stay numbers ("Rp10,000.00" → 10000). `None` from /// the host pastes the plain text alone. pub paste_html: Option Option>>, + /// Read the system clipboard's plain text for the ribbon and menu Paste commands (Cmd/Ctrl+V + /// gets it from egui's `Event::Paste`). `None` from the host, or no service (web), pastes + /// GridCraft's own clipboard. + pub paste_text: Option Option>>, /// Web: deliver a file to the user (name, bytes). pub download: Option>, /// Web: start an asynchronous file pick; the bytes arrive later in `inbox`. @@ -392,6 +396,17 @@ impl SheetApp { self.run_or_alert("edit.paste", params); } + /// Ribbon and menu Paste: paste the text on the system clipboard, as Cmd/Ctrl+V does. With no + /// text there, or no host clipboard (web), GridCraft's own clipboard is pasted. + pub fn paste_from_clipboard(&mut self) { + let text = self.services.paste_text.as_mut().and_then(|read| read()).filter(|t| !t.is_empty()); + match text { + // Same line endings as egui's `Event::Paste` text. + Some(text) => self.paste_external(text.replace("\r\n", "\n")), + None => self.run_or_alert("edit.paste", json!({})), + } + } + /// Web: `file.exportPdf` without a path returns the PDF as base64; hand it to the browser as a download. fn deliver_pdf(&self, result: &Json) { if result.get("path").is_some() { diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index e8291ac..e02316c 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -483,6 +483,8 @@ fn menu_items(app: &mut SheetApp, ui: &mut Ui, items: &[(&str, &str, serde_json: if response.clicked() { if let Some(d) = id.strip_prefix("dialog:") { app.open_dialog(d, p.clone()); + } else if *id == "edit.paste" { + app.paste_from_clipboard(); } else { act(app, id, p.clone()); } @@ -495,17 +497,14 @@ fn home(app: &mut SheetApp, ui: &mut Ui) { let st = active_style(app); // Clipboard let paste = big_button(ui, Icon::Paste, "Paste", "Paste (⌘V)", true); + paste.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::Button, ui.is_enabled(), "Paste")); for sequence in keytip_table::group_extension(app.keytips.prefix().unwrap_or_default(), &["edit.paste", "edit.pasteSpecial", "dialog:pasteSpecial"]) { keytip(app, ui, sequence, paste.rect.center_bottom()); } if paste.clicked() { - if let Some(text) = app.grid.system_clipboard.take() { - app.paste_external(text); - } else { - act(app, "edit.paste", json!({})); - } + app.paste_from_clipboard(); } let paste_keys = matches!(app.keytips.prefix(), Some("HV" | "E")); // Keep a keyboard-opened menu anchored to the button when a pointer click diff --git a/crates/ui-egui/tests/clipboard.rs b/crates/ui-egui/tests/clipboard.rs index 71fe336..1e1181b 100644 --- a/crates/ui-egui/tests/clipboard.rs +++ b/crates/ui-egui/tests/clipboard.rs @@ -3,9 +3,12 @@ use std::cell::RefCell; use std::rc::Rc; -use egui::{Event, OutputCommand}; +use egui::accesskit::Role; +use egui::{Event, Modifiers, OutputCommand, PointerButton}; +use egui_kittest::kittest::Queryable as _; use gridcraft_engine::Session; -use gridcraft_ui_egui::{Services, SheetApp}; +use gridcraft_engine::core::{CellRef, Value}; +use gridcraft_ui_egui::{Services, SheetApp, grid}; use serde_json::json; fn harness(services: Services) -> egui_kittest::Harness<'static, SheetApp> { @@ -116,3 +119,131 @@ fn copying_multiple_areas_copies_lined_up_areas_and_explains_the_rest() { assert_eq!(h.state().message.as_ref().map(|(_, text)| text.as_str()), Some("This action won't work on multiple selections.")); assert_eq!(h.state().session.clipboard.as_ref().map(|c| c.areas.len()), Some(2), "the earlier copy stays"); } + +#[test] +fn clicking_the_ribbon_paste_button_or_its_menu_does_not_freeze() { + // #273: a click on Paste deadlocked the UI thread (a click-state query inside the egui context + // lock), so the frame never returned. Run the clicks on a worker and fail on a timeout instead of + // hanging the test run if that comes back. + let (done, finished) = std::sync::mpsc::channel(); + let worker = std::thread::spawn(move || { + let cell = value_at; + let mut h = harness(Services::default()); + h.state_mut().run("edit.copy", json!({"range": "A1:B2"})).unwrap(); + h.state_mut().run("selection.set", json!({"cell": "D4"})).unwrap(); + // No host clipboard here (as on the web): GridCraft's own copy is pasted. + h.get_by_role_and_label(Role::Button, "Paste").click(); + h.run_steps(3); + assert_eq!(cell(&h, "D4"), Some(Value::from("Name"))); + assert_eq!(cell(&h, "E5"), Some(Value::number(42.0))); + + // The secondary click opens the paste options menu; its items paste too. + h.state_mut().run("selection.set", json!({"cell": "G1"})).unwrap(); + h.get_by_role_and_label(Role::Button, "Paste").click_secondary(); + h.run_steps(3); + h.get_by_label("Paste Values").click(); + h.run_steps(3); + assert_eq!(cell(&h, "G1"), Some(Value::from("Name"))); + assert_eq!(cell(&h, "H2"), Some(Value::number(42.0))); + let _ = done.send(()); + }); + match finished.recv_timeout(std::time::Duration::from_secs(60)) { + Ok(()) => assert!(worker.join().is_ok()), + Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => { + if let Err(panic) = worker.join() { + std::panic::resume_unwind(panic); + } + } + Err(std::sync::mpsc::RecvTimeoutError::Timeout) => panic!("clicking Paste froze the UI thread"), + } +} + +type Harness = egui_kittest::Harness<'static, SheetApp>; + +fn value_at(h: &Harness, a1: &str) -> Option { + let at = gridcraft_engine::core::CellRef::parse(a1).unwrap(); + h.state().session.active().and_then(|d| d.wb.active().map(|s| s.value(at))) +} + +/// A host clipboard holding `text`, and how many times the app read it. +fn system_clipboard(text: &str) -> (Rc>, Services) { + let clipboard = Rc::new(RefCell::new((text.to_string(), 0))); + let read = clipboard.clone(); + let services = Services { + paste_text: Some(Box::new(move || { + let mut clipboard = read.borrow_mut(); + clipboard.1 += 1; + Some(clipboard.0.clone()) + })), + ..Default::default() + }; + (clipboard, services) +} + +#[test] +fn ribbon_paste_pastes_the_system_clipboard_like_cmd_v() { + // Text copied in another app: the Paste button pastes it, as Cmd/Ctrl+V does. + let (clipboard, services) = system_clipboard("x\t1\r\ny\t2\r\n"); + let mut h = harness(services); + h.state_mut().run("selection.set", json!({"cell": "D4"})).unwrap(); + h.get_by_role_and_label(Role::Button, "Paste").click(); + h.run_steps(2); + assert_eq!(clipboard.borrow().1, 1, "one clipboard read per click"); + assert_eq!(value_at(&h, "D4"), Some(Value::from("x"))); + assert_eq!(value_at(&h, "E4"), Some(Value::number(1.0))); + assert_eq!(value_at(&h, "D5"), Some(Value::from("y"))); + assert_eq!(value_at(&h, "E5"), Some(Value::number(2.0))); + assert!(h.state().message.is_none(), "{:?}", h.state().message); + + // The clipboard text is GridCraft's own copy: the copy is pasted with its formulas. + h.state_mut().run("cell.set", json!({"cell": "A3", "input": "=B2*2"})).unwrap(); + let copied = h.state_mut().run("edit.copy", json!({"range": "A3"})).unwrap(); + clipboard.borrow_mut().0 = copied["text"].as_str().unwrap().to_string(); + h.state_mut().run("selection.set", json!({"cell": "B9"})).unwrap(); + h.get_by_role_and_label(Role::Button, "Paste").click(); + h.run_steps(2); + let sh = h.state().session.active().unwrap().wb.active().unwrap(); + assert_eq!(sh.cell(CellRef::parse("B9").unwrap()).unwrap().formula.as_ref().unwrap().text, "C8*2"); + + // Nothing on the system clipboard (an image, say): GridCraft's own copy is pasted. + clipboard.borrow_mut().0.clear(); + h.state_mut().run("selection.set", json!({"cell": "B12"})).unwrap(); + h.get_by_role_and_label(Role::Button, "Paste").click(); + h.run_steps(2); + let sh = h.state().session.active().unwrap().wb.active().unwrap(); + assert_eq!(sh.cell(CellRef::parse("B12").unwrap()).unwrap().formula.as_ref().unwrap().text, "C11*2"); +} + +#[test] +fn paste_menu_items_paste_the_system_clipboard() { + let (clipboard, services) = system_clipboard("from menu"); + let mut h = harness(services); + let ribbon = h.get_by_role_and_label(Role::Button, "Paste").rect(); + let click_menu_paste = |h: &Harness| h.get_all_by_role_and_label(Role::Button, "Paste").find(|n| n.rect() != ribbon).unwrap().click(); + + // Home › Paste options menu › Paste. + h.state_mut().run("selection.set", json!({"cell": "D4"})).unwrap(); + h.get_by_role_and_label(Role::Button, "Paste").click_secondary(); + h.run_steps(2); + click_menu_paste(&h); + h.run_steps(2); + assert_eq!(value_at(&h, "D4"), Some(Value::from("from menu"))); + + // The cell's context menu › Paste. + clipboard.borrow_mut().0 = "from cell menu".into(); + h.state_mut().run("selection.set", json!({"cell": "F6"})).unwrap(); + let app = h.state(); + let sh = app.session.active().unwrap().wb.active().unwrap(); + let geo = grid::Geo::new(sh, app.grid.rect.unwrap(), app.view().scroll); + let pos = geo.cell_rect(sh, CellRef::parse("F6").unwrap()).center(); + h.input_mut().events.push(Event::PointerMoved(pos)); + for pressed in [true, false] { + h.input_mut().events.push(Event::PointerButton { pos, button: PointerButton::Secondary, pressed, modifiers: Modifiers::NONE }); + h.run_steps(2); + } + assert!(h.state().grid.context_menu.is_some()); + click_menu_paste(&h); + h.run_steps(2); + assert_eq!(value_at(&h, "F6"), Some(Value::from("from cell menu"))); + assert_eq!(clipboard.borrow().1, 2); +} diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 10abcbc..065f335 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -1,6 +1,6 @@ # UI parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-11 · **Change:** minor (AutoFit reserves filter dropdown space, observed against Windows Excel) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-12 · **Change:** minor (ribbon and menu Paste paste the system clipboard like Cmd/Ctrl+V; regression test for the Paste button freeze, #273; Paste button gets an accessible name) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Interaction, controls and feel, against Excel. Part of [target-app-parity.md](target-app-parity.md). **UI/UX fidelity: ~55% ready, 50–80 h** (estimated). Ribbon command coverage is measured @@ -160,6 +160,7 @@ scrollbar bugs 2–4 · pixel/feel pass against observed Excel 7–12. | Date | Change | Summary | |---|---|---| +| 2026-10-12 | minor | Paste from the ribbon (button and options menu), the cell and header context menus and the macOS menu bar's Edit menu pastes what is on the system clipboard, as Cmd/Ctrl+V does, instead of only GridCraft's own copy (the web build, which can't read the clipboard on request, still pastes GridCraft's copy). The desktop app reads the clipboard off the UI thread and gives up after 5 s, so a clipboard source that never answers can't freeze it. Regression test for the Paste button click freeze in 0.5.0 (#273, fixed by #272); the Paste button gets an accessible "Paste" name for screen readers | | 2026-10-11 | minor | AutoFit accounts for table and ordinary filter dropdowns, including low zoom; header text excludes the button area | | 2026-10-11 | minor | Outline band for grouped rows and columns, Show / Hide Detail on the Data tab, numpad navigation keys in shortcuts (#373) | | 2026-10-11 | minor | Shift+Space / Ctrl+Space no longer start an edit; Control+Space selects the column on Mac (#338) |