diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 5b22184f5..83133c01c 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -262,21 +262,36 @@ fn clear(s: &mut Session, p: &Json, what: &str) -> Result { }) }) }; + // Notes and links aren't cell contents, so the cell comparison that guards a protected sheet + // doesn't see them go: a locked cell's are refused here, stored cell or not. + if matches!(what, "all" | "comments" | "hyperlinks") { + let d = s.doc()?; + if let Some(sh) = d.wb.sheet(sheet).filter(|sh| sh.is_protected()) { + for &whole in &ranges { + let skip = filtered_rows(sh, whole); + let notes = matches!(what, "all" | "comments").then(|| sh.comments.keys()).into_iter().flatten(); + let links = matches!(what, "all" | "hyperlinks").then(|| sh.hyperlinks.keys()).into_iter().flatten(); + for &c in notes.chain(links).filter(|c| whole.contains(**c) && !skip.contains(&c.row)) { + if d.wb.styles.get(sh.style_id(c)).protection.locked + && let Some(m) = locked_cell_error(sh, c, &d.unlocked_ranges) + { + return Err(EngineError::Other(m)); + } + } + } + } + } edit(s, |cx| { - for r in &ranges { - let r = match cx.wb.sheet(sheet).and_then(|sh| sh.used_range()) { - Some(u) => match r.intersection(&u) { - Some(x) => x, - None => continue, - }, - None => continue, + for &whole in &ranges { + // Cells are only stored inside the used range. Notes and links are kept apart from + // them, so they are cleared over the whole range: one from a file can sit on a cell + // with nothing stored in it. + let used = cx.wb.sheet(sheet).and_then(|sh| sh.used_range()).and_then(|u| whole.intersection(&u)); + let skip = cx.wb.sheet(sheet).map(|sh| filtered_rows(sh, whole)).unwrap_or_default(); + let cells: Vec<(CellRef, Cell)> = match (cx.wb.sheet(sheet), used) { + (Some(sh), Some(r)) => sh.cells.iter_range(r).filter(|(c, _)| !skip.contains(&c.row)).map(|(c, x)| (c, x.clone())).collect(), + _ => Vec::new(), }; - let skip = cx.wb.sheet(sheet).map(|sh| filtered_rows(sh, r)).unwrap_or_default(); - let cells: Vec<(CellRef, Cell)> = cx - .wb - .sheet(sheet) - .map(|sh| sh.cells.iter_range(r).filter(|(c, _)| !skip.contains(&c.row)).map(|(c, x)| (c, x.clone())).collect()) - .unwrap_or_default(); // Report every stored cell we touch, even beyond a million coordinates in a sparse // selection. Removed formula anchors must also reach calc so their spills disappear. cx.changed.extend(cells.iter().map(|(c, _)| (sheet, *c))); @@ -296,6 +311,18 @@ fn clear(s: &mut Session, p: &Json, what: &str) -> Result { } } } + let cleared = |c: CellRef| whole.contains(c) && !skip.contains(&c.row); + // A removed note or link changes its cell even with nothing stored there. + let metadata: Vec = cx + .wb + .sheet(sheet) + .map(|sh| { + let notes = matches!(what, "all" | "comments").then(|| sh.comments.keys()).into_iter().flatten(); + let links = matches!(what, "all" | "hyperlinks").then(|| sh.hyperlinks.keys()).into_iter().flatten(); + notes.chain(links).copied().filter(|c| cleared(*c)).collect() + }) + .unwrap_or_default(); + cx.changed.extend(metadata.into_iter().map(|c| (sheet, c))); let sh = cx.sheet_mut(sheet)?; for (c, mut cell) in cells { match what { @@ -312,14 +339,15 @@ fn clear(s: &mut Session, p: &Json, what: &str) -> Result { _ => {} } } - let cleared = |c: CellRef| r.contains(c) && !skip.contains(&c.row); if matches!(what, "all" | "comments") { sh.comments.retain(|c, _| !cleared(*c)); } if matches!(what, "all" | "hyperlinks") { sh.hyperlinks.retain(|c, _| !cleared(*c)); } - if matches!(what, "all" | "formats") { + if let Some(r) = used + && matches!(what, "all" | "formats") + { let mut unmerged = false; sh.merges.retain(|m| { let keep = !r.contains_range(m) || skip.range(m.start.row..=m.end.row).next().is_some(); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 4d0cabd17..fe42b6977 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -5476,3 +5476,99 @@ fn workbook_display_options_are_commands() { assert!(view(&s).show_vertical_scroll); assert_eq!(s.execute("view.showSheetTabs", json!({})).unwrap()["on"], json!(true), "toggles back"); } + +/// A link from a file can sit on a cell with nothing stored in it, past the used range (or on a +/// sheet with no cells at all). Clear Hyperlinks, and Remove Hyperlink in the cell menu, must +/// still remove it. +#[test] +fn clear_hyperlinks_removes_links_on_cells_with_nothing_stored() { + let mut s = s(); + s.execute("cell.set", json!({"cell": "A1", "input": "x"})).unwrap(); + let link = || gridcraft_model::Hyperlink { target: "https://example.com".into(), tooltip: None, external: true }; + let (a1, c5) = (CellRef::parse("A1").unwrap(), CellRef::parse("C5").unwrap()); + std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb).sheet_mut(0).unwrap().hyperlinks.insert(c5, link()); + s.execute("edit.clearHyperlinks", json!({"range": "C5"})).unwrap(); + assert!(!s.doc().unwrap().wb.active().unwrap().hyperlinks.contains_key(&c5), "the link past the used range is removed"); + + // No cells at all. + s.execute("edit.clearAll", json!({"range": "A1"})).unwrap(); + std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb).sheet_mut(0).unwrap().hyperlinks.insert(a1, link()); + s.execute("edit.clearHyperlinks", json!({"range": "A1"})).unwrap(); + assert!(!s.doc().unwrap().wb.active().unwrap().hyperlinks.contains_key(&a1), "the link on an empty sheet is removed"); +} + +/// Notes and links aren't cell contents, so the cell comparison that guards a protected sheet +/// doesn't see them go: Clear Hyperlinks and Clear Comments refuse locked cells themselves, +/// stored or not. +#[test] +fn clearing_links_and_notes_respects_sheet_protection() { + let mut s = s(); + s.execute("cell.set", json!({"cell": "A1", "input": "x"})).unwrap(); + let link = || gridcraft_model::Hyperlink { target: "https://example.com".into(), tooltip: None, external: true }; + let (a1, c5) = (CellRef::parse("A1").unwrap(), CellRef::parse("C5").unwrap()); + { + let sh = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb).sheet_mut(0).unwrap(); + sh.hyperlinks.insert(a1, link()); + sh.hyperlinks.insert(c5, link()); + } + s.execute("review.newNote", json!({"cell": "C5", "text": "note", "author": "Example"})).unwrap(); + s.execute("review.protectSheet", json!({})).unwrap(); + for (cmd, range) in [("edit.clearHyperlinks", "A1"), ("edit.clearHyperlinks", "C5"), ("edit.clearComments", "C5")] { + assert!(s.execute(cmd, json!({"range": range})).is_err(), "{cmd} on locked {range}"); + } + let sh = s.doc().unwrap().wb.active().unwrap().clone(); + assert!(sh.hyperlinks.contains_key(&a1) && sh.hyperlinks.contains_key(&c5) && sh.comments.contains_key(&c5), "nothing removed"); + + s.execute("review.unprotectSheet", json!({})).unwrap(); + s.execute("edit.clearHyperlinks", json!({"range": "A1:C5"})).unwrap(); + s.execute("edit.clearComments", json!({"range": "C5"})).unwrap(); + let sh = s.doc().unwrap().wb.active().unwrap().clone(); + assert!(sh.hyperlinks.is_empty() && sh.comments.is_empty()); +} + +/// A removed link or note is a change to its cell even when nothing is stored there, so a +/// recorded macro's footprint lists it. +#[test] +fn clearing_links_and_notes_records_their_cells() { + let mut s = s(); + let c5 = CellRef::parse("C5").unwrap(); + std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb) + .sheet_mut(0) + .unwrap() + .hyperlinks + .insert(c5, gridcraft_model::Hyperlink { target: "https://example.com".into(), tooltip: None, external: true }); + s.execute("review.newNote", json!({"cell": "D7", "text": "note", "author": "Example"})).unwrap(); + s.doc_mut().unwrap().touched = Some(Vec::new()); + s.execute("edit.clearHyperlinks", json!({"range": "C5"})).unwrap(); + s.execute("edit.clearComments", json!({"range": "D7"})).unwrap(); + let touched: Vec = s.doc().unwrap().touched.clone().unwrap_or_default().into_iter().map(|(_, c)| c.a1()).collect(); + assert!(touched.contains(&"C5".to_string()) && touched.contains(&"D7".to_string()), "{touched:?}"); +} + +/// The locked-cell check for notes and links keeps Allow Edit Ranges: a range without a password +/// allows them at once, one with a password once it's unlocked in this session. +#[test] +fn clearing_links_and_notes_keeps_allow_edit_ranges() { + let mut s = s(); + s.execute("cell.set", json!({"cell": "A1", "input": "x"})).unwrap(); + let link = || gridcraft_model::Hyperlink { target: "https://example.com".into(), tooltip: None, external: true }; + let (a1, c5) = (CellRef::parse("A1").unwrap(), CellRef::parse("C5").unwrap()); + { + let sh = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb).sheet_mut(0).unwrap(); + sh.hyperlinks.insert(a1, link()); + sh.hyperlinks.insert(c5, link()); + } + s.execute("review.newNote", json!({"cell": "C5", "text": "note", "author": "Example"})).unwrap(); + // Ranges are added before protecting: a protected sheet refuses `add`. + s.execute("review.allowEditRanges", json!({"action": "add", "title": "Open", "range": "A1"})).unwrap(); + s.execute("review.allowEditRanges", json!({"action": "add", "title": "Kept", "range": "C5", "password": "example"})).unwrap(); + s.execute("review.protectSheet", json!({})).unwrap(); + + s.execute("edit.clearHyperlinks", json!({"range": "A1"})).unwrap(); + assert!(s.execute("edit.clearComments", json!({"range": "C5"})).is_err(), "a password range stays locked until unlocked"); + s.execute("review.allowEditRanges", json!({"action": "unlock", "title": "Kept", "password": "example"})).unwrap(); + s.execute("edit.clearHyperlinks", json!({"range": "C5"})).unwrap(); + s.execute("edit.clearComments", json!({"range": "C5"})).unwrap(); + let sh = s.doc().unwrap().wb.active().unwrap().clone(); + assert!(sh.hyperlinks.is_empty() && sh.comments.is_empty()); +} diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 33be245a5..ec61c5c56 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -564,6 +564,8 @@ impl Dialog { d.values.insert("text".into(), json!(text)); d.values.insert("tooltip".into(), json!(link.tooltip)); d.original_link_text = Some(text); + // As in Excel, the dialog on a linked cell edits that link. + d.title = "Edit Hyperlink".into(); } d } diff --git a/crates/ui-egui/src/grid/menus.rs b/crates/ui-egui/src/grid/menus.rs index b05a04bc7..0e7872334 100644 --- a/crates/ui-egui/src/grid/menus.rs +++ b/crates/ui-egui/src/grid/menus.rs @@ -129,6 +129,9 @@ pub(super) fn context_menu(app: &mut SheetApp, ui: &mut egui::Ui) { egui::Frame::popup(ui.style()).show(ui, |ui| { ui.set_min_width(200.0); let insert_copied = insert_copied_item(app); + // On a cell with a link Excel offers to edit, open or remove it (#2120). + let link = + app.session.active().and_then(|d| d.wb.active().and_then(|sh| sh.hyperlinks.get(&d.selection.active)).map(|l| l.target.clone())); let mut items: Vec<(&str, &str)> = vec![ ("Cut", "edit.cut"), ("Copy", "edit.copy"), @@ -154,6 +157,14 @@ pub(super) fn context_menu(app: &mut SheetApp, ui: &mut egui::Ui) { if let Some(item) = insert_copied { items.insert(6, item); } + if link.is_some() + && let Some(i) = items.iter().position(|(_, id)| *id == "ui:insertLink") + { + items.splice( + i..=i, + [("Edit Hyperlink…", "ui:insertLink"), ("Open Hyperlink", "ui:openLink"), ("Remove Hyperlink", "edit.clearHyperlinks")], + ); + } if show_details_offered(app) && let Some(i) = items.iter().position(|(_, id)| *id == "data.sortDescending") { @@ -178,6 +189,11 @@ pub(super) fn context_menu(app: &mut SheetApp, ui: &mut egui::Ui) { "ui:newNote" => app.open_dialog("comment", json!({"threaded": false})), "ui:defineName" => app.open_dialog("defineName", json!({})), "ui:insertLink" => app.open_dialog("insertLink", json!({})), + "ui:openLink" => { + if let Some(target) = &link { + super::interact::follow_link(app, target); + } + } "ui:pickList" => app.open_dialog("pickList", json!({})), "ui:filterValue" => { if let Some(d) = app.session.active() { diff --git a/crates/ui-egui/tests/it/context_menu_stays_open.rs b/crates/ui-egui/tests/it/context_menu_stays_open.rs index c48c659d0..365c28c92 100644 --- a/crates/ui-egui/tests/it/context_menu_stays_open.rs +++ b/crates/ui-egui/tests/it/context_menu_stays_open.rs @@ -100,3 +100,36 @@ fn a_quick_right_click_between_pixels_keeps_the_menu_open_on_retina() { h.run_steps(4); assert!(h.state().grid.context_menu.is_some(), "the menu stays open"); } + +#[test] +fn a_linked_cell_offers_to_edit_open_and_remove_its_link() { + // #2120: the menu only offered "Link…", and its dialog was titled Insert Hyperlink. + use egui_kittest::kittest::Queryable as _; + let mut h = harness(); + h.state_mut().session.execute("insert.link", serde_json::json!({"cell": "B2", "target": "#Sheet1!D9", "text": "Go"})).unwrap(); + h.run_steps(2); + quick_right_click(&mut h, "B2"); + assert!(h.query_by_label("Edit Hyperlink…").is_some() && h.query_by_label("Open Hyperlink").is_some(), "linked cell menu"); + assert!(h.query_by_label("Link…").is_none()); + // Open Hyperlink follows it (an internal reference selects its cell). + h.get_by_label("Open Hyperlink").click(); + h.run_steps(4); + assert_eq!(h.state().session.active().unwrap().selection.active, CellRef::parse("D9").unwrap()); + // Edit Hyperlink opens the link's own dialog. + quick_right_click(&mut h, "B2"); + h.get_by_label("Edit Hyperlink…").click(); + h.run_steps(4); + let d = h.state().dialog.as_ref().expect("the link dialog"); + assert_eq!(d.title, "Edit Hyperlink"); + assert_eq!(d.values.get("target").and_then(|v| v.as_str()), Some("#Sheet1!D9")); + h.state_mut().dialog = None; + h.run_steps(2); + // Remove Hyperlink takes the link away; a cell without one offers Link… again. + quick_right_click(&mut h, "B2"); + h.get_by_label("Remove Hyperlink").click(); + h.run_steps(4); + let sh = h.state().session.active().unwrap().wb.active().unwrap().clone(); + assert!(!sh.hyperlinks.contains_key(&CellRef::parse("B2").unwrap())); + quick_right_click(&mut h, "B2"); + assert!(h.query_by_label("Link…").is_some() && h.query_by_label("Edit Hyperlink…").is_none()); +} diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 0f817f97a..8cc8fd002 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-11-03 · **Change:** minor (Display options for this workbook: sheet tabs and scroll bars; earlier: dragging over row or column headers in point mode extends the reference; earlier: a picture on the system clipboard pastes as a floating picture at the active cell with Cmd/Ctrl+V or Paste (#699); earlier: frozen columns keep their text, gridlines and merged borders whole while the sheet scrolls (#2544); earlier: every Paste Special option has its access key; earlier: the Draw tab's live stroke preview uses the pen's colour and thickness; earlier: copy quotes only cells with a tab or line break, as Excel does; earlier: Ctrl+. Next Corner of Selection; earlier: dragging the selection's border onto data asks before replacing it; earlier: RefEdit reference boxes in dialogs: Goal Seek, Solver and Data Analysis use them, the arrow keys select while a dialog is collapsed and F4 cycles a reference's anchors; earlier: Python cells: Python mode in the formula bar; earlier: Insert › Equation and the equation editor; earlier: every floating object selects, resizes from eight handles and deletes with the keyboard; earlier: Formulas › Error Checking › Trace Error; trace arrows stay through a zoom (#3097); earlier: Page Setup sets the page order, Down, then over or Over, then down, which printing, the print preview, Page Break Preview and Page Layout view follow; earlier: Formulas › Function Library as labelled buttons with category menus; Recently Used and the Insert Function categories; Esc leaves a File-page text field before closing the view; earlier: checkbox cells print as boxes; Ctrl+arrow and Ctrl+Shift+arrow while pointing a formula reference; editing a hyperlink loads its existing fields; AutoSum over a selected row puts its total to the right; ribbon catalog count 276 / 292) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-11-03 · **Change:** minor (a linked cell's menu edits, opens or removes the link; earlier: Display options for this workbook: sheet tabs and scroll bars; earlier: dragging over row or column headers in point mode extends the reference; earlier: a picture on the system clipboard pastes as a floating picture at the active cell with Cmd/Ctrl+V or Paste (#699); earlier: frozen columns keep their text, gridlines and merged borders whole while the sheet scrolls (#2544); earlier: every Paste Special option has its access key; earlier: the Draw tab's live stroke preview uses the pen's colour and thickness; earlier: copy quotes only cells with a tab or line break, as Excel does; earlier: Ctrl+. Next Corner of Selection; earlier: dragging the selection's border onto data asks before replacing it; earlier: RefEdit reference boxes in dialogs: Goal Seek, Solver and Data Analysis use them, the arrow keys select while a dialog is collapsed and F4 cycles a reference's anchors; earlier: Python cells: Python mode in the formula bar; earlier: Insert › Equation and the equation editor; earlier: every floating object selects, resizes from eight handles and deletes with the keyboard; earlier: Formulas › Error Checking › Trace Error; trace arrows stay through a zoom (#3097); earlier: Page Setup sets the page order, Down, then over or Over, then down, which printing, the print preview, Page Break Preview and Page Layout view follow; earlier: Formulas › Function Library as labelled buttons with category menus; Recently Used and the Insert Function categories; Esc leaves a File-page text field before closing the view; earlier: checkbox cells print as boxes; Ctrl+arrow and Ctrl+Shift+arrow while pointing a formula reference; editing a hyperlink loads its existing fields; AutoSum over a selected row puts its total to the right; ribbon catalog count 276 / 292) · **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 @@ -238,6 +238,7 @@ scrollbar bugs 2–4 · pixel/feel pass against observed Excel 7–12. | Date | Change | Summary | |---|---|---| +| 2026-11-03 | minor | Cell context menu on a cell with a link: Edit Hyperlink… (the link dialog, titled Edit Hyperlink and filled with the link), Open Hyperlink and Remove Hyperlink, as in Excel, instead of Link… (#2120) | | 2026-11-03 | minor | File › Options › Display options for this workbook: Show horizontal scroll bar, Show vertical scroll bar and Show sheet tabs (`view.showHorizontalScrollBar`, `view.showVerticalScrollBar`, `view.showSheetTabs`), saved with the workbook; with the sheet tabs off the scroll bar takes the bar, with the scroll bar off the tabs do, with both off the bar goes, and without the vertical scroll bar the grid takes its width (#3101) | | 2026-11-03 | minor | Find and Replace with a range selected searches only that range, as in Excel: Find Next moves the active cell through it and keeps it selected, Find All lists its matches, Replace and Replace All change only them; one selected cell still searches the sheet. `edit.find` / `edit.replace` take `within: selection` (#3224) | | 2026-11-03 | minor | Point mode: dragging over row or column headers extends the whole-row or whole-column reference, as it does over cells |