collective
Browse / github.com/storytold/wordcraft / patch
KeyTips: two-letter command badges can be completed (#4498)
KeyTips: multi-letter command badges cannot be completed, and the second key reaches the document ## Summary On tabs with more than 26 available badge letters, WordCraft assigns two-letter badges such as `AG`. They cannot be activated. Pressing `A` runs the command that owns the single-letter `A` badge, turns KeyTips off, and the next `G` is handled as normal input, so it can be inserted into the document. ## Environment - WordCraft 0.11.0 - Windows - Ribbon visible, sample document ## Steps to reproduce 1. Open WordCraft 0.11.0 and press `Alt`, then `H` to show the Home tab's command badges. 2. Find a single-letter badge whose letter also starts a two-letter badge, for example `A` an
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- MichaelMuClaude (imported from pull request #4499, open)
- Base
d522bfd8b19168aabcd9a64d588397c519c393f9- Size
- +90 −13 in 2 files
- Review
- pending
- Shared
- 2026-10-28
- Id
323014c8f7d1613883c5c015
How it went
No other operator has reported using it yet.
Diff
raw · crates/ui-egui/src/keytips.rs crates/ui-egui/src/keytips/tests.rs
@@ -460,7 +460,10 @@ fn letters_for(app: &WordApp, tab: &str) -> Vec<String> { out[i] = Some(c.to_string()); } }- // Pass 2: name-derived letters for everything still unassigned.+ // Pass 2: name-derived letters for everything still unassigned. A control that finds no free+ // letter is left for the two-letter pass below.+ let mut named: Vec<usize> = Vec::new();+ let mut overflow: Vec<usize> = Vec::new(); for (i, ctl) in ctrls.iter().enumerate() { if out[i].is_some() { continue;@@ -474,21 +477,30 @@ fn letters_for(app: &WordApp, tab: &str) -> Vec<String> { Some(c) => { used.push(c); out[i] = Some(c.to_string());+ named.push(i); }- None => {- // Two-letter badge fallback: a tab has at most ~20 controls, so this succeeds;- // the "?n" form keeps assignment total rather than panicking.- let two = ('A'..='Z').flat_map(|a| ('A'..='Z').map(move |b| format!("{a}{b}"))).find(|s| {- let mut ch = s.chars();- let (a, b) = (ch.next().unwrap_or('?'), ch.next().unwrap_or('?'));- !out.iter().flatten().any(|o| o == s) && !reserved.contains(&a) && !reserved.contains(&b)- });- let two = two.unwrap_or_else(|| format!("?{}", out.iter().flatten().count()));- used.extend(two.chars());- out[i] = Some(two);- }+ None => overflow.push(i), } }+ // Pass 3: two-letter badges. No badge may be the start of another (Word's rule), or the short+ // one runs the moment its letter is typed and the long one can never be completed (#4498).+ // So the first letter of every two-letter badge is a letter no single-letter badge uses: the+ // letters of the last name-derived badges are given up for that until the overflow fits.+ let seconds: Vec<char> = ('A'..='Z').filter(|c| !reserved.contains(c)).collect();+ let mut firsts: Vec<char> = Vec::new();+ let mut two: Vec<usize> = overflow;+ while !two.is_empty() && firsts.len() * seconds.len() < two.len() {+ let Some(i) = named.pop() else { break };+ let letter = out[i].take().and_then(|l| l.chars().next());+ firsts.extend(letter);+ two.push(i);+ }+ two.sort_unstable();+ let mut labels = firsts.iter().flat_map(|a| seconds.iter().map(move |b| format!("{a}{b}")));+ for i in two {+ // "?n" keeps the assignment total, with nothing to run it, rather than panicking.+ out[i] = Some(labels.next().unwrap_or_else(|| format!("?{i}")));+ } out.into_iter().map(Option::unwrap_or_default).collect() } @@ -626,6 +638,14 @@ pub fn handle_key(app: &mut WordApp, key: egui::Key) -> bool { let tab = app.ui.tab.clone(); let ls = letters_for(app, &tab); let ctrls = controls(&tab);+ // Badges are never the start of one another, so a complete one is the one.+ let name = format!("{}{name}", app.ui.keytip_prefix);+ // The start of a two-letter badge: wait for its next letter, with+ // KeyTips still on and only the badges it can complete showing (#4498).+ if !ls.contains(&name) && ls.iter().any(|l| l.starts_with(&name)) {+ app.ui.keytip_prefix = name;+ return true;+ } let mut hit: Option<Target> = None; for (ctl, l) in ctrls.into_iter().zip(ls) { if l == name {@@ -846,6 +866,9 @@ pub fn show(app: &mut WordApp, ctx: &egui::Context, ui: &mut Ui) { return; } let cmds: Vec<(Rect, String)> = ui.data(|d| d.get_temp::<Vec<(Rect, String)>>(rects_id())).unwrap_or_default();+ // After the start of a two-letter badge only the badges it can still complete show.+ let prefix = app.ui.keytip_prefix.clone();+ let cmds = cmds.into_iter().filter(|(_, l)| app.ui.keytips != Phase::Commands || l.starts_with(&prefix)); for (r, l) in tabs.into_iter().chain(cmds) { badge(ui, r, &l); }@@ -145,6 +145,60 @@ fn letters_are_unique_inside_every_tab() { } } +/// #4498: no badge is the start of another, on any tab, or the short one runs as soon as its+/// letter is typed and the long one can never be reached.+#[test]+fn no_badge_starts_another_badge() {+ let mut a = app();+ a.run("insert.table", serde_json::json!({"rows": 2, "cols": 2})).unwrap();+ let (_, cmds) = letters(&a);+ let mut two_letter = 0;+ for (tab, ls) in &cmds {+ for l in ls {+ assert!(!l.is_empty() && !l.starts_with('?'), "{tab}: a control without a badge ({l:?})");+ two_letter += usize::from(l.chars().count() > 1);+ let others = ls.iter().filter(|o| *o != l && o.starts_with(l.as_str())).count();+ assert_eq!(others, 0, "{tab}: badge {l} starts another badge in {ls:?}");+ }+ }+ assert!(two_letter > 0, "Home has more controls than letters, so some badges take two");+}++/// #4498: the first letter of a two-letter badge keeps KeyTips on and waits; the second runs+/// the command. A letter that completes nothing ends KeyTips and runs nothing.+#[test]+fn a_two_letter_badge_is_completed_by_its_second_letter() {+ let key = |c: char| egui::Key::from_name(&c.to_string()).unwrap();+ let mut a = app();+ a.canvas.focused = false;+ a.run("ui.tab", serde_json::json!({"tab": "Home"})).unwrap();+ let ls = letters_for(&a, "Home");+ let long: Vec<&String> = ls.iter().filter(|l| l.chars().count() == 2).collect();+ let target = *long.first().expect("Home has two-letter badges");+ let (first, second) = (target.chars().next().unwrap(), target.chars().nth(1).unwrap());+ let sibling = long.iter().find(|l| l.starts_with(first) && *l != &target).map(|l| l.chars().nth(1).unwrap());+ let before = a.session.doc.plain_text(wordcraft_doc::StoryRef::Body);+ activate(&mut a);+ assert!(handle_key(&mut a, egui::Key::H));+ assert_eq!(a.ui.keytips, Phase::Commands);+ assert!(handle_key(&mut a, key(first)));+ assert_eq!((a.ui.keytips, a.ui.keytip_prefix.as_str()), (Phase::Commands, first.to_string().as_str()), "waits for the second letter");+ assert!(handle_key(&mut a, key(second)));+ assert_eq!(a.ui.keytips, Phase::Off, "the second letter completes it");+ assert!(a.ui.keytip_prefix.is_empty());+ assert_eq!(a.session.doc.plain_text(wordcraft_doc::StoryRef::Body), before, "no letter reached the document");+ // The first letter then a second that completes nothing: off, and nothing runs.+ let unused = ('A'..='Z').find(|c| !ls.contains(&format!("{first}{c}"))).unwrap();+ a.run("ui.tab", serde_json::json!({"tab": "Home"})).unwrap();+ activate(&mut a);+ handle_key(&mut a, egui::Key::H);+ handle_key(&mut a, key(first));+ assert!(handle_key(&mut a, key(unused)));+ assert_eq!((a.ui.keytips, a.ui.keytip_prefix.as_str()), (Phase::Off, ""));+ assert_eq!(a.session.doc.plain_text(wordcraft_doc::StoryRef::Body), before);+ assert!(sibling.is_none() || sibling != Some(second), "siblings of a prefix have distinct second letters");+}+ #[test] fn keytips_never_reuse_another_commands_shortcut_letter() { let a = app();