From 0db810d0e82e040e090cc468976c6a7952bbc3ec Mon Sep 17 00:00:00 2001
From: alexanderalber <90604107+alexanderalber@users.noreply.github.com>
Date: Mon, 12 Oct 2026 04:15:27 +0200
Subject: [PATCH] =?UTF-8?q?Layout=20=E2=80=BA=20Page=20Setup:=20the=20grou?=
 =?UTF-8?q?p=20launcher=20opens=20the=20Page=20Setup=20dialog=20(#471)=20(?=
 =?UTF-8?q?#563)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The launcher ran ui.dialog, which needs a dialog name, but group
launchers run their command with no parameters, so nothing opened. It
now runs layout.pageSetup, which asks the UI for the Page Setup dialog,
as the Font and Paragraph launchers run their dialog commands.

The test opens the dialog through layout.pageSetup and checks that no
ribbon group uses ui.dialog as its launcher.

generated with Claude Code

Co-authored-by: Brandon Thomas <echelon@gmail.com>
---
 crates/ui-egui/src/ribbon.rs | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs
index c91c16e14..9308e8e75 100644
--- a/crates/ui-egui/src/ribbon.rs
+++ b/crates/ui-egui/src/ribbon.rs
@@ -994,7 +994,7 @@ fn design(app: &mut WordApp, ui: &mut Ui) {
 }
 
 fn layout(app: &mut WordApp, ui: &mut Ui) {
-    group(ui, "Page Setup", Some("ui.dialog"), app, |ui, app| {
+    group(ui, "Page Setup", Some("layout.pageSetup"), app, |ui, app| {
         // Text Direction works on what the caret is in: table cells, a text box, else the section.
         let dir = wordcraft_engine::cmd::page::text_direction_at(&app.session);
         menu_button(ui, app, "textDirection", Some("Text\nDirection"), "Text Direction", true, |ui, app| {
@@ -2293,6 +2293,21 @@ mod tests {
         assert!(!contextual_tabs(&s).contains(&"Shape Format"));
     }
 
+    #[test]
+    fn page_setup_launcher_opens_the_dialog() {
+        // A group launcher runs its command with no parameters (#471).
+        let mut app = crate::WordApp::new(Session::new(wordcraft_doc::Document::new()), crate::Services::default());
+        app.run("layout.pageSetup", json!({})).unwrap();
+        assert!(matches!(app.dialog, Some(crate::dialogs::Dialog::PageSetup { .. })));
+        // `ui.dialog` needs a dialog name, so it can't be a launcher.
+        let launchers: Vec<&str> = include_str!("ribbon.rs")
+            .split("group(ui, \"")
+            .filter_map(|g| g.split_once('"')?.1.strip_prefix(", Some(\"")?.split('"').next())
+            .collect();
+        assert!(launchers.contains(&"layout.pageSetup"), "{launchers:?}");
+        assert!(!launchers.contains(&"ui.dialog"), "{launchers:?}");
+    }
+
     #[test]
     fn smart_art_picker_inserts_and_design_tab_follows_the_selection() {
         let mut app = crate::WordApp::new(Session::new(wordcraft_doc::Document::new()), Default::default());