diff --git a/contributors/people.toml b/contributors/people.toml index 5ce80fe59..f79ecec4c 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -103,3 +103,6 @@ real_name = "Helen Kwok" [people.dotCooCoo] real_name = "Robert Lee" sync_github_name = true + +[people.Plutonium-fream] +sync_github_name = true diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 82b252607..92187d5c0 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -72,6 +72,8 @@ pub struct Dialog { focus_initialized: bool, /// The formula box that takes references clicked or dragged on the grid (New Formatting Rule). pick: Option, + /// The linked cell's initial display text; unchanged text must not replace a number or formula. + original_link_text: Option, } /// A formula box in point mode, as in Excel: a click on the grid writes the cell's absolute @@ -165,6 +167,7 @@ impl Dialog { self.values.insert(pick.key.into(), json!(ed.text)); } + /// Creates a command-backed form with its initial field values. fn form(name: &str, title: &str, command: &'static str, fields: Vec, defaults: Json) -> Dialog { Dialog { name: name.into(), @@ -183,9 +186,11 @@ impl Dialog { back: None, focus_initialized: false, pick: None, + original_link_text: None, } } + /// Creates a dialog whose body handles its own controls instead of a standard form. pub(crate) fn custom(name: &str, title: &str, defaults: Json) -> Dialog { Dialog { name: name.into(), @@ -204,9 +209,11 @@ impl Dialog { back: None, focus_initialized: false, pick: None, + original_link_text: None, } } + /// Opens the named dialog with defaults from the active workbook, including a cell's stored hyperlink. pub fn open(app: &mut SheetApp, name: &str, params: Json) -> Option { let sel = app.session.active().map(|d| d.selection.current().a1()).unwrap_or_default(); let active = app.session.active().map(|d| d.selection.active.a1()).unwrap_or_default(); @@ -494,17 +501,30 @@ impl Dialog { json!({"url": url}), ) } - "insertLink" => Dialog::form( - "insertLink", - "Insert Hyperlink", - "insert.link", - vec![ - Text { key: "text", label: "Display:" }, - Text { key: "target", label: "Link to (web address, e-mail or Sheet!A1):" }, - Text { key: "tooltip", label: "ScreenTip:" }, - ], - json!({"target": "https://"}), - ), + "insertLink" => { + let mut d = Dialog::form( + "insertLink", + "Insert Hyperlink", + "insert.link", + vec![ + Text { key: "text", label: "Display:" }, + Text { key: "target", label: "Link to (web address, e-mail or Sheet!A1):" }, + Text { key: "tooltip", label: "ScreenTip:" }, + ], + json!({"target": "https://"}), + ); + if let Some(doc) = app.session.active() + && let Some(sheet) = doc.wb.active() + && let Some(link) = sheet.hyperlinks.get(&doc.selection.active) + { + let text = gridcraft_engine::display::cell_text(&doc.wb, sheet, doc.selection.active); + d.values.insert("target".into(), json!(link.target)); + d.values.insert("text".into(), json!(text)); + d.values.insert("tooltip".into(), json!(link.tooltip)); + d.original_link_text = Some(text); + } + d + } "sparkline" => Dialog::form( "sparkline", "Create Sparklines", diff --git a/crates/ui-egui/src/dialogs/show.rs b/crates/ui-egui/src/dialogs/show.rs index 07debbca6..071a94e5b 100644 --- a/crates/ui-egui/src/dialogs/show.rs +++ b/crates/ui-egui/src/dialogs/show.rs @@ -15,9 +15,17 @@ use super::{Dialog, Field, Json, Key, Map, RefPick, SheetApp, json, vec2}; /// Builds the command params from form values for dialogs that need translation. /// The parameters OK sends to the dialog's command: its field values, reshaped per dialog. +/// Unchanged hyperlink display text is omitted so its number or formula stays intact. pub(crate) fn build_params(d: &Dialog) -> Json { let mut v = Json::Object(d.values.clone()); match d.name.as_str() { + "insertLink" => { + if d.original_link_text.as_deref().is_some_and(|text| d.values.get("text").and_then(Json::as_str) == Some(text)) + && let Some(m) = v.as_object_mut() + { + m.remove("text"); + } + } "advancedFilter" => { if let Some(m) = v.as_object_mut() { m.remove("action"); diff --git a/crates/ui-egui/tests/it/hyperlink_dialog.rs b/crates/ui-egui/tests/it/hyperlink_dialog.rs new file mode 100644 index 000000000..63886305c --- /dev/null +++ b/crates/ui-egui/tests/it/hyperlink_dialog.rs @@ -0,0 +1,176 @@ +//! #2876: editing a stored hyperlink starts from its address, display text and ScreenTip. + +use egui_kittest::kittest::Queryable as _; +use gridcraft_engine::core::CellRef; +use gridcraft_engine::model::{Cell, Hyperlink}; +use gridcraft_ui_egui::SheetApp; +use gridcraft_ui_egui::control::{Outcome, handle}; +use serde_json::{Value as Json, json}; + +type Harness = egui_kittest::Harness<'static, SheetApp>; + +/// Runs the same dialog controls used by the public automation interface. +fn control(h: &mut Harness, method: &str, params: Json) -> Json { + let ctx = h.ctx.clone(); + let Outcome::Done(result) = handle(h.state_mut(), &ctx, method, ¶ms) else { panic!("unexpected screenshot response") }; + assert_eq!(result["ok"], true, "{method}: {result}"); + result +} + +/// A non-default active cell carrying a stored hyperlink and the requested input. +fn linked_harness(input: &str, target: &str) -> Harness { + let mut session = crate::support::blank_session(); + session.execute("cell.set", json!({"cell": "B2", "input": input})).unwrap(); + session.execute("insert.link", json!({"cell": "B2", "target": target, "tooltip": "Original tip"})).unwrap(); + session.execute("selection.set", json!({"cell": "B2"})).unwrap(); + crate::support::harness(session, 4) +} + +/// Captures both content and stored link metadata so cancellation and undo are exact. +fn linked_cell(h: &Harness) -> (Cell, Hyperlink) { + let sheet = h.state().session.active().unwrap().wb.active().unwrap(); + let at = CellRef::parse("B2").unwrap(); + (sheet.cells.get(at).cloned().unwrap(), sheet.hyperlinks.get(&at).cloned().unwrap()) +} + +/// Both relationship URLs and workbook locations prefill the same form. +#[test] +fn existing_external_and_internal_links_prefill_their_fields() { + for target in ["https://example.org/original?q=1", "Sheet1!F9"] { + let mut h = linked_harness("Original label", target); + control(&mut h, "ui.dialog", json!({"name": "insertLink"})); + h.run_steps(3); + let dialog = h.state().dialog.as_ref().unwrap(); + assert_eq!(dialog.values["target"], target); + assert_eq!(dialog.values["text"], "Original label"); + assert_eq!(dialog.values["tooltip"], "Original tip"); + assert!(h.query_by_label("Display:").is_some()); + assert!(h.query_by_label("ScreenTip:").is_some()); + } +} + +/// The keyboard entry point shares the prefilled form rather than resetting the address. +#[test] +fn ctrl_k_opens_the_existing_link_for_editing() { + let mut h = linked_harness("Original label", "https://example.org/original"); + h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::K); + h.run_steps(3); + let dialog = h.state().dialog.as_ref().unwrap(); + assert_eq!(dialog.name, "insertLink"); + assert_eq!(dialog.values["target"], "https://example.org/original"); +} + +/// Both the OK button and automation confirmation update one link and undo in one step. +#[test] +fn editing_a_link_confirms_and_undo_restores_it() { + for button in [false, true] { + let mut h = linked_harness("Original label", "https://example.org/original"); + let before = linked_cell(&h); + control(&mut h, "ui.dialog", json!({"name": "insertLink"})); + control(&mut h, "ui.dialog.set", json!({"field": "target", "value": "Sheet1!F9"})); + control(&mut h, "ui.dialog.set", json!({"field": "text", "value": "Updated label"})); + control(&mut h, "ui.dialog.set", json!({"field": "tooltip", "value": "Updated tip"})); + if button { + h.run_steps(4); + h.get_by_label(" OK ").click(); + h.run_steps(4); + } else { + control(&mut h, "ui.dialog.confirm", json!({})); + } + assert!(h.state().dialog.is_none()); + let (cell, link) = linked_cell(&h); + assert_eq!(cell.value.display(), "Updated label"); + assert_eq!(link.target, "Sheet1!F9"); + assert_eq!(link.tooltip.as_deref(), Some("Updated tip")); + assert!(!link.external); + h.state_mut().run("edit.undo", json!({})).unwrap(); + assert_eq!(linked_cell(&h), before); + } +} + +/// Closing a partially edited form leaves the existing link and cell untouched. +#[test] +fn cancelling_link_edits_keeps_the_original_cell_and_link() { + let mut h = linked_harness("Original label", "Sheet1!F9"); + let before = linked_cell(&h); + control(&mut h, "ui.dialog", json!({"name": "insertLink"})); + control(&mut h, "ui.dialog.set", json!({"field": "target", "value": "https://example.org/changed"})); + control(&mut h, "ui.dialog.set", json!({"field": "text", "value": "Changed label"})); + control(&mut h, "ui.dialog.set", json!({"field": "tooltip", "value": "Changed tip"})); + control(&mut h, "ui.dialog.cancel", json!({})); + assert!(h.state().dialog.is_none()); + assert_eq!(linked_cell(&h), before); +} + +/// Rendered forms keep linked numbers, formulas and number formats when their display text is unchanged. +#[test] +fn unchanged_display_text_preserves_number_and_formula_cells() { + for (input, format, expected) in [("42", "General", "42"), ("=6*7", "General", "42"), ("42", "0.00", "42.00"), ("=6*7", "0.00", "42.00")] { + let mut h = linked_harness(input, "https://example.org/original"); + h.state_mut().run("home.numberFormat", json!({"code": format})).unwrap(); + let (before, _) = linked_cell(&h); + let doc = h.state().session.active().unwrap(); + let display = gridcraft_engine::display::cell_text(&doc.wb, doc.wb.active().unwrap(), CellRef::parse("B2").unwrap()); + assert_eq!(display, expected, "formatted input {input}"); + control(&mut h, "ui.dialog", json!({"name": "insertLink"})); + h.run_steps(3); + assert_eq!(h.state().dialog.as_ref().unwrap().values["text"], display); + control(&mut h, "ui.dialog.set", json!({"field": "target", "value": "https://example.org/updated"})); + h.run_steps(3); + control(&mut h, "ui.dialog.confirm", json!({})); + let (after, link) = linked_cell(&h); + assert_eq!(after, before, "input {input}"); + assert_eq!(link.target, "https://example.org/updated"); + assert_eq!(link.tooltip.as_deref(), Some("Original tip")); + control(&mut h, "ui.dialog", json!({"name": "insertLink"})); + h.run_steps(3); + h.get_by_label(" OK ").click(); + h.run_steps(3); + assert!(h.state().dialog.is_none()); + assert_eq!(linked_cell(&h).0, before, "unedited OK for {input}"); + } +} + +/// A missing ScreenTip stays absent when only the address changes or OK is pressed unchanged. +#[test] +fn editing_a_link_without_a_tooltip_keeps_it_absent() { + let mut h = linked_harness("Original label", "https://example.org/original"); + h.state_mut().run("insert.link", json!({"target": "https://example.org/original"})).unwrap(); + assert_eq!(linked_cell(&h).1.tooltip, None); + control(&mut h, "ui.dialog", json!({"name": "insertLink"})); + h.run_steps(3); + let dialog = h.state().dialog.as_ref().unwrap(); + assert_eq!(dialog.values.get("tooltip").and_then(Json::as_str).unwrap_or_default(), ""); + control(&mut h, "ui.dialog.set", json!({"field": "target", "value": "https://example.org/updated"})); + h.run_steps(3); + control(&mut h, "ui.dialog.confirm", json!({})); + let (cell, link) = linked_cell(&h); + assert_eq!(cell.value.display(), "Original label"); + assert_eq!(link.target, "https://example.org/updated"); + assert_eq!(link.tooltip, None); + control(&mut h, "ui.dialog", json!({"name": "insertLink"})); + h.run_steps(3); + h.get_by_label(" OK ").click(); + h.run_steps(3); + assert!(h.state().dialog.is_none()); + assert_eq!(linked_cell(&h).1.tooltip, None); +} + +/// A new link still begins with the web-address default and uses its target as a blank cell's label. +#[test] +fn new_links_keep_the_existing_defaults() { + let mut h = crate::support::blank_harness(4); + control(&mut h, "ui.dialog", json!({"name": "insertLink"})); + h.run_steps(3); + let dialog = h.state().dialog.as_ref().unwrap(); + assert_eq!(dialog.values["target"], "https://"); + assert_eq!(dialog.values.get("text").and_then(Json::as_str).unwrap_or_default(), ""); + assert_eq!(dialog.values.get("tooltip").and_then(Json::as_str).unwrap_or_default(), ""); + control(&mut h, "ui.dialog.set", json!({"field": "target", "value": "https://example.org/new"})); + h.run_steps(3); + control(&mut h, "ui.dialog.confirm", json!({})); + let sheet = h.state().session.active().unwrap().wb.active().unwrap(); + let at = CellRef::parse("A1").unwrap(); + assert_eq!(sheet.value(at).display(), "https://example.org/new"); + assert_eq!(sheet.hyperlinks[&at].target, "https://example.org/new"); +} diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index ad5f7aa19..f55dcee70 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -51,6 +51,7 @@ mod header_autofit; mod header_column_drag; mod host_chrome; mod hyperlink_ctrl_click; +mod hyperlink_dialog; mod in_cell_editor_size; mod in_cell_editor_wrap; mod insert_copied_cells; diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 2329a2720..3a0b08ace 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-25 · **Change:** minor (AutoSum over a selected row puts its total to the right; earlier: macro interface on the Automate tab; ribbon catalog count 276 / 292) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-29 · **Change:** minor (editing a hyperlink loads its existing fields) · **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 @@ -152,6 +152,10 @@ a character (e.g. Ctrl+Alt+A for `ä`) never trigger a shortcut, Ctrl+letter one ## Dialogs +Insert Hyperlink loads an existing cell link's address, display text and ScreenTip for editing +(#2876). Keeping the display text unchanged preserves the cell's number or formula; Cancel +keeps the original link and an edit can be undone in one step. + Format Cells, Insert Function/Formula Builder, Find and Replace, Go To and Go To Special, Sort, Name Manager, Data Validation, Paste Special, Conditional Formatting manager, Page Setup, Goal Seek, Scenario Manager, Data Analysis, Forecast Sheet, Macros (macros, Automate scripts and a @@ -190,6 +194,7 @@ scrollbar bugs 2–4 · pixel/feel pass against observed Excel 7–12. | Date | Change | Summary | |---|---|---| +| 2026-10-29 | minor | Insert Hyperlink opens with an existing link's address, display text and ScreenTip (#2876). An address-only edit preserves the cell's content and formula; Cancel leaves the link unchanged, and Undo restores the edit | | 2026-10-25 | minor | AutoSum (Alt+=, Home › AutoSum) over a selected row puts its total to the right of the row, or in its last cell when that is empty, as in Excel; it wrote a one-cell total under each cell. A selection's empty last column takes each row's total and its empty last row each column's, with the block's total in the corner when both are empty | | 2026-10-25 | minor | Paste Name lays out only the names in view, so a workbook with thousands of names opens it at once; the arrow keys still scroll the chosen name into view. Go To matches a `Sheet2!Name` reference and hides a workbook name behind the active sheet's own name in any case and script (`Größe` = `GRÖSSE`), as formulas do. The two Page Layout view rows are one; the 0.7 in side margin used when Align with Page Margins is off is marked unverified | | 2026-10-24 | minor | Macro step editor: step list with breakpoints, drag to reorder and a command picker, an inspector with reference boxes, a debug bar and grid overlays |