From 3f45edf14483db65053fe6e12179cc03291ba9fa Mon Sep 17 00:00:00 2001
From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com>
Date: Mon, 12 Oct 2026 08:31:21 +0200
Subject: [PATCH] Fix ribbon checkboxes sending the pre-click value (#520)

---
 crates/ui-egui/src/ribbon.rs | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs
index e8291ac..9f62a35 100644
--- a/crates/ui-egui/src/ribbon.rs
+++ b/crates/ui-egui/src/ribbon.rs
@@ -343,6 +343,12 @@ fn anchor_keytip(app: &SheetApp, ui: &Ui, response: &egui::Response, targets: &[
     }
 }
 
+/// The `on` value for a ribbon checkbox command. A click has already flipped the checkbox bool, so
+/// it is sent as is; a key tip leaves the bool untouched, so the command gets the opposite.
+fn checkbox_on(clicked: bool, shown: bool) -> bool {
+    shown == clicked
+}
+
 /// Consume a one-shot keytip request for control `id` (set by `Action::Control`).
 fn invoke(app: &mut SheetApp, id: &str) -> bool {
     if app.ui.keytip_invoke == Some(id) {
@@ -1698,13 +1704,13 @@ fn page_layout(app: &mut SheetApp, ui: &mut Ui) {
         let resp = ui.checkbox(&mut v, screen);
         control_keytip(app, ui, &resp, "view.gridlines");
         if resp.changed() || invoke(app, "view.gridlines") {
-            act(app, "view.gridlines", json!({"on": !v}));
+            act(app, "view.gridlines", json!({"on": checkbox_on(resp.changed(), v)}));
         }
         let mut p = sh.2;
         let resp = ui.checkbox(&mut p, print);
         control_keytip(app, ui, &resp, "pageLayout.printGridlines");
         if resp.changed() || invoke(app, "pageLayout.printGridlines") {
-            act(app, "pageLayout.printGridlines", json!({"on": !p}));
+            act(app, "pageLayout.printGridlines", json!({"on": checkbox_on(resp.changed(), p)}));
         }
     });
     ui.vertical(|ui| {
@@ -1713,13 +1719,13 @@ fn page_layout(app: &mut SheetApp, ui: &mut Ui) {
         let resp = ui.checkbox(&mut v, screen);
         control_keytip(app, ui, &resp, "view.headings");
         if resp.changed() || invoke(app, "view.headings") {
-            act(app, "view.headings", json!({"on": !v}));
+            act(app, "view.headings", json!({"on": checkbox_on(resp.changed(), v)}));
         }
         let mut p = sh.3;
         let resp = ui.checkbox(&mut p, print);
         control_keytip(app, ui, &resp, "pageLayout.printHeadings");
         if resp.changed() || invoke(app, "pageLayout.printHeadings") {
-            act(app, "pageLayout.printHeadings", json!({"on": !p}));
+            act(app, "pageLayout.printHeadings", json!({"on": checkbox_on(resp.changed(), p)}));
         }
     });
     let rtl = app.session.active().and_then(|d| d.wb.active()).is_some_and(|s| s.right_to_left);
@@ -2118,7 +2124,7 @@ fn view(app: &mut SheetApp, ui: &mut Ui) {
         let resp = ui.checkbox(&mut fb, lang.tr("Formula Bar"));
         control_keytip(app, ui, &resp, "view.formulaBar");
         if resp.changed() || invoke(app, "view.formulaBar") {
-            act(app, "view.formulaBar", json!({"on": !fb}));
+            act(app, "view.formulaBar", json!({"on": checkbox_on(resp.changed(), fb)}));
         }
         let mut g = s.0;
         let resp = ui.checkbox(&mut g, lang.tr("Gridlines"));
@@ -2482,3 +2488,18 @@ mod title_bar_tests {
         assert_eq!(MAC_CONTENT_LEFT - lights_end, margin);
     }
 }
+
+#[cfg(test)]
+mod checkbox_on_tests {
+    use super::*;
+
+    #[test]
+    fn click_sends_the_new_checkbox_value_and_key_tip_the_opposite() {
+        // Click: the checkbox bool already holds the new choice.
+        assert!(!checkbox_on(true, false));
+        assert!(checkbox_on(true, true));
+        // Key tip: the bool still holds the current setting, so toggle it.
+        assert!(checkbox_on(false, false));
+        assert!(!checkbox_on(false, true));
+    }
+}