From 41a99962aafd823431f6557138a7a36908c29f0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Fri, 9 Oct 2026 20:17:16 -0400 Subject: [PATCH] ElyShow: an open dialog is never pushed aside; a stuck show start is gone (S-04) --- apps/ely-show/src/chart.rs | 4 ++-- apps/ely-show/src/check.rs | 4 ++-- apps/ely-show/src/dialog.rs | 19 ++++++++++++++++--- apps/ely-show/src/new.rs | 4 ++-- apps/ely-show/src/output.rs | 7 ++++--- apps/ely-show/src/remarks.rs | 4 ++-- apps/ely-show/src/slideshow/start.rs | 10 +++++----- apps/ely-show/src/table.rs | 8 ++------ apps/ely-show/src/tests/slides.rs | 17 +++++++++++++++++ crates/ely-ui/src/i18n/show.rs | 2 ++ tasks/BUGS.md | 2 +- tasks/reviews/S-04-r1.md | 3 +++ tasks/reviews/S-04-r2.md | 1 + 13 files changed, 59 insertions(+), 26 deletions(-) create mode 100644 tasks/reviews/S-04-r1.md create mode 100644 tasks/reviews/S-04-r2.md diff --git a/apps/ely-show/src/chart.rs b/apps/ely-show/src/chart.rs index aa6f072..98ef68f 100644 --- a/apps/ely-show/src/chart.rs +++ b/apps/ely-show/src/chart.rs @@ -186,8 +186,8 @@ impl Deck { return log::info!("show: a chart dialog for tab {tab} came late"); } let editing = Editing::new(tab, chart, window, cx); - deck.dialog = Some(Dialog::Chart(Box::new(editing))); - cx.notify(); + // Refused while another dialog is open; it says so. + let _ = deck.open_dialog(Dialog::Chart(Box::new(editing)), cx); }) }) .detach_and_log_err(cx); diff --git a/apps/ely-show/src/check.rs b/apps/ely-show/src/check.rs index 55db2a8..3173b4f 100644 --- a/apps/ely-show/src/check.rs +++ b/apps/ely-show/src/check.rs @@ -128,8 +128,8 @@ impl Deck { Ok((check, slides)) if deck.dialog_due(asked, id) => { let found = findings(&check, &slides, &missing); log::info!("show: the check found {} things", found.len()); - deck.dialog = Some(Dialog::Check(found)); - cx.notify(); + // Refused while another dialog is open; it says so. + let _ = deck.open_dialog(Dialog::Check(found), cx); } Ok(_) => log::info!("show: the check is no longer due"), Err(error) => deck.engine_failed(&format!("the check: {error}"), cx), diff --git a/apps/ely-show/src/dialog.rs b/apps/ely-show/src/dialog.rs index dedab59..88297cf 100644 --- a/apps/ely-show/src/dialog.rs +++ b/apps/ely-show/src/dialog.rs @@ -1,6 +1,7 @@ //! ElyShow's one open dialog. -use ely_gpui_component::forms::TextInput; +use ely_gpui_component::{forms::TextInput, primitives::Severity}; +use ely_office::prelude::*; use gpui::{App, Context, Entity, IntoElement as _}; use crate::deck::Deck; @@ -41,6 +42,18 @@ pub(crate) fn edit( } impl Deck { + /// Opens `dialog`, or hands it back while another is open. + pub(crate) fn open_dialog(&mut self, dialog: Dialog, cx: &mut Context) -> Result<(), Dialog> { + if self.dialog.is_some() { + let text = Deck::text(cx, "show.dialog.busy", &[]); + self.toast(text, Severity::Warning, cx); + return Err(dialog); + } + self.dialog = Some(dialog); + cx.notify(); + Ok(()) + } + /// A read that will open a dialog; earlier ones lapse. pub fn ask_dialog(&mut self) -> u64 { self.dialog_asked += 1; @@ -56,9 +69,9 @@ impl Deck { } } - /// Whether read `asked` may open its dialog now. + /// Whether read `asked` still stands; `open_dialog` checks the rest. pub fn dialog_due(&self, asked: u64, id: u64) -> bool { - self.dialog_asked == asked && self.dialog.is_none() && self.core.active == Some(id) + self.dialog_asked == asked && self.core.active == Some(id) } /// The open dialog, drawn. diff --git a/apps/ely-show/src/new.rs b/apps/ely-show/src/new.rs index 15d4ac2..fade91b 100644 --- a/apps/ely-show/src/new.rs +++ b/apps/ely-show/src/new.rs @@ -34,8 +34,8 @@ impl Deck { match ely_lok::templates("presnt") { Ok(templates) => { log::info!("show: {} bundled templates", templates.len()); - self.dialog = Some(Dialog::New(Choosing { templates })); - cx.notify(); + // Refused while another dialog is open; it says so. + let _ = self.open_dialog(Dialog::New(Choosing { templates }), cx); } Err(error) => self.engine_failed(&error.to_string(), cx), } diff --git a/apps/ely-show/src/output.rs b/apps/ely-show/src/output.rs index ae87b29..adfb8fd 100644 --- a/apps/ely-show/src/output.rs +++ b/apps/ely-show/src/output.rs @@ -148,7 +148,8 @@ impl Deck { |worker, doc| ely_lok::show::slide_show(&worker.values(doc, ely_lok::show::SLIDE_SHOW)?), move |deck, read, _, cx| match read { Ok(slides) if deck.dialog_due(asked, id) => { - deck.dialog = Some(Dialog::Output(Output { + // Refused while another dialog is open; it says so. + let output = Output { tab: id, print, kind: Kind::Slides, @@ -156,8 +157,8 @@ impl Deck { per: 3, current, hidden: slides.iter().map(|slide| slide.hidden).collect(), - })); - cx.notify(); + }; + let _ = deck.open_dialog(Dialog::Output(output), cx); } Ok(_) => log::info!("show: the output dialog is no longer due"), Err(error) => deck.engine_failed(&format!("the slides: {error}"), cx), diff --git a/apps/ely-show/src/remarks.rs b/apps/ely-show/src/remarks.rs index aba59c7..38ba8ee 100644 --- a/apps/ely-show/src/remarks.rs +++ b/apps/ely-show/src/remarks.rs @@ -77,8 +77,8 @@ impl Deck { input.set_text(text, cx); input }); - self.dialog = Some(Dialog::Remark { tab, edit, field }); - cx.notify(); + // Refused while another dialog is open; it says so. + let _ = self.open_dialog(Dialog::Remark { tab, edit, field }, cx); } /// Sends the dialog's text: a new comment or an edit. diff --git a/apps/ely-show/src/slideshow/start.rs b/apps/ely-show/src/slideshow/start.rs index b34e401..855a587 100644 --- a/apps/ely-show/src/slideshow/start.rs +++ b/apps/ely-show/src/slideshow/start.rs @@ -196,11 +196,11 @@ impl Deck { }; match cx.displays().len() { 1 => deck.open_show(starting, cx), - _ => { - deck.show_starting = true; - deck.dialog = Some(Dialog::Show(starting)); - cx.notify(); - } + _ => match deck.open_dialog(Dialog::Show(starting), cx) { + Ok(()) => deck.show_starting = true, + Err(Dialog::Show(refused)) => discard(worker, refused.doc, refused.copy, cx), + Err(_) => unreachable!("the show dialog comes back"), + }, } }, cx, diff --git a/apps/ely-show/src/table.rs b/apps/ely-show/src/table.rs index e0983d1..e07450b 100644 --- a/apps/ely-show/src/table.rs +++ b/apps/ely-show/src/table.rs @@ -21,12 +21,8 @@ use crate::dialog::{Dialog, edit}; impl Deck { pub fn ask_table(&mut self, cx: &mut Context) { if let Some(tab) = self.core.active { - self.dialog = Some(Dialog::Table { - tab, - columns: 3, - rows: 3, - }); - cx.notify(); + // Refused while another dialog is open; it says so. + let _ = self.open_dialog(Dialog::Table { tab, columns: 3, rows: 3 }, cx); } } diff --git a/apps/ely-show/src/tests/slides.rs b/apps/ely-show/src/tests/slides.rs index 9f51870..cb984aa 100644 --- a/apps/ely-show/src/tests/slides.rs +++ b/apps/ely-show/src/tests/slides.rs @@ -104,6 +104,23 @@ fn the_table_dialog_inserts_a_table() { }); } +#[test] +fn an_open_dialog_is_not_pushed_aside() { + let dir = tempfile::tempdir().expect("a temp dir"); + let mut cx = TestAppContext::single(); + let Some(deck) = blank(&mut cx, dir.path()) else { + eprintln!("SKIP: no LOK build"); + return; + }; + update(&mut cx, &deck, |deck, cx| deck.ask_table(cx)); + update(&mut cx, &deck, |deck, cx| deck.ask_new(cx)); + // The chart dialog opens after a background read. + update(&mut cx, &deck, |deck, cx| deck.ask_chart(cx)); + cx.run_until_parked(); + let kept = cx.update(|app| matches!(deck.read(app).dialog, Some(crate::dialog::Dialog::Table { .. }))); + assert!(kept, "the table dialog stays"); +} + #[test] fn typed_notes_reach_the_slide() { let dir = tempfile::tempdir().expect("a temp dir"); diff --git a/crates/ely-ui/src/i18n/show.rs b/crates/ely-ui/src/i18n/show.rs index bb1b869..85b049d 100644 --- a/crates/ely-ui/src/i18n/show.rs +++ b/crates/ely-ui/src/i18n/show.rs @@ -2,6 +2,7 @@ pub(super) const EN: &[(&str, &str)] = &[ ("menu.slide", "Slide"), + ("show.dialog.busy", "Finish or close the open dialog first"), ("show.slide.new", "New Slide"), ("show.slide.duplicate", "Duplicate Slide"), ("show.slide.delete", "Delete Slide"), @@ -189,6 +190,7 @@ pub(super) const EN: &[(&str, &str)] = &[ pub(super) const ZH_CN: &[(&str, &str)] = &[ ("menu.slide", "幻灯片"), + ("show.dialog.busy", "请先完成或关闭已打开的对话框"), ("show.slide.new", "新建幻灯片"), ("show.slide.duplicate", "复制幻灯片"), ("show.slide.delete", "删除幻灯片"), diff --git a/tasks/BUGS.md b/tasks/BUGS.md index 5fc9b06..c145fbd 100644 --- a/tasks/BUGS.md +++ b/tasks/BUGS.md @@ -26,7 +26,7 @@ - [x] S-01 切换幻灯片后主画布不重画 — 已由 be31260（R2.4）修好并有单测，此处补勾 - [x] S-02 `deck.rs` 超过 500 行 — 随 S-03 把 Extra 移到 extra.rs，deck.rs 463 行 - [x] S-03 ElyShow 的缩放一两帧后弹回“适合幻灯片”，缩放功能不可用 — 只在标签首次观察时适合一次；R1 PASS -- [ ] S-04 选屏对话框被别的对话框顶掉后，`show_starting` 卡住、放映副本泄漏 +- [x] S-04 选屏对话框被别的对话框顶掉后，`show_starting` 卡住、放映副本泄漏 — 有对话框开着时不再顶掉它，提示先完成；选屏对话框被拒即丢副本；R2 PASS - [ ] S-05 一条批注的时间恰为午夜或只有日期，整个批注栏不显示 - [ ] S-06 导出或打印的“从–到”填 0 时导出全部幻灯片 - [ ] S-07 母版视图下演示检查的行按幻灯片序号跳转 diff --git a/tasks/reviews/S-04-r1.md b/tasks/reviews/S-04-r1.md new file mode 100644 index 0000000..0bc4c18 --- /dev/null +++ b/tasks/reviews/S-04-r1.md @@ -0,0 +1,3 @@ +FAIL + +apps/ely-show/src/chart.rs:189 — `open_chart` 的异步回调仍直接覆盖 `deck.dialog`；其守卫只核对标签与选中图表，绕过了新增的 `open_dialog`。单屏隔离探针先调用 `ask_table`，再调用 `ask_chart` 并推进调度，原表格对话框被图表对话框替换，保留原对话框的断言失败（`/tmp/ely-s04-chart-probe.log`）。两屏下，选屏对话框打开后执行“插入图表”会走同一覆盖路径，丢弃 `Starting`，保留 `show_starting = true`，且跳过 `discard`，因此 S-04 的放映卡住与副本泄漏路径仍存在。修复方向：让图表回调中的首次打开经过 `open_dialog`，保留当前对话框并显示忙碌提示；补充图表入口的覆盖回归测试。临时副本仅把这处首次打开改为 `open_dialog` 后，同一探针通过（`/tmp/ely-s04-chart-control.log`）。独立验证：LOK worker 与 ElyShow 构建成功，ElyShow 80 项测试、ely-ui 34 项测试及注释检查通过；移除拒绝逻辑后，现有 `an_open_dialog_is_not_pushed_aside` 测试失败。原生双屏选屏流程受单屏环境限制；隔离源码副本、独立构建目录与增量缓存均已清理。 diff --git a/tasks/reviews/S-04-r2.md b/tasks/reviews/S-04-r2.md new file mode 100644 index 0000000..7ef22e9 --- /dev/null +++ b/tasks/reviews/S-04-r2.md @@ -0,0 +1 @@ +PASS