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)
This commit is contained in:
+125
-13
@@ -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<Box<dyn Effect>> {
|
||||
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<Box<dyn Effect>> {
|
||||
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()
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user