M7: add a persistent language-aware spelling review dialog

Keep compact context menus, but show a correction form for user spelling/editor commands. Advance without wrapping after change, ignore or dictionary actions; reject stale targets and retain programmatic command results.

Add navigation/UI regressions and light/dark preview CI. cargo xtask ci was attempted on the preparation PC but cargo is absent; hosted validation is required before integration.
This commit is contained in:
totoz64
2026-10-26 10:00:13 +01:00
parent 2c99a42780
commit b2787bf36e
10 changed files with 397 additions and 50 deletions
+56
View File
@@ -0,0 +1,56 @@
name: Spelling review preview
on:
pull_request:
paths:
- 'crates/ui-egui/src/dialogs_spelling.rs'
- 'crates/ui-egui/src/dialogs_review.rs'
- 'crates/engine/src/cmd/review.rs'
- '.github/workflows/spelling-review.yml'
workflow_dispatch:
permissions:
contents: read
concurrency:
group: spelling-review-preview-${{ github.ref }}
cancel-in-progress: true
jobs:
preview:
runs-on: ubuntu-24.04
timeout-minutes: 45
env:
CARGO_INCREMENTAL: '0'
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1
- name: Install build and software-rendering dependencies
run: |
sudo apt-get update
sudo apt-get install -y --no-install-recommends \
pkg-config libxkbcommon-dev libwayland-dev libx11-dev libxcursor-dev libxrandr-dev \
libxi-dev libgtk-3-dev libfontconfig1-dev libfreetype-dev libvulkan1 mesa-vulkan-drivers
- uses: dtolnay/rust-toolchain@b725eda525362e7cf34af9a7243cac1e3f01a7ec
with:
components: rustfmt
- uses: Swatinem/rust-cache@d1527c8f67ef9af357290475d4af4b0fab8f12e0
- name: Formatting
id: formatting
continue-on-error: true
run: cargo fmt --all -- --check
- name: Proofing navigation regression
run: cargo test --locked -p wordcraft-engine --lib spelling_review_dialog_navigation
- name: Correction form regressions
run: cargo test --locked -p wordcraft-ui-egui --lib spelling_review_dialog -- --skip preview
- name: Render light and dark correction forms
run: cargo test --locked -p wordcraft-ui-egui --lib spelling_review_dialog_preview -- --ignored --nocapture
- name: Save visual review artifacts
if: always()
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02
with:
name: spelling-review-preview
path: spelling-review-*.png
if-no-files-found: error
- name: Require formatting success
if: steps.formatting.outcome != 'success'
run: exit 1
+3 -1
View File
File diff suppressed because one or more lines are too long
+31 -4
View File
@@ -66,6 +66,7 @@ pub fn specs() -> Vec<CommandSpec> {
CommandSpec::new("review.changes", "Reviewing Pane", "Review › Tracking", list_changes).pure(),
CommandSpec::new("review.spelling", "Spelling & Grammar", "Review › Proofing", next_issue)
.key(super::mac_or("F7 / Alt+F7 / Mod+Alt+L", "F7 / Alt+F7"))
.params(r#"{"wrap"?: bool (default true; false stops at the end of the current story)}"#)
.pure(),
CommandSpec::new("review.issues", "Proofing Issues", "Review › Proofing", all_issues).pure(),
CommandSpec::new("review.suggestions", "Spelling Suggestions", "Review › Proofing", suggestions).params(r#"{"pos"?: Pos}"#).pure(),
@@ -989,15 +990,20 @@ fn issue_json(s: &Session, a: &Pos, b: &Pos, i: &wordcraft_proof::Issue) -> Valu
}
/// F7: select the next issue after the caret and return it with suggestions.
fn next_issue(s: &mut Session, _: &Value) -> CmdResult {
fn next_issue(s: &mut Session, v: &Value) -> CmdResult {
let caret = s.sel.ordered().1;
let story = caret.story;
let paths = s.doc.para_paths(story);
let start = paths.iter().position(|p| *p == caret.path).unwrap_or(0);
for k in 0..paths.len() {
let Some(path) = paths.get((start + k) % paths.len().max(1)) else { break };
let wrap = p::bool(v, "wrap").unwrap_or(true);
// Scan forward first, including the rest of the caret's paragraph. A second pass
// optionally visits the earlier part; never select the first typo repeatedly in
// a single-paragraph story. Dialogs disable wrapping so Ignore Once can finish.
for (path, before) in paths.iter().skip(start).map(|path| (path, false)).chain(
paths.iter().take(if wrap { start.saturating_add(1) } else { 0 }).map(|path| (path, true)),
) {
for (a, b, i) in para_issues(s, story, path) {
if k == 0 && a.off < caret.off && start + k < paths.len() && paths.len() > 1 {
if *path == caret.path && (a.off < caret.off) != before {
continue;
}
let r = issue_json(s, &a, &b, &i);
@@ -2246,3 +2252,24 @@ mod tests {
}
}
}
#[cfg(test)]
mod spelling_navigation_tests {
use super::*;
use wordcraft_doc::Document;
#[test]
fn spelling_review_dialog_navigation_moves_forward_and_wraps_only_when_requested() {
for text in ["The brwn fox and brwn dog.", "The brwn fox.\nThe brwn dog."] {
let mut s = Session::new(Document::from_text(text));
let first = s.run("review.spelling", &json!({"wrap": false})).unwrap();
assert_eq!(first["text"], "brwn");
let first_selection = s.sel.clone();
let second = s.run("review.spelling", &json!({"wrap": false})).unwrap();
assert_eq!(second["text"], "brwn");
assert_ne!(s.sel, first_selection);
assert_eq!(s.run("review.spelling", &json!({"wrap": false})).unwrap()["done"], true);
assert_eq!(s.run("review.spelling", &json!({})).unwrap()["start"], first["start"]);
}
}
}
-32
View File
@@ -6217,38 +6217,6 @@ mod tests {
assert_eq!(h.state().session.doc.plain_text(StoryRef::Body), "The quick brown fox");
}
/// #3340: review.spelling (the Review tab's Spelling & Grammar button and the Home tab's
/// Editor button both run it) only moved the selection to the next issue — with no way to see
/// or apply a suggestion from there, just the generic selection toolbar, it looked broken.
/// Running it now also opens the same suggestions menu Shift+F10 and a Control-click already
/// open for the word at the caret.
#[test]
fn spelling_and_grammar_opens_suggestions_for_the_issue_it_selects() {
use egui_kittest::kittest::Queryable;
let mut a = WordApp::new(wordcraft_engine::Session::new(wordcraft_doc::Document::new()), Default::default());
a.run("text.insert", json!({"text": "The quick brwn fox"})).unwrap();
a.session.sel = wordcraft_engine::Selection::caret(Pos::body(0, 0));
a.run("review.spelling", json!({})).unwrap();
assert!(a.canvas.context_menu_from_keys, "queues the open the context menu already uses for Shift+F10");
let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1440.0, 900.0)).build_ui_state(
|ui, app: &mut WordApp| {
let ctx = ui.ctx().clone();
app.logic(&ctx);
app.ui(ui);
},
a,
);
for _ in 0..6 {
h.step();
}
assert!(h.state().canvas.context_menu_open, "Spelling & Grammar opens the suggestions menu");
h.get_by_label("brown").click();
for _ in 0..3 {
h.step();
}
assert_eq!(h.state().session.doc.plain_text(StoryRef::Body), "The quick brown fox");
}
#[test]
fn floating_picture_can_be_dragged_in_front_of_or_behind_text() {
for wrap in ["inFrontOfText", "behindText"] {
+4
View File
@@ -17,6 +17,7 @@ use crate::theme::{Tokens, semibold};
#[derive(Clone, Debug, Serialize)]
#[serde(tag = "form", rename_all = "camelCase")]
pub enum ReviewDialog {
Spelling(Box<crate::dialogs_spelling::SpellingForm>),
Compare(CompareForm),
AutoCorrect(Box<AutoCorrectForm>),
}
@@ -24,6 +25,7 @@ pub enum ReviewDialog {
impl ReviewDialog {
pub fn name(&self) -> &'static str {
match self {
ReviewDialog::Spelling(_) => "spelling",
ReviewDialog::Compare(f) if f.combine => "combine",
ReviewDialog::Compare(_) => "compare",
ReviewDialog::AutoCorrect(_) => "autoCorrect",
@@ -33,6 +35,7 @@ impl ReviewDialog {
/// The window title (English; the caller translates it).
pub fn title(&self) -> &'static str {
match self {
ReviewDialog::Spelling(_) => "Spelling & Grammar",
ReviewDialog::Compare(f) if f.combine => "Combine Documents",
ReviewDialog::Compare(_) => "Compare Documents",
ReviewDialog::AutoCorrect(_) => "AutoCorrect",
@@ -57,6 +60,7 @@ pub fn open(name: &str, app: &mut WordApp) -> Option<ReviewDialog> {
/// Draws the dialog; returns true to close it.
pub fn body(app: &mut WordApp, ui: &mut Ui, d: &mut ReviewDialog) -> bool {
match d {
ReviewDialog::Spelling(f) => crate::dialogs_spelling::body(app, ui, f),
ReviewDialog::Compare(f) => compare_ui(app, ui, f),
ReviewDialog::AutoCorrect(f) => autocorrect_ui(app, ui, f),
}
+262
View File
@@ -0,0 +1,262 @@
//! A persistent proofing form over the shared review commands, independent of language data.
use egui::Ui;
use serde::Serialize;
use serde_json::{Value, json};
use wordcraft_engine::Selection;
use crate::WordApp;
#[derive(Clone, Debug, Serialize)]
pub struct SpellingForm {
issue: Value,
replacement: String,
message: String,
#[serde(skip)]
document: u64,
#[serde(skip)]
epoch: u64,
#[serde(skip)]
selection: Selection,
}
impl SpellingForm {
pub fn new(app: &WordApp, issue: Value) -> Self {
let replacement = issue.get("suggestions").and_then(Value::as_array).and_then(|a| a.first()).and_then(Value::as_str).unwrap_or("").into();
Self {
issue,
replacement,
message: String::new(),
document: app.session.document_id(),
epoch: app.session.ime_body_epoch(),
selection: app.session.sel.clone(),
}
}
fn done(&self) -> bool {
self.issue.get("done").and_then(Value::as_bool) == Some(true)
}
// Other windows and commands can change a target while this form is open.
// Never apply a cached suggestion to a different selection or document body.
fn current(&self, app: &mut WordApp) -> bool {
self.document == app.session.document_id()
&& self.epoch == app.session.ime_body_epoch()
&& self.selection == app.session.sel
&& app.session.run("review.suggestions", &json!({})).ok().as_ref() == Some(&self.issue)
}
fn next(&mut self, app: &mut WordApp, wrap: bool) {
match app.execute("review.spelling", json!({"wrap": wrap})) {
Ok(issue) => *self = Self::new(app, issue),
Err(e) => self.message = e,
}
}
fn act(&mut self, app: &mut WordApp, command: Option<&str>) {
if !self.current(app) {
self.message = tl!("The document or selection changed. Check again to continue.").into();
return;
}
if let Some(command) = command {
let params = if command == "review.applySuggestion" { json!({"text": self.replacement}) } else { json!({}) };
if let Err(e) = app.run(command, params) {
self.message = e;
return;
}
}
self.next(app, false);
}
}
/// Changing or ignoring advances within the same form; Close and Escape dismiss it.
pub fn body(app: &mut WordApp, ui: &mut Ui, form: &mut SpellingForm) -> bool {
ui.set_width(420.0);
if form.done() {
ui.label(tl!("The spelling and grammar check is complete."));
} else {
let spelling = form.issue.get("kind").and_then(Value::as_str) == Some("spelling");
ui.horizontal_wrapped(|ui| {
ui.label(tl!(if spelling { "Spelling" } else { "Grammar" }));
ui.label(form.issue.get("lang").and_then(Value::as_str).unwrap_or(""));
});
ui.label(egui::RichText::new(form.issue.get("text").and_then(Value::as_str).unwrap_or("")).strong());
ui.label(tl!(form.issue.get("message").and_then(Value::as_str).unwrap_or("")));
ui.add_space(8.0);
ui.label(tl!("Suggestions"));
let suggestions: Vec<&str> = form.issue.get("suggestions").and_then(Value::as_array).into_iter().flatten().filter_map(Value::as_str).collect();
egui::ScrollArea::vertical().id_salt("spelling_suggestions").max_height(120.0).show(ui, |ui| {
if suggestions.is_empty() {
ui.label(egui::RichText::new(tl!("(no suggestions)")).italics());
}
for suggestion in &suggestions {
if ui.selectable_label(form.replacement == *suggestion, *suggestion).clicked() {
form.replacement = (*suggestion).into();
}
}
});
let label = ui.label(tl!("Change to:"));
ui.add(egui::TextEdit::singleline(&mut form.replacement).char_limit(1024).desired_width(400.0)).labelled_by(label.id);
let can_change = (!form.replacement.is_empty() || suggestions.contains(&"")) && crate::widgets::enabled(app, "review.applySuggestion");
let mut action = None;
ui.horizontal_wrapped(|ui| {
if ui.add_enabled(can_change, egui::Button::new(tl!("Change"))).clicked() {
action = Some(Some("review.applySuggestion"));
}
if ui.button(tl!("Ignore Once")).clicked() {
action = Some(None);
}
if spelling {
if ui.button(tl!("Ignore All")).clicked() {
action = Some(Some("review.ignoreAll"));
}
if ui.button(tl!("Add to Dictionary")).clicked() {
action = Some(Some("review.addToDictionary"));
}
}
});
if let Some(command) = action {
form.act(app, command);
}
}
if !form.message.is_empty() {
ui.colored_label(ui.visuals().error_fg_color, &form.message);
}
ui.add_space(8.0);
let mut close = false;
ui.horizontal(|ui| {
if ui.button(tl!("Check Again")).clicked() {
form.next(app, true);
}
close = ui.button(tl!("Close")).clicked();
});
close || crate::dialogs::cancel_key_pressed(ui)
}
#[cfg(test)]
mod tests {
use egui_kittest::kittest::Queryable;
use wordcraft_doc::{Document, Pos, StoryRef};
use wordcraft_engine::Session;
use super::*;
use crate::{Services, dialogs::Dialog, dialogs_review::ReviewDialog};
fn app(text: &str) -> WordApp {
WordApp::new(Session::new(Document::from_text(text)), Services::default())
}
fn frame(ui: &mut Ui, app: &mut WordApp) {
let ctx = ui.ctx().clone();
app.logic(&ctx);
app.ui(ui);
}
fn harness(text: &str) -> egui_kittest::Harness<'static, WordApp> {
let mut app = app(text);
app.run("review.spelling", json!({})).unwrap();
let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1440.0, 900.0)).build_ui_state(frame, app);
h.run_steps(6);
h
}
fn form(app: &WordApp) -> &SpellingForm {
match &app.dialog {
Some(Dialog::Review(d)) => match &**d {
ReviewDialog::Spelling(f) => f,
_ => panic!("spelling form expected"),
},
_ => panic!("spelling dialog expected"),
}
}
#[test]
fn spelling_review_dialog_changes_a_suggestion_and_undo_restores_the_word() {
let mut h = harness("The quick brwn fox.");
assert!(!h.state().canvas.context_menu_open);
h.get_by_label("brown").click();
h.run_steps(2);
h.get_by_label("Change").click();
h.run_steps(3);
assert_eq!(h.state().session.doc.plain_text(StoryRef::Body), "The quick brown fox.");
assert!(form(h.state()).done());
h.get_by_label("Close").click();
h.run_steps(2);
assert!(h.state().dialog.is_none());
h.state_mut().run("edit.undo", json!({})).unwrap();
assert_eq!(h.state().session.doc.plain_text(StoryRef::Body), "The quick brwn fox.");
}
#[test]
fn spelling_review_dialog_ignore_once_advances_and_finishes_without_changing_text() {
let text = "The brwn fox and brwn dog.";
let mut h = harness(text);
let first = h.state().session.sel.clone();
h.get_by_label("Ignore Once").click();
h.run_steps(3);
assert_ne!(h.state().session.sel, first);
assert_eq!(form(h.state()).issue["text"], "brwn");
h.get_by_label("Ignore Once").click();
h.run_steps(3);
assert!(form(h.state()).done());
assert_eq!(h.state().session.doc.plain_text(StoryRef::Body), text);
h.get_by_label("Check Again").click();
h.run_steps(3);
assert_eq!(h.state().session.sel, first, "Ignore Once never adds a word to the dictionary");
}
#[test]
fn spelling_review_dialog_ignore_all_and_add_to_dictionary_advance() {
for (word, button) in [("zqproofingdialogignore", "Ignore All"), ("zqproofingdialogcustom", "Add to Dictionary")] {
let text = format!("The {word} fox and {word} dog.");
let mut h = harness(&text);
h.get_by_label(button).click();
h.run_steps(3);
assert!(form(h.state()).done());
assert_eq!(h.state().session.doc.plain_text(StoryRef::Body), text);
assert!(h.state_mut().session.run("review.issues", &json!({})).unwrap().as_array().unwrap().is_empty());
}
}
#[test]
fn spelling_review_dialog_refuses_a_stale_selection_or_replaced_body() {
for replace in [false, true] {
let mut a = app("The brwn fox and brwn dog.");
let issue = a.execute("review.spelling", json!({})).unwrap();
assert!(a.dialog.is_none(), "programmatic review calls never open a window");
let mut f = SpellingForm::new(&a, issue);
f.replacement = "brown".into();
if replace {
a.execute("document.setText", json!({"text": "The brwn fox and brwn dog."})).unwrap();
} else {
a.session.sel = Selection::caret(Pos::body(0, 0));
}
let before = a.session.doc.plain_text(StoryRef::Body);
f.act(&mut a, Some("review.applySuggestion"));
assert!(!f.message.is_empty());
assert_eq!(a.session.doc.plain_text(StoryRef::Body), before);
}
}
#[test]
fn spelling_review_dialog_shows_completion_without_issues_and_escape_closes() {
let mut h = harness("The quick brown fox.");
assert!(form(h.state()).done());
h.key_press(egui::Key::Escape);
h.run_steps(2);
assert!(h.state().dialog.is_none());
}
#[test]
#[ignore = "offscreen visual review; requires a software or hardware graphics adapter"]
fn spelling_review_dialog_preview() {
let mut a = app("The quick brwn fox.");
a.run("review.spelling", json!({})).unwrap();
let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1000.0, 700.0)).wgpu().build_ui_state(frame, a);
for (name, visuals) in [("light", egui::Visuals::light()), ("dark", egui::Visuals::dark())] {
h.ctx.set_visuals(visuals);
h.run_steps(6);
h.render().unwrap().save(format!("spelling-review-{name}.png")).unwrap();
}
}
}
+7 -9
View File
@@ -45,6 +45,7 @@ pub mod dialogs_notes;
pub mod dialogs_para;
pub mod dialogs_refs;
pub mod dialogs_review;
pub mod dialogs_spelling;
pub mod dialogs_ribbon;
pub mod dialogs_smart_art;
pub mod dialogs_table_style;
@@ -1114,15 +1115,12 @@ impl WordApp {
if matches!(id, "insert.picture" | "insert.icon") {
self.select_inserted_picture(&r);
}
// review.spelling only moves the selection to the next issue; without this, the ribbon's
// Spelling & Grammar and Editor buttons looked broken — just a selection with no way to
// see or apply a suggestion (#3340). Reuse the context menu's suggestions, as Shift+F10
// already does for the word at the caret.
if id == "review.spelling"
&& let Ok(v) = &r
&& v.get("done").and_then(serde_json::Value::as_bool) != Some(true)
{
self.canvas.context_menu_from_keys = true;
// User proofing opens a persistent correction form; scripts keep the engine's
// structured result. Right-click and Shift+F10 retain their compact suggestions.
if matches!(id, "review.spelling" | "review.editor") && let Ok(v) = &r {
self.dialog = Some(dialogs::Dialog::Review(Box::new(dialogs_review::ReviewDialog::Spelling(Box::new(
dialogs_spelling::SpellingForm::new(self, v.clone()),
)))));
}
let r = system_clipboard::explain(self, id, r);
// Check Accessibility shows its findings in the Accessibility pane, kept up to date there.
+3 -2
View File
File diff suppressed because one or more lines are too long
+24
View File
@@ -0,0 +1,24 @@
# Spelling and grammar correction form
> **Last updated:** 2026-10-26 · **Status:** implementation proposed; hosted checks and visual acceptance must be verified before integration.
Review › Spelling & Grammar, Home › Editor and F7 open a persistent correction form. The issue's text, explanation and proofing language appear above the suggestions. Choose a suggestion or type replacement text in Change to, then choose Change. Each replacement uses the existing review command and is one undoable document edit. Ignore Once advances without changing the document or dictionaries. Ignore All lasts for the session; Add to Dictionary uses the existing personal dictionary. Those two actions are offered only for spelling issues.
The pass begins at the next issue from the current selection and continues to the end of the current story (body, header, note or other active text). It does not traverse all document stories or restrict itself to a selected range. Check Again starts another search with wrapping; ignored-once occurrences can be revisited. Completion stays visible until Close or Escape. The compact right-click/Shift+F10 menu is retained.
The form consumes language-aware engine results. It works with the existing proofing languages and is independent of French contribution #3185; adding an interface does not install a dictionary or add grammar rules. Interface labels use the shared translation lookup and fall back to English when a catalog lacks a key. No release date or completeness increase is implied.
If another command changes the document body or selection while the form is open, an old suggestion is refused. Check Again obtains a fresh target. Read-only/protected document commands retain the engine's existing checks and display refusals in the form.
## Verification
- Engine regression: one- and two-paragraph navigation, forward-only completion and explicit wrapping.
- UI regressions: suggestion application and undo, Ignore Once progress/completion, Ignore All, personal dictionary addition, stale selection/body refusal, no-issue completion and Escape.
- Offscreen preview: the ignored spelling_review_dialog_preview test writes light/dark PNGs; inspect them before integration.
- Required repository gate: cargo xtask ci. Cargo is unavailable on the preparation PC, so hosted checks are required; test definitions alone are not evidence of success.
## Revision history
| Date | Change | Summary |
|---|---|---|
| 2026-10-26 | minor | Separate language-independent correction-form proposal and verification scope |
+7 -2
View File
File diff suppressed because one or more lines are too long