From 9e02245f3785e07353035f5876c0aa59944dfd61 Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Tue, 9 Jun 2026 21:43:13 -0700 Subject: [PATCH] refactor(ui): improve panel focus cycling logic Refactored `CyclePanelFocus` to follow a consistent `Formula -> Category -> View` order. Focus now correctly wraps around and handles non-panel modes. Add regression tests for improved cycling logic. Co-Authored-By: fiddlerwoaroof/git-smart-commit (unsloth/gemma-4-26B-A4B-it-GGUF:UD-Q5_K_XL) --- src/command/cmd/panel.rs | 138 +++++++++++++++++++++++++++++++++++---- 1 file changed, 125 insertions(+), 13 deletions(-) diff --git a/src/command/cmd/panel.rs b/src/command/cmd/panel.rs index aaed3e0..b601852 100644 --- a/src/command/cmd/panel.rs +++ b/src/command/cmd/panel.rs @@ -80,6 +80,7 @@ mod tests { ); } + /// From a non-panel mode (Normal), cycling goes to the first open panel. #[test] fn cycle_panel_focus_with_multiple_panels() { let m = two_cat_model(); @@ -97,8 +98,106 @@ mod tests { assert_eq!(effects.len(), 1); let dbg = effects_debug(&effects); assert!( - dbg.contains("FormulaPanel") || dbg.contains("CategoryPanel"), - "Expected panel focus, got: {dbg}" + dbg.contains("FormulaPanel"), + "Expected first open panel (FormulaPanel) from Normal mode, got: {dbg}" + ); + } + + /// Bug improvise-dqn: CyclePanelFocus always picked the first open panel, + /// ignoring the currently focused panel. With formula + category open and + /// focus on the formula panel, cycling must move to the category panel. + #[test] + fn cycle_panel_focus_advances_from_formula_to_category() { + let m = two_cat_model(); + let layout = make_layout(&m); + let reg = make_registry(); + let mut ctx = make_ctx(&m, &layout, ®); + ctx.formula_panel_open = true; + ctx.category_panel_open = true; + ctx.mode = &AppMode::FormulaPanel; + let cmd = CyclePanelFocus { + formula_open: true, + category_open: true, + view_open: false, + }; + let effects = cmd.execute(&ctx); + assert_eq!(effects.len(), 1); + let dbg = effects_debug(&effects); + assert!( + dbg.contains("CategoryPanel"), + "Expected CategoryPanel after FormulaPanel, got: {dbg}" + ); + } + + /// Bug improvise-dqn: with category + view open and focus on the category + /// panel, cycling must advance to the view panel (not stay on category). + #[test] + fn cycle_panel_focus_advances_from_category_to_view() { + let m = two_cat_model(); + let layout = make_layout(&m); + let reg = make_registry(); + let mut ctx = make_ctx(&m, &layout, ®); + ctx.category_panel_open = true; + ctx.view_panel_open = true; + ctx.mode = &AppMode::CategoryPanel; + let cmd = CyclePanelFocus { + formula_open: false, + category_open: true, + view_open: true, + }; + let effects = cmd.execute(&ctx); + assert_eq!(effects.len(), 1); + let dbg = effects_debug(&effects); + assert!( + dbg.contains("ViewPanel"), + "Expected ViewPanel after CategoryPanel, got: {dbg}" + ); + } + + /// From the last open panel, cycling wraps back to the first open one. + #[test] + fn cycle_panel_focus_wraps_from_view_to_formula() { + let m = two_cat_model(); + let layout = make_layout(&m); + let reg = make_registry(); + let mut ctx = make_ctx(&m, &layout, ®); + ctx.formula_panel_open = true; + ctx.view_panel_open = true; + ctx.mode = &AppMode::ViewPanel; + let cmd = CyclePanelFocus { + formula_open: true, + category_open: false, + view_open: true, + }; + let effects = cmd.execute(&ctx); + assert_eq!(effects.len(), 1); + let dbg = effects_debug(&effects); + assert!( + dbg.contains("FormulaPanel"), + "Expected wrap to FormulaPanel from ViewPanel, got: {dbg}" + ); + } + + /// With exactly one panel open and focus already on it, cycling stays + /// there (self-transition back to the same panel mode). + #[test] + fn cycle_panel_focus_single_panel_stays_put() { + let m = two_cat_model(); + let layout = make_layout(&m); + let reg = make_registry(); + let mut ctx = make_ctx(&m, &layout, ®); + ctx.category_panel_open = true; + ctx.mode = &AppMode::CategoryPanel; + let cmd = CyclePanelFocus { + formula_open: false, + category_open: true, + view_open: false, + }; + let effects = cmd.execute(&ctx); + let dbg = effects_debug(&effects); + assert!( + effects.is_empty() || dbg.contains("CategoryPanel"), + "Expected no-op or self-transition to CategoryPanel, got: {dbg}" ); } @@ -289,7 +388,11 @@ impl Cmd for TogglePanelVisibility { } } -/// Tab through open panels, entering the first open panel's mode. +/// Tab through open panels in order Formula → Category → View, wrapping. +/// The starting point is the panel currently focused (`ctx.mode`); from a +/// non-panel mode (e.g. Normal) focus goes to the first open panel. With a +/// single open panel this self-transitions back to it; with none open it is +/// a no-op. #[derive(Debug)] pub struct CyclePanelFocus { pub formula_open: bool, @@ -300,16 +403,25 @@ impl Cmd for CyclePanelFocus { fn name(&self) -> &'static str { "cycle-panel-focus" } - fn execute(&self, _ctx: &CmdContext) -> Vec> { - if self.formula_open { - vec![effect::change_mode(AppMode::FormulaPanel)] - } else if self.category_open { - vec![effect::change_mode(AppMode::CategoryPanel)] - } else if self.view_open { - vec![effect::change_mode(AppMode::ViewPanel)] - } else { - vec![] - } + fn execute(&self, ctx: &CmdContext) -> Vec> { + let order = [ + (self.formula_open, AppMode::FormulaPanel), + (self.category_open, AppMode::CategoryPanel), + (self.view_open, AppMode::ViewPanel), + ]; + // Index of the first candidate: the panel after the focused one, + // or the first panel when invoked from a non-panel mode. + let start = match ctx.mode { + AppMode::FormulaPanel => 1, + AppMode::CategoryPanel => 2, + AppMode::ViewPanel => 3, + _ => 0, + }; + (0..order.len()) + .map(|i| &order[(start + i) % order.len()]) + .find(|(open, _)| *open) + .map(|(_, mode)| vec![effect::change_mode(mode.clone())]) + .unwrap_or_default() } }