Review: Previous/Next Comment, Show Comments, Show All Notes, Convert to Comments (#281)

- review.previousComment / review.nextComment select the previous/next
  threaded comment in reading order (row by row), across visible sheets in
  tab order, wrapping around; disabled when the workbook has no comments.
- review.showComments {on?} toggles the Comments pane as session state, so
  the CLI, MCP and control channel can drive it; the UI pane follows it.
- review.showAllNotes {on?} displays every note (session display setting,
  not saved in the file).
- review.convertToComments turns every note in the workbook into a threaded
  comment with the same author and text, as one undoable edit.
- Ribbon: Previous/Next and a Show Comments toggle in the Comments group;
  Show All Notes and Convert to Comments in the Notes menu. Strings in all
  catalogs. Parity checklist regenerated (263 / 290).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
vangel76
2026-10-11 12:04:03 +02:00
co-authored by Claude Opus 5.5
parent 9f896c1249
commit 6f47a8b463
15 changed files with 270 additions and 25 deletions
+167 -2
View File
@@ -1,6 +1,8 @@
//! Review tab: protection, spelling (basic), workbook statistics.
//! Review tab: protection, spelling (basic), workbook statistics, comment navigation and the
//! Comments pane, Show All Notes and Convert to Comments.
use gridcraft_model::{SheetProtection, password_hash};
use gridcraft_core::CellRef;
use gridcraft_model::{SheetProtection, Visibility, password_hash};
use serde_json::{Value as Json, json};
use super::*;
@@ -20,9 +22,94 @@ pub fn specs() -> Vec<CommandSpec> {
cmd!("review.protectWorkbook", "Protect Workbook", ["Review", "Protect"], None, "{on?: bool}", has_doc, protect_workbook),
cmd!(query "review.workbookStatistics", "Workbook Statistics", ["Review", "Proofing"], None, "{}", has_doc, stats),
cmd!(query "review.checkAccessibility", "Check Accessibility", ["Review", "Accessibility"], None, "{} → issues", has_doc, accessibility),
cmd!(noundo "review.previousComment", "Previous Comment", ["Review", "Comments"], None, "{} → {sheet, cell}", has_comments, |s, _| {
step_comment(s, false)
}),
cmd!(noundo "review.nextComment", "Next Comment", ["Review", "Comments"], None, "{} → {sheet, cell}", has_comments, |s, _| step_comment(
s, true
)),
cmd!(query "review.showComments", "Show Comments", ["Review", "Comments"], None, "{on?: bool} (toggles the Comments pane)", has_doc, |s, p| {
s.comments_pane = bool_param(p, "on").unwrap_or(!s.comments_pane);
Ok(json!({"on": s.comments_pane}))
}),
cmd!(query "review.showAllNotes", "Show All Notes", ["Review", "Notes"], None, "{on?: bool}", has_doc, |s, p| {
s.show_all_notes = bool_param(p, "on").unwrap_or(!s.show_all_notes);
Ok(json!({"on": s.show_all_notes}))
}),
cmd!(
"review.convertToComments",
"Convert to Comments",
["Review", "Notes"],
None,
"{} (every note in the workbook) → {converted}",
has_doc,
convert_notes
),
]
}
/// Threaded comments in navigation order: visible sheets in tab order, then row by row.
fn comment_cells(s: &Session) -> Vec<(usize, CellRef)> {
let Some(d) = s.active() else { return vec![] };
d.wb.sheets
.iter()
.enumerate()
.filter(|(_, sh)| sh.visibility == Visibility::Visible)
// `CellRef` orders by row, then column: reading order.
.flat_map(|(i, sh)| sh.comments.iter().filter(|(_, c)| c.threaded).map(move |(at, _)| (i, *at)))
.collect()
}
fn has_comments(s: &Session) -> std::result::Result<(), String> {
has_doc(s)?;
if comment_cells(s).is_empty() { Err("there are no comments in this workbook".into()) } else { Ok(()) }
}
/// Selects the next (or previous) threaded comment after the active cell, moving on to the next
/// sheet and wrapping around at the end of the workbook.
fn step_comment(s: &mut Session, forward: bool) -> Result<Json> {
let list = comment_cells(s);
let d = s.doc()?;
let here = (d.wb.active_sheet, d.selection.active);
let target = if forward {
list.iter().find(|c| **c > here).or_else(|| list.first())
} else {
list.iter().rev().find(|c| **c < here).or_else(|| list.last())
};
let Some(&(sheet, at)) = target else {
return Err(EngineError::Other("There are no comments in this workbook.".into()));
};
let d = s.doc_mut()?;
d.wb_switch_sheet(sheet);
d.selection = crate::Selection::at(at);
if let Some(sh) = d.wb.active() {
d.selection.expand_merges(sh);
}
let name = d.wb.active().map(|sh| sh.name.clone()).unwrap_or_default();
Ok(json!({"sheet": name, "cell": at.a1()}))
}
/// Review › Notes › Convert to Comments: every note in the workbook becomes a threaded comment
/// with the same author and text.
fn convert_notes(s: &mut Session, _: &Json) -> Result<Json> {
let sheets: Vec<usize> =
s.doc()?.wb.sheets.iter().enumerate().filter(|(_, sh)| sh.comments.values().any(|c| !c.threaded)).map(|(i, _)| i).collect();
if sheets.is_empty() {
return Ok(json!({"converted": 0}));
}
edit(s, |cx| {
let mut n = 0usize;
for i in &sheets {
for c in cx.sheet_mut(*i)?.comments.values_mut().filter(|c| !c.threaded) {
c.threaded = true;
c.visible = false;
n += 1;
}
}
Ok(json!({"converted": n}))
})
}
fn protect_sheet(s: &mut Session, p: &Json) -> Result<Json> {
let sheet = target_sheet(s, p)?;
let b = |k: &str| bool_param(p, k).unwrap_or(false);
@@ -97,3 +184,81 @@ fn accessibility(s: &mut Session, _: &Json) -> Result<Json> {
}
Ok(Json::Array(issues))
}
#[cfg(test)]
mod tests {
use super::*;
fn book() -> Session {
let mut s = Session::new();
s.new_workbook();
s.execute("home.insertSheet", json!({"name": "Two"})).unwrap();
s.execute("sheet.activate", json!({"sheet": 0})).unwrap();
for (sheet, cell) in [(0, "C1"), (0, "A3"), (1, "B2")] {
s.execute("review.newComment", json!({"sheet": sheet, "cell": cell, "text": "c", "author": "Ann"})).unwrap();
}
s.execute("review.newNote", json!({"sheet": 0, "cell": "A2", "text": "note", "author": "Bob"})).unwrap();
s
}
fn at(s: &Session) -> (usize, String) {
let d = s.doc().unwrap();
(d.wb.active_sheet, d.selection.active.a1())
}
#[test]
fn next_and_previous_comment_go_in_reading_order_across_sheets_and_wrap() {
let mut s = book();
s.execute("selection.set", json!({"cell": "B1"})).unwrap();
// Notes are skipped: A2 holds a note, not a comment.
let order = [(0, "C1"), (0, "A3"), (1, "B2"), (0, "C1")];
for (sheet, cell) in order {
s.execute("review.nextComment", json!({})).unwrap();
assert_eq!(at(&s), (sheet, cell.to_string()));
}
for (sheet, cell) in [(1, "B2"), (0, "A3"), (0, "C1")] {
s.execute("review.previousComment", json!({})).unwrap();
assert_eq!(at(&s), (sheet, cell.to_string()));
}
}
#[test]
fn comment_navigation_is_disabled_without_comments() {
let mut s = Session::new();
s.new_workbook();
s.execute("review.newNote", json!({"cell": "A1", "text": "note"})).unwrap();
assert!(s.execute("review.nextComment", json!({})).is_err());
assert!(s.execute("review.previousComment", json!({})).is_err());
assert_eq!(at(&s), (0, "A1".to_string()));
}
#[test]
fn convert_to_comments_turns_every_note_into_a_comment_and_undoes() {
let mut s = book();
s.execute("review.newNote", json!({"sheet": 1, "cell": "D4", "text": "second", "author": "Cy"})).unwrap();
s.execute("review.showNote", json!({"sheet": 1, "cell": "D4"})).unwrap();
let r = s.execute("review.convertToComments", json!({})).unwrap();
assert_eq!(r["converted"], 2);
let wb = &s.doc().unwrap().wb;
let a2 = &wb.sheets[0].comments[&CellRef::parse("A2").unwrap()];
let d4 = &wb.sheets[1].comments[&CellRef::parse("D4").unwrap()];
assert!(a2.threaded && d4.threaded && !d4.visible);
assert_eq!((a2.author.as_str(), a2.text.as_str(), d4.author.as_str(), d4.text.as_str()), ("Bob", "note", "Cy", "second"));
// Nothing left to convert; one undo restores the notes.
assert_eq!(s.execute("review.convertToComments", json!({})).unwrap()["converted"], 0);
s.execute("edit.undo", json!({})).unwrap();
assert!(!s.doc().unwrap().wb.sheets[0].comments[&CellRef::parse("A2").unwrap()].threaded);
}
#[test]
fn show_comments_and_show_all_notes_toggle() {
let mut s = book();
assert_eq!(s.execute("review.showComments", json!({})).unwrap()["on"], true);
assert_eq!(s.execute("review.showComments", json!({})).unwrap()["on"], false);
assert_eq!(s.execute("review.showComments", json!({"on": true})).unwrap()["on"], true);
assert!(s.comments_pane);
assert_eq!(s.execute("review.showAllNotes", json!({})).unwrap()["on"], true);
assert_eq!(s.execute("review.showAllNotes", json!({"on": false})).unwrap()["on"], false);
assert!(!s.show_all_notes);
}
}