W3b review: quit asks from the empty window; background requests keep the active document

- Quitting from the Start screen's empty window asks about other windows' documents.
- A request for another window's document is handled when that window draws; the
  active window and document stay.
- Keys after a window closes or goes to the back in the same frame are dropped.
- A window whose document closed isn't entered.
- A window's title is kept only when it is still the drawn one after drawing.
- Control input queued for a closed viewport is dropped instead of stalling.
- Comment reply state is keyed by document and comment.
This commit is contained in:
aiongg
2026-10-22 17:14:18 +00:00
parent 338b5d6360
commit a6d5820e16
5 changed files with 141 additions and 49 deletions
+4 -6
View File
@@ -111,20 +111,18 @@ impl WordApp {
/// The open documents with work that would be lost on quitting: the shown one first, then
/// the others in the order they were opened. Unsaved changes, a save the browser hasn't
/// confirmed, or comment text not yet written to the document.
/// confirmed, or comment text not yet written to the document. The Start screen's
/// placeholder is never one, though other windows' documents are.
fn unsaved_documents(&self) -> Vec<u64> {
if !self.has_document() {
return Vec::new();
}
let active = self.session.document_id();
let mut out = Vec::new();
if self.session.dirty || self.save_unconfirmed() || panes::has_comment_drafts(self) {
if self.has_document() && (self.session.dirty || self.save_unconfirmed() || panes::has_comment_drafts(self)) {
out.push(active);
}
for d in self.session.documents() {
let unconfirmed = || self.saves_in_flight.iter().any(|s| s.marked_saved && s.document == d.id);
let drafts = || self.parked.get(&d.id).is_some_and(|p| !p.comment_drafts.is_empty());
if d.id != active && (d.dirty || unconfirmed() || drafts()) {
if d.id != active && Some(d.id) != self.no_document && (d.dirty || unconfirmed() || drafts()) {
out.push(d.id);
}
}
+14
View File
@@ -238,10 +238,14 @@ pub fn canvas_events(app: &mut WordApp, ctx: &egui::Context) {
return;
}
let events = ctx.input(|i| i.events.clone());
let window = app.windows.shown();
if app.session.view.read_mode {
// Read Mode is for navigation and selection, not typing into the document. The ribbon's
// Edit Document button or Escape returns to Print Layout before editing.
for e in &events {
if app.windows.shown() != window {
break;
}
match e {
// Not while a dialog or menu that takes Escape is open.
egui::Event::Key { key: Key::Escape, pressed: true, .. } if app.escape_free => {
@@ -276,6 +280,11 @@ pub fn canvas_events(app: &mut WordApp, ctx: &egui::Context) {
app.canvas.paste_mods = Some(held);
}
for e in events {
// A key closed the window (Mod+W) or brought another forward: the frame's other keys
// were typed in this one and are dropped.
if app.windows.shown() != window {
break;
}
// Which presses reached the canvas, and the modifiers held with Mod, in event order: Mod
// can go down and up within one frame, leaving the frame's modifiers without it.
match &e {
@@ -712,7 +721,12 @@ pub fn global_shortcuts(app: &mut WordApp, ctx: &egui::Context) {
return;
}
let events = ctx.input(|i| i.events.clone());
let window = app.windows.shown();
for e in events {
// The rest of the keys were pressed in a window that closed or went to the back.
if app.windows.shown() != window {
break;
}
if let egui::Event::Key { key, pressed: true, modifiers, .. } = e {
if leave_full_screen(app, key, modifiers, false) {
continue;
+5 -3
View File
@@ -1438,12 +1438,14 @@ impl WordApp {
if self.quit_requested {
return true;
}
// No document open (#1129): nothing to save.
if !self.has_document() {
// Nothing to save, also with no document open in this window (#1129) and none elsewhere.
if !self.has_unsaved_documents() {
return true;
}
// A comment draft the document refuses is an unsaved change: the prompt can drop it.
let _ = panes::flush_comment_drafts(self);
if self.has_document() {
let _ = panes::flush_comment_drafts(self);
}
if !self.has_unsaved_documents() {
return true;
}
+5 -3
View File
@@ -442,6 +442,8 @@ fn comments(app: &mut WordApp, ui: &mut Ui) {
ui.separator();
let list = app.session.run("review.comments", &json!({})).unwrap_or_default();
let t = Tokens::get(ui.ctx());
// Comment ids are the document's own; another window's document has the same ones.
let document = app.session.document_id();
egui::ScrollArea::vertical().show(ui, |ui| {
for c in list.as_array().cloned().unwrap_or_default() {
let id = c.get("id").and_then(Value::as_u64).unwrap_or(0);
@@ -460,7 +462,7 @@ fn comments(app: &mut WordApp, ui: &mut Ui) {
});
// Editable comment text (one undo step per edit, like the balloons).
comment_editor(app, ui, id as u32, None, false);
let open_key = egui::Id::new(("comment_reply_open", id));
let open_key = egui::Id::new(("comment_reply_open", document, id));
let mut replying = ui.data(|d| d.get_temp::<bool>(open_key)).unwrap_or(false);
ui.horizontal(|ui| {
if ui.small_button(tl!("Go to")).clicked()
@@ -471,7 +473,7 @@ fn comments(app: &mut WordApp, ui: &mut Ui) {
if ui.small_button(tl!("Reply")).clicked() {
replying = true;
ui.data_mut(|d| d.insert_temp(open_key, true));
ui.data_mut(|d| d.insert_temp(egui::Id::new(("comment_reply_focus", id)), true));
ui.data_mut(|d| d.insert_temp(egui::Id::new(("comment_reply_focus", document, id)), true));
}
let resolved = c.get("resolved").and_then(Value::as_bool).unwrap_or(false);
if ui.small_button(if resolved { tl!("Reopen") } else { tl!("Resolve") }).clicked() {
@@ -482,7 +484,7 @@ fn comments(app: &mut WordApp, ui: &mut Ui) {
}
});
if replying {
let focus = ui.data_mut(|d| d.remove_temp::<bool>(egui::Id::new(("comment_reply_focus", id)))).unwrap_or(false);
let focus = ui.data_mut(|d| d.remove_temp::<bool>(egui::Id::new(("comment_reply_focus", document, id)))).unwrap_or(false);
if reply_editor(app, ui, id as u32, None, focus) {
ui.data_mut(|d| d.remove::<bool>(open_key));
}
+113 -37
View File
@@ -295,14 +295,17 @@ impl WordApp {
}
/// Put the drawn window's state away and draw window `key`, with its document active. False
/// when there is no such window.
/// when there is no such window, or its document has closed ([`WordApp::prune_windows`]
/// closes it then); the drawn window stays.
pub(crate) fn enter_window(&mut self, key: u64) -> bool {
if key == self.windows.shown {
return true;
}
let Some(mut state) = self.windows.get_mut(key).and_then(|w| w.ui.take()) else {
let session = &self.session;
let Some(w) = self.windows.get_mut(key).filter(|w| w.ui.as_ref().is_some_and(|ui| session.is_open(ui.shown_document))) else {
return false;
};
let Some(mut state) = w.ui.take() else { return false };
let from = self.shown_document;
self.swap_window(&mut state);
let left = std::mem::replace(&mut self.windows.shown, key);
@@ -390,24 +393,22 @@ impl WordApp {
/// [`WordApp::sync_document`] with native windows: the session's active document `id` isn't
/// the drawn window's. Another open document shows in its window, which comes to the front,
/// or in a new window; when the drawn window's document has closed, its window closes and the
/// one showing `id` comes forward. False when the window itself is to show `id` instead: its
/// document was replaced, or it is the Start screen's empty window.
/// one showing `id` comes forward. The Start screen's empty window stays beside the window
/// that shows `id`. False when the window itself is to show `id` instead: its document was
/// replaced, or it is the Start screen's empty window and no other window shows `id`.
pub(crate) fn follow_in_windows(&mut self, id: u64) -> bool {
let left = self.shown_document;
let stand_in = self.no_document == Some(left);
if self.session.is_open(left) && !stand_in {
match self.windows.of_doc(id).filter(|&k| k != self.windows.shown) {
Some(key) => {
self.enter_window(key);
self.focus_window(key);
}
None => self.open_window(id),
let other = self.windows.of_doc(id).filter(|&k| k != self.windows.shown);
if self.session.is_open(left) {
match other {
Some(key) if self.enter_window(key) => self.focus_window(key),
_ if stand_in => return false,
_ => self.open_window(id),
}
return true;
}
if !self.session.is_open(left)
&& let Some(key) = self.windows.of_doc(id).filter(|&k| k != self.windows.shown)
{
if let Some(key) = other {
self.close_shown_window(key);
return true;
}
@@ -440,28 +441,17 @@ impl WordApp {
}
/// A request made for open document `document`, which another window shows: that window
/// handles it when it draws next, and comes to the front when the request opens something
/// (a dialog, the Print page). False when no other window shows the document (or windows
/// aren't native).
/// handles it when it draws next (a dialog opens there). The active window and document stay
/// as they are, so the control channel and agents go on working where they were. False when
/// no other window shows the document (or windows aren't native).
pub(crate) fn queue_for_window(&mut self, document: u64, req: &Value) -> bool {
if !self.windows.native {
return false;
}
let Some(key) = self.windows.of_doc(document).filter(|&k| k != self.windows.shown) else { return false };
let opens = req.get("open").is_some();
if opens && !self.windows.showing {
// Between frames: the window comes forward and opens it now.
self.enter_window(key);
self.focus_window(key);
self.handle_request(req);
return true;
}
if let Some(w) = self.windows.get_mut(key) {
w.requests.push(req.clone());
}
if opens {
self.focus_window(key);
}
if let Some(ctx) = &self.ctx {
ctx.request_repaint();
}
@@ -569,6 +559,11 @@ impl WordApp {
return;
}
self.window_ui(ui);
// Drawing can close the window or bring another forward: the fields then hold another
// window's state.
if self.windows.shown != key {
return;
}
let title = self.sent_title.clone();
if let Some(w) = self.windows.get_mut(key) {
w.title = title;
@@ -618,7 +613,17 @@ impl WordApp {
ctx.add_plugin(ViewportInput::default());
self.windows.input_plugin = true;
}
self.settle_windows();
let target = self.active_viewport();
// Input queued for a window that has closed since (or moved into the root viewport)
// would wait forever.
let live: Vec<ViewportId> = self.windows.open.iter().map(|w| self.windows.viewport(w.key)).filter(|&v| v != ViewportId::ROOT).collect();
ctx.with_plugin(|p: &mut ViewportInput| {
if p.target.is_some_and(|t| !live.contains(&t)) {
p.target = None;
p.events.clear();
}
});
let waiting = ctx.with_plugin(|p: &mut ViewportInput| !p.events.is_empty()).unwrap_or(true);
if waiting {
return;
@@ -908,29 +913,100 @@ mod tests {
}
/// A request an agent's command makes for the document of another window is that window's:
/// Print opens the Print page there and brings it forward, and the window that was active
/// keeps its own page.
/// Print opens the Print page there when it draws, and the active window and document stay
/// as they are, so the agent's next command works where it did.
#[test]
fn a_request_for_another_windows_document_is_handled_there() {
let ctx = backend();
let mut a = app("First");
let first = a.session.document_id();
let first_window = a.windows.shown();
frames(&ctx, &mut a, 2);
a.run("file.new", json!({})).unwrap();
let second = a.windows.shown();
let second = a.session.document_id();
frames(&ctx, &mut a, 2);
let child = a.active_viewport();
a.session.with_document(first, |s| s.run("file.print", &json!({}))).unwrap().unwrap();
for req in std::mem::take(&mut a.session.ui_requests) {
assert_eq!(req["document"], first);
a.handle_request(&req);
}
assert_eq!(a.session.document_id(), first, "the first window comes forward");
assert!(a.ui.backstage && a.ui.backstage_page == "print");
assert_eq!((a.session.document_id(), a.active_viewport()), (second, child), "the active document stays");
assert!(!a.ui.backstage);
frames(&ctx, &mut a, 2);
assert_eq!(a.active_viewport(), ViewportId::ROOT);
assert!(a.ui.backstage, "the Print page stays in its window");
let other = a.windows.get(second).and_then(|w| w.ui.as_ref()).map(|w| w.backstage);
assert_eq!(other, Some(false), "the other window keeps its page");
assert_eq!((a.session.document_id(), a.active_viewport()), (second, child));
assert!(!a.ui.backstage, "the active window keeps its page");
let other = a.windows.get(first_window).and_then(|w| w.ui.as_ref()).map(|w| (w.backstage, w.backstage_page.as_str()));
assert_eq!(other, Some((true, "print")), "the Print page opened in the document's window");
}
/// Quitting from the Start screen's empty window asks about the documents other windows
/// show (one an agent made and typed in).
#[test]
fn quitting_from_the_empty_window_asks_about_the_other_windows() {
let ctx = backend();
let mut a = WordApp::new(Session::new(wordcraft_doc::Document::new()), Services::default());
frames(&ctx, &mut a, 2);
a.run("file.close", json!({})).unwrap();
assert!(!a.has_document());
let empty = a.windows.shown();
let placeholder = a.session.document_id();
let other = a.session.with_document(placeholder, |s| s.add_document(wordcraft_doc::Document::new())).unwrap();
a.session.with_document(other, |s| s.run("text.insert", &json!({"text": "Agent"}))).unwrap().unwrap();
frames(&ctx, &mut a, 2);
assert_eq!((a.window_count(), a.windows.shown()), (2, empty), "the agent's document has a window behind");
assert!(!a.has_document());
assert!(!a.quit());
assert_eq!((a.session.document_id(), prompt(&a), text(&a).as_str()), (other, Some("saveChanges"), "Agent"));
assert_ne!(a.windows.shown(), empty, "the document's own window asks");
a.run("ui.saveChanges", json!({"answer": "dontSave"})).unwrap();
assert!(a.quit_requested);
}
/// Mod+W in a window with no changes closes it; the keys typed after it in the same frame
/// don't reach the window that comes forward.
#[test]
fn keys_after_closing_a_window_go_nowhere() {
let ctx = backend();
let mut a = app("First");
frames(&ctx, &mut a, 2);
a.run("file.new", json!({})).unwrap();
let child = a.active_viewport();
focus(child);
frames(&ctx, &mut a, 8);
assert!(a.canvas.focused);
let mac = cfg!(target_os = "macos");
let modifiers = egui::Modifiers { ctrl: !mac, mac_cmd: mac, command: true, ..Default::default() };
EVENTS.with(|e| {
e.borrow_mut().entry(child).or_default().extend([
egui::Event::Key { key: egui::Key::W, physical_key: None, pressed: true, repeat: false, modifiers },
egui::Event::Text("x".into()),
])
});
frame(&ctx, &mut a);
assert_eq!(a.window_count(), 1, "Mod+W closed the window");
frames(&ctx, &mut a, 2);
assert_eq!(text(&a), "First");
}
/// A window whose document closed without the windows following (an agent closed it) isn't
/// drawn or made active; the next frame closes it.
#[test]
fn a_window_whose_document_closed_is_not_entered() {
let ctx = backend();
let mut a = app("First");
let first = a.session.document_id();
let first_window = a.windows.shown();
frames(&ctx, &mut a, 2);
a.run("file.new", json!({})).unwrap();
let (second, second_window) = (a.session.document_id(), a.windows.shown());
frames(&ctx, &mut a, 2);
a.session.close_document(first).unwrap();
assert!(!a.enter_window(first_window));
assert_eq!((a.windows.shown(), a.session.document_id()), (second_window, second));
frames(&ctx, &mut a, 2);
assert_eq!((a.window_count(), a.session.document_id()), (1, second));
}
/// A composition in one window shows and commits only there: the other window draws no