Merge pull request #286 from vangel76/feat/review-comment-navigation

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

crates/engine/src/cmd/review.rs

@@ -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::{EditRange, PasswordHash, SheetProtection, password_hash};
+use gridcraft_core::CellRef;
+use gridcraft_model::{EditRange, PasswordHash, SheetProtection, Visibility, password_hash};
 use serde_json::{Value as Json, json};
 
 use super::*;
@@ -32,9 +34,94 @@ pub fn specs() -> Vec<CommandSpec> {
             undoable: true,
         },
         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);
@@ -225,6 +312,84 @@ fn accessibility(s: &mut Session, _: &Json) -> Result<Json> {
 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);
+    }
+}
+
+#[cfg(test)]
+mod edit_range_tests {
+    use super::*;
+
     fn protected_book() -> Session {
         let mut s = Session::new();
         s.new_workbook();

crates/engine/src/lib.rs

@@ -212,6 +212,11 @@ pub struct Session {
     pub draw_tool: String,
     pub draw_color: String,
     pub draw_width: f32,
+    /// Review › Show Comments: the Comments pane is open.
+    pub comments_pane: bool,
+    /// Review › Notes › Show All Notes: every note is displayed. An application display setting
+    /// (like Excel's comment indicator option), so it's not saved in the file.
+    pub show_all_notes: bool,
 }
 
 impl Session {

crates/ui-egui/src/dialogs.rs

@@ -706,7 +706,8 @@ impl Dialog {
             "saveChanges" => Dialog::custom("saveChanges", "Save Changes?", json!({"title": p("title")})),
             "start" => Dialog::custom("start", "GridCraft", json!({})),
             "comments" => {
-                app.grid.pane = Some("comments".into());
+                app.grid.pane = None;
+                app.run_or_alert("review.showComments", json!({"on": true}));
                 return None;
             }
             "watch" | "selectionPane" | "formatChart" => {

crates/ui-egui/src/grid.rs

@@ -2310,8 +2310,9 @@ fn paint_overlays(
     }
     // Notes: shown on hover (or always when marked visible).
     let hover = app.grid.hover_cell;
+    let all_notes = app.session.show_all_notes;
     for (c, cm) in &sh.comments {
-        let show = cm.visible || hover == Some(*c);
+        let show = cm.visible || (all_notes && !cm.threaded) || hover == Some(*c);
         if !show || app.grid.drag != Drag::None {
             continue;
         }

crates/ui-egui/src/i18n.rs

@@ -342,6 +342,12 @@ const JAPANESE: &[(&str, &str)] = &[
     ("Check\nAccessibility", "アクセシビリティ チェック"),
     ("Threaded Comments", "スレッド化されたコメント"),
     ("Show\nComments", "コメントの表示"),
+    ("Previous", "前へ"),
+    ("Next", "次へ"),
+    ("Previous Comment", "前のコメント"),
+    ("Next Comment", "次のコメント"),
+    ("Show All Notes", "すべてのメモを表示"),
+    ("Convert to Comments", "コメントに変換"),
     ("Protect\nWorkbook", "ブックの保護"),
     ("Allow Edit\nRanges", "範囲の編集を許可"),
     ("Allow Edit Ranges", "範囲の編集を許可"),

crates/ui-egui/src/i18n/cs.rs

@@ -184,6 +184,12 @@ pub(super) fn translate(text: &str) -> Option<&'static str> {
         "Gridlines" => "Mřížka",
         "Headings" => "Záhlaví",
         "Zoom" => "Lupa",
+        "Previous" => "Předchozí",
+        "Next" => "Další",
+        "Previous Comment" => "Předchozí komentář",
+        "Next Comment" => "Další komentář",
+        "Show All Notes" => "Zobrazit všechny poznámky",
+        "Convert to Comments" => "Převést na komentáře",
         "Checkbox" => "Zaškrtávací políčko",
         "Insert checkboxes in the selected cells" => "Vložit zaškrtávací políčka do vybraných buněk",
         "Sheet\nRight-to-Left" => "List zprava\ndoleva",

crates/ui-egui/src/i18n/de.rs

@@ -184,6 +184,12 @@ pub(super) fn translate(text: &str) -> Option<&'static str> {
         "Gridlines" => "Gitternetzlinien",
         "Headings" => "Überschriften",
         "Zoom" => "Zoom",
+        "Previous" => "Zurück",
+        "Next" => "Weiter",
+        "Previous Comment" => "Vorheriger Kommentar",
+        "Next Comment" => "Nächster Kommentar",
+        "Show All Notes" => "Alle Notizen anzeigen",
+        "Convert to Comments" => "In Kommentare konvertieren",
         "Checkbox" => "Kontrollkästchen",
         "Insert checkboxes in the selected cells" => "Kontrollkästchen in die ausgewählten Zellen einfügen",
         "Sheet\nRight-to-Left" => "Blatt von rechts\nnach links",

crates/ui-egui/src/i18n/es.rs

@@ -185,6 +185,12 @@ pub(super) fn translate(text: &str) -> Option<&'static str> {
         "Gridlines" => "Líneas de cuadrícula",
         "Headings" => "Encabezados",
         "Zoom" => "Zoom",
+        "Previous" => "Anterior",
+        "Next" => "Siguiente",
+        "Previous Comment" => "Comentario anterior",
+        "Next Comment" => "Comentario siguiente",
+        "Show All Notes" => "Mostrar todas las notas",
+        "Convert to Comments" => "Convertir en comentarios",
         "Checkbox" => "Casilla",
         "Insert checkboxes in the selected cells" => "Insertar casillas en las celdas seleccionadas",
         "Sheet\nRight-to-Left" => "Hoja de derecha\na izquierda",

crates/ui-egui/src/i18n/ko.rs

@@ -179,6 +179,12 @@ pub(super) fn translate(text: &str) -> Option<&'static str> {
         "Check\nAccessibility" => "접근성\n검사",
         "Threaded Comments" => "스레드 댓글",
         "Show\nComments" => "댓글\n표시",
+        "Previous" => "이전",
+        "Next" => "다음",
+        "Previous Comment" => "이전 댓글",
+        "Next Comment" => "다음 댓글",
+        "Show All Notes" => "모든 메모 표시",
+        "Convert to Comments" => "댓글로 변환",
         "Protect\nWorkbook" => "통합 문서\n보호",
         "Allow Edit\nRanges" => "범위 편집\n허용",
         "Allow Edit Ranges" => "범위 편집 허용",

crates/ui-egui/src/i18n/pt.rs

@@ -180,6 +180,12 @@ pub(super) fn translate(text: &str) -> Option<&'static str> {
         "Check\nAccessibility" => "Verificar\nAcessibilidade",
         "Threaded Comments" => "Comentários Encadeados",
         "Show\nComments" => "Mostrar\nComentários",
+        "Previous" => "Anterior",
+        "Next" => "Próximo",
+        "Previous Comment" => "Comentário Anterior",
+        "Next Comment" => "Próximo Comentário",
+        "Show All Notes" => "Mostrar Todas as Notas",
+        "Convert to Comments" => "Converter em Comentários",
         "Protect\nWorkbook" => "Proteger\nPasta de Trabalho",
         "Allow Edit\nRanges" => "Permitir Edição\nde Intervalos",
         "Allow Edit Ranges" => "Permitir Edição de Intervalos",

crates/ui-egui/src/i18n/ru.rs

@@ -179,6 +179,12 @@ pub(super) fn translate(text: &str) -> Option<&'static str> {
         "Check\nAccessibility" => "Проверка\nдоступности",
         "Threaded Comments" => "Комментарии с ветками",
         "Show\nComments" => "Показать\nкомментарии",
+        "Previous" => "Назад",
+        "Next" => "Далее",
+        "Previous Comment" => "Предыдущий комментарий",
+        "Next Comment" => "Следующий комментарий",
+        "Show All Notes" => "Показать все заметки",
+        "Convert to Comments" => "Преобразовать в комментарии",
         "Protect\nWorkbook" => "Защитить\nкнигу",
         "Allow Edit\nRanges" => "Разрешить\nизменение диапазонов",
         "Allow Edit Ranges" => "Разрешить изменение диапазонов",

crates/ui-egui/src/i18n/zh.rs

@@ -179,6 +179,12 @@ pub(super) fn translate(text: &str) -> Option<&'static str> {
         "Check\nAccessibility" => "检查\n辅助功能",
         "Threaded Comments" => "线程式批注",
         "Show\nComments" => "显示\n批注",
+        "Previous" => "上一条",
+        "Next" => "下一条",
+        "Previous Comment" => "上一条批注",
+        "Next Comment" => "下一条批注",
+        "Show All Notes" => "显示所有备注",
+        "Convert to Comments" => "转换为批注",
         "Protect\nWorkbook" => "保护\n工作簿",
         "Allow Edit\nRanges" => "允许编辑\n区域",
         "Allow Edit Ranges" => "允许编辑区域",

crates/ui-egui/src/panes.rs

@@ -7,7 +7,15 @@ use crate::SheetApp;
 use crate::theme::{self, Tokens};
 
 pub fn show(app: &mut SheetApp, ui: &mut Ui) {
-    let Some(pane) = app.grid.pane.clone() else { return };
+    // The Comments pane is engine state (`review.showComments`, so agents can drive it); the other
+    // panes are UI state. One pane shows at a time: opening another one closes Comments.
+    if app.grid.pane.as_deref() == Some("comments") {
+        app.grid.pane = None;
+        app.run_or_alert("review.showComments", json!({"on": true}));
+    } else if app.grid.pane.is_some() && app.session.comments_pane {
+        app.run_or_alert("review.showComments", json!({"on": false}));
+    }
+    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| {
         ui.horizontal(|ui| {
@@ -21,6 +29,9 @@ pub fn show(app: &mut SheetApp, ui: &mut Ui) {
             ui.label(egui::RichText::new(title).font(theme::ui_bold(15.0)));
             ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| {
                 if crate::widgets::icon_button(ui, crate::icons::Icon::Close, t.text_dim, "Close", vec2(22.0, 22.0)).clicked() {
+                    if pane == "comments" {
+                        app.run_or_alert("review.showComments", json!({"on": false}));
+                    }
                     app.grid.pane = None;
                 }
             });

crates/ui-egui/src/ribbon.rs

@@ -1650,15 +1650,43 @@ fn review(app: &mut SheetApp, ui: &mut Ui) {
     if big_button(ui, Icon::Comment, "New\nComment", "New Comment", false).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());
-    }
     if big_button(ui, Icon::Delete, "Delete", "Delete Comment", false).clicked() {
         act(app, "review.deleteComment", json!({}));
     }
+    ui.vertical(|ui| {
+        for (icon, label, tip, id) in
+            [(Icon::Left, "Previous", "Previous Comment", "review.previousComment"), (Icon::Right, "Next", "Next Comment", "review.nextComment")]
+        {
+            if small_button(ui, icon, label, tip, false).clicked() {
+                act(app, id, json!({}));
+                app.grid.ensure_visible = true;
+            }
+        }
+    });
+    let shown = app.session.comments_pane;
+    let r = big_button(ui, Icon::Comment, "Show\nComments", "Comments pane", false);
+    if shown {
+        ui.painter().rect_stroke(r.rect, 5.0, Stroke::new(2.0, Tokens::get(ui.ctx()).accent), StrokeKind::Inside);
+    }
+    if r.clicked() {
+        act(app, "review.showComments", json!({"on": !shown}));
+        if app.session.comments_pane {
+            app.grid.pane = None;
+        }
+    }
     let notes = big_button(ui, Icon::Note, "Notes", "Notes", true);
     egui::Popup::menu(&notes).show(|ui| {
-        menu_items(app, ui, &[("New Note", "dialog:comment", json!({"threaded": false})), ("Show/Hide Note", "review.showNote", json!({}))]);
+        menu_items(
+            app,
+            ui,
+            &[
+                ("New Note", "dialog:comment", json!({"threaded": false})),
+                ("Show/Hide Note", "review.showNote", json!({})),
+                ("Show All Notes", "review.showAllNotes", json!({})),
+                ("-", "", json!({})),
+                ("Convert to Comments", "review.convertToComments", json!({})),
+            ],
+        );
     });
     sep(ui);
     let protected = app.session.active().and_then(|d| d.wb.active().map(|s| s.is_protected())).unwrap_or(false);

docs/gaps.md

@@ -153,6 +153,7 @@ No alpha blockers: every core workflow passes the [alpha gate](roadmap.md#alpha-
 
 | Date | Change | Summary |
 |---|---|---|
+| 2026-10-11 | minor | §16: comment navigation, Show Comments, Show All Notes and Convert to Comments landed (#281) |
 | 2026-10-11 | minor | §15: ODDFPRICE, ODDFYIELD, ODDLPRICE, ODDLYIELD added (18 functions missing) |
 | 2026-10-11 | minor | §15: PERCENTOF and EUROCONVERT added (20 functions missing) |
 | 2026-10-11 | minor | §15: BAHTTEXT added (21 functions missing) |

docs/parity-checklist.md

@@ -4,7 +4,7 @@
 
 Each entry is an Excel ribbon or menu command (feature names only). It counts as implemented when GridCraft registers a command with the same id. This measures breadth only; the overall assessment (depth, fidelity, effort) is in [target-app-parity.md](target-app-parity.md).
 
-**Overall: 266 / 290 commands implemented (91.7%).**
+**Overall: 271 / 290 commands implemented (93.4%).**
 
 | Tab | Implemented | Total | Parity |
 |---|---|---|---|
@@ -15,7 +15,7 @@ Each entry is an Excel ribbon or menu command (feature names only). It counts as
 | Page Layout | 23 | 23 | 100% |
 | Formulas | 23 | 23 | 100% |
 | Data | 22 | 28 | 79% |
-| Review | 10 | 18 | 56% |
+| Review | 15 | 18 | 83% |
 | View | 20 | 20 | 100% |
 | Automate | 3 | 3 | 100% |
 | Table Design | 12 | 13 | 92% |
@@ -273,7 +273,7 @@ Missing:
 - `data.forecastSheet` Forecast Sheet (Forecast)
 - `data.analyzeData` Analyze Data (Analysis)
 
-## Review (10 / 18, 56%)
+## Review (15 / 18, 83%)
 
 Implemented:
 
@@ -282,8 +282,13 @@ Implemented:
 - `review.workbookStatistics` Workbook Statistics (Proofing)
 - `review.checkAccessibility` Check Accessibility (Accessibility)
 - `review.deleteComment` Delete Comment (Comments)
+- `review.previousComment` Previous Comment (Comments)
+- `review.nextComment` Next Comment (Comments)
+- `review.showComments` Show Comments (Comments)
 - `review.newNote` New Note (Notes)
 - `review.showNote` Show/Hide Note (Notes)
+- `review.showAllNotes` Show All Notes (Notes)
+- `review.convertToComments` Convert to Comments (Notes)
 - `review.unprotectSheet` Unprotect Sheet (Protect)
 - `review.protectWorkbook` Protect Workbook (Protect)
 - `review.allowEditRanges` Allow Edit Ranges (Protect)
@@ -292,11 +297,6 @@ Missing:
 
 - `review.smartLookup` Smart Lookup (Insights)
 - `review.translate` Translate (Language)
-- `review.previousComment` Previous Comment (Comments)
-- `review.nextComment` Next Comment (Comments)
-- `review.showComments` Show Comments (Comments)
-- `review.showAllNotes` Show All Notes (Notes)
-- `review.convertToComments` Convert to Comments (Notes)
 - `review.showChanges` Show Changes (Changes)
 
 ## View (20 / 20, 100%)