From 839dd27925f1fd1dccb837d38c21643d6654dac9 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Tue, 25 Aug 2026 13:26:54 +0100 Subject: [PATCH] feat(maestro): make TUI panel focus unmistakable --- crates/maestro-tui/src/render.rs | 121 +++++++++++++++++---- crates/maestro-tui/src/sidebar.rs | 173 +++++++++++++++++++++--------- 2 files changed, 223 insertions(+), 71 deletions(-) diff --git a/crates/maestro-tui/src/render.rs b/crates/maestro-tui/src/render.rs index 035dabf..562a160 100644 --- a/crates/maestro-tui/src/render.rs +++ b/crates/maestro-tui/src/render.rs @@ -108,36 +108,29 @@ pub fn draw(f: &mut Frame, inputs: RenderInputs<'_>) -> RegionMap { map.sidebar = Some(sidebar_area); } - // 1-row tab bar + 1-row horizontal rule (doubles as the pane's focus - // indicator) + flex pane area below. Saves the 4 rows the old - // `Length(3) + Borders::ALL` layout was eating before any content showed. + // The pane always owns its full border, so focus changes only recolour + // chrome and never change the rectangles handed to the input router. let rows = Layout::default() .direction(Direction::Vertical) - .constraints([ - Constraint::Length(1), - Constraint::Length(1), - Constraint::Min(0), - ]) + .constraints([Constraint::Length(1), Constraint::Min(0)]) .split(right); let tab_area = rows[0]; - let separator_area = rows[1]; - let pane_area = rows[2]; + let pane_area = rows[1]; inputs.tab_bar.render(f, tab_area); map.tab_bar = Some(tab_area); let pane_focused = inputs.state.focus == Focus::Pane; - f.render_widget( - Block::default() - .borders(Borders::TOP) - .border_style(focus_style(pane_focused)), - separator_area, - ); + let pane_block = Block::default() + .borders(Borders::ALL) + .border_style(focus_style(pane_focused)); + let pane_inner = pane_block.inner(pane_area); + f.render_widget(pane_block, pane_area); if let Some(p) = inputs.pane { - p.render(f, pane_area); + p.render(f, pane_inner); } if let Some(d) = inputs.draft { - d.render(f, pane_area, pane_focused); + d.render(f, pane_inner, pane_focused); } map.pane = Some(pane_area); @@ -351,4 +344,96 @@ mod tests { assert!(out.contains(s), "missing {s:?}:\n{out}"); } } + + #[test] + fn panel_rectangles_are_stable_when_focus_moves() { + use crate::overlays::HelpOverlay; + use crate::sidebar::Sidebar; + use std::collections::HashSet; + + let sidebar = Sidebar { + sessions: Vec::new(), + highlight: None, + attached: HashSet::new(), + }; + let tab_bar = TabBar { + tab_names: vec!["1".into()], + active_idx: 0, + }; + let help = HelpOverlay { lines: Vec::new() }; + let draw_map = |focus| { + let mut state = AppState::new(); + state.focus = focus; + let inputs = RenderInputs { + sidebar: &sidebar, + tab_bar: &tab_bar, + pane: None, + draft: None, + state: &state, + help: &help, + worktrees: None, + }; + let backend = ratatui::backend::TestBackend::new(80, 24); + let mut terminal = ratatui::Terminal::new(backend).unwrap(); + let mut map = None; + terminal.draw(|f| map = Some(draw(f, inputs))).unwrap(); + map.unwrap() + }; + + let sidebar_focus = draw_map(Focus::Sidebar); + let pane_focus = draw_map(Focus::Pane); + assert_eq!(sidebar_focus.sidebar, pane_focus.sidebar); + assert_eq!(sidebar_focus.tab_bar, pane_focus.tab_bar); + assert_eq!(sidebar_focus.pane, pane_focus.pane); + } + + #[test] + fn panel_border_colours_follow_focus_without_restyling_the_pane() { + use crate::overlays::HelpOverlay; + use crate::sidebar::Sidebar; + use std::collections::HashSet; + + let sidebar = Sidebar { + sessions: Vec::new(), + highlight: None, + attached: HashSet::new(), + }; + let tab_bar = TabBar { + tab_names: vec!["1".into()], + active_idx: 0, + }; + let help = HelpOverlay { lines: Vec::new() }; + let render = |focus| { + let mut state = AppState::new(); + state.focus = focus; + let inputs = RenderInputs { + sidebar: &sidebar, + tab_bar: &tab_bar, + pane: None, + draft: None, + state: &state, + help: &help, + worktrees: None, + }; + let backend = ratatui::backend::TestBackend::new(80, 24); + let mut terminal = ratatui::Terminal::new(backend).unwrap(); + terminal + .draw(|f| { + draw(f, inputs); + }) + .unwrap(); + terminal.backend().buffer().clone() + }; + + let sidebar_focused = render(Focus::Sidebar); + let pane_focused = render(Focus::Pane); + let p = palette::active(); + assert_eq!(sidebar_focused[(0, 0)].style().fg, Some(p.border_focused)); + assert_eq!(pane_focused[(0, 0)].style().fg, Some(p.border_unfocused)); + assert_eq!( + sidebar_focused[(28, 1)].style().fg, + Some(p.border_unfocused) + ); + assert_eq!(pane_focused[(28, 1)].style().fg, Some(p.border_focused)); + } } diff --git a/crates/maestro-tui/src/sidebar.rs b/crates/maestro-tui/src/sidebar.rs index 960de55..84ddbb9 100644 --- a/crates/maestro-tui/src/sidebar.rs +++ b/crates/maestro-tui/src/sidebar.rs @@ -31,12 +31,13 @@ const META_INDENT: &str = " "; /// Spawn-similar and kill, shown on the highlighted card's top-right corner. const ACTIONS: &str = "⧉ ✕"; -/// Every card sits on the palette's card surface so cards read as cards -/// against the panel; the highlighted card swaps in the selection surface -/// instead (`Modifier::REVERSED` made the card unreadable against a -/// background). -fn card_base(highlighted: bool) -> Style { - palette::active().surface(highlighted).text_style() +/// Cards use the selection surface only while the sidebar owns focus. An +/// unfocused highlight remains an edge-marked cursor without claiming the +/// keyboard via a filled selection background. +fn card_base(highlighted: bool, focused: bool) -> Style { + palette::active() + .surface(highlighted && focused) + .text_style() } /// Renderable sidebar state. @@ -52,14 +53,13 @@ impl Sidebar { self.render_with_focus(f, area, false); } - /// Same as [`Sidebar::render`] but tints the right-edge separator in the - /// focus colour when `focused`. Box borders eat too many rows/cols for - /// the v1 layout — a single vertical rule keeps the panel boundary - /// without stealing space. + /// Render the panel with a persistent border. Focus changes its colour + /// and whether the highlighted card claims the selection surface, never + /// its geometry. pub fn render_with_focus(&self, f: &mut Frame, area: Rect, focused: bool) { let border_style = palette::active().border_style(focused); let block = Block::default() - .borders(Borders::RIGHT) + .borders(Borders::ALL) .border_style(border_style); let inner = block.inner(area); f.render_widget(block, area); @@ -71,22 +71,22 @@ impl Sidebar { ))]; for s in &self.sessions { let highlighted = self.highlight == Some(s.id); - let base = card_base(highlighted); - lines.push(Line::from(self.name_row(s, width))); - let meta = self.meta_row(s, width); + let base = card_base(highlighted, focused); + lines.push(Line::from(self.name_row(s, width, focused))); + let meta = self.meta_row(s, width, focused); lines.push(Line::from(pad_to_width(meta, width, base))); } f.render_widget(Paragraph::new(lines), inner); } - fn name_row(&self, s: &Session, width: usize) -> Vec> { + fn name_row(&self, s: &Session, width: usize, focused: bool) -> Vec> { let glyph = match (s.status, self.attached.contains(&s.id)) { (SessionStatus::Dead, _) => "✗", (SessionStatus::Running, true) => "●", (SessionStatus::Running, false) => "○", }; let highlighted = self.highlight == Some(s.id); - let base = card_base(highlighted); + let base = card_base(highlighted, focused); // Line-1 actions are keyboard affordances shown on the highlighted // row only; they reserve their width so the name truncates with `…` // before it can ever reach them (docs/SIDEBAR.md). @@ -102,8 +102,16 @@ impl Sidebar { let sep = if highlighted { 1 } else { 0 }; let name_budget = usable.saturating_sub(prefix + sep); + let edge = highlighted && !focused; let mut spans: Vec> = vec![ - Span::styled(INDENT, base), + Span::styled( + if edge { "▏ " } else { INDENT }, + if edge { + base.fg(palette::active().focus_accent) + } else { + base + }, + ), Span::styled(format!("{glyph} "), base), ]; spans.extend( @@ -122,7 +130,7 @@ impl Sidebar { } /// The adaptive second line: ` [] [] [ | exited] `. - fn meta_row(&self, s: &Session, width: usize) -> Vec> { + fn meta_row(&self, s: &Session, width: usize, focused: bool) -> Vec> { let now = Utc::now(); let held = held_for(s, now); let held_secs = held.map(|d| d.num_seconds()).unwrap_or(0); @@ -131,11 +139,22 @@ impl Sidebar { let blocked = s.blocked_work.filter(|b| !b.is_clear()); let dead = s.status == SessionStatus::Dead; let highlighted = self.highlight == Some(s.id); - let base = card_base(highlighted); - let dot_style = base.patch(Style::default().fg(activity_color(s.activity, highlighted))); - let muted = palette::active().surface(highlighted).muted_style(); + let base = card_base(highlighted, focused); + let dot_style = + base.patch(Style::default().fg(activity_color(s.activity, highlighted, focused))); + let muted = palette::active() + .surface(highlighted && focused) + .muted_style(); + let edge = highlighted && !focused; let mut spans = vec![ - Span::styled(META_INDENT, base), + Span::styled( + if edge { " ▏ " } else { META_INDENT }, + if edge { + base.fg(palette::active().focus_accent) + } else { + base + }, + ), Span::styled("●", dot_style), ]; if let Some(d) = held.filter(|_| held_secs >= META_LINE_AFTER_SECS) { @@ -179,8 +198,10 @@ impl Sidebar { /// state word is never spelled out — this colour carries it. The highlighted /// card takes its state colours from the selection surface: a colour picked /// for the dark card background is not legible on the selection background. -fn activity_color(state: ActivityState, highlighted: bool) -> Color { - palette::active().surface(highlighted).activity(state) +fn activity_color(state: ActivityState, highlighted: bool, focused: bool) -> Color { + palette::active() + .surface(highlighted && focused) + .activity(state) } /// docs/SIDEBAR.md duration format — floored whole seconds / minutes / hours, @@ -281,9 +302,15 @@ mod tests { } fn draw(sidebar: &Sidebar, w: u16, h: u16) -> Buffer { + draw_with_focus(sidebar, w, h, true) + } + + fn draw_with_focus(sidebar: &Sidebar, w: u16, h: u16, focused: bool) -> Buffer { let backend = TestBackend::new(w, h); let mut terminal = Terminal::new(backend).unwrap(); - terminal.draw(|f| sidebar.render(f, f.area())).unwrap(); + terminal + .draw(|f| sidebar.render_with_focus(f, f.area(), focused)) + .unwrap(); terminal.backend().buffer().clone() } @@ -309,7 +336,10 @@ mod tests { /// Row text without the trailing space padding and right border. fn content(row: &str) -> &str { - row.trim_end().trim_end_matches('│').trim_end() + row.trim() + .trim_start_matches('│') + .trim_end_matches('│') + .trim() } #[test] @@ -417,22 +447,22 @@ mod tests { attached: Default::default(), }; let all = rows(&draw(&sb, 28, 8)); - assert_eq!(content(&all[0]), "sessions"); + assert_eq!(content(&all[1]), "sessions"); assert!(row_containing(&all, "claude").contains('○')); assert!( - all[2].contains('●'), + all[3].contains('●'), "line 2 must carry the dot: {:?}", - all[2] + all[3] ); assert!( - all[2].contains("bash"), + all[3].contains("bash"), "line 2 must carry the command: {:?}", - all[2] + all[3] ); assert!( - !all[2].contains("10s"), + !all[3].contains("10s"), "fresh cards have no duration: {:?}", - all[2] + all[3] ); } @@ -660,7 +690,7 @@ mod tests { highlight: None, attached: Default::default(), }; - let all = rows(&draw(&sb, 40, 8)); + let all = rows(&draw(&sb, 42, 8)); let idx = all.iter().position(|l| l.contains("claude")).unwrap(); let meta = content(&all[idx + 1]); assert!(meta.contains('…'), "truncated detail ends with …: {meta:?}"); @@ -752,14 +782,14 @@ mod tests { }; let all = rows(&draw(&sb, 40, 6)); assert!( - all[1].contains("friendly name"), + all[2].contains("friendly name"), "name must stay on line 1: {:?}", - all[1] + all[2] ); assert!( - all[2].contains("bash"), + all[3].contains("bash"), "command must be on line 2: {:?}", - all[2] + all[3] ); } @@ -773,14 +803,14 @@ mod tests { }; let all = rows(&draw(&sb, 10, 6)); assert!( - all[2].contains('●'), + all[3].contains('●'), "dot must survive narrow cards: {:?}", - all[2] + all[3] ); assert!( - all[2].contains('…'), + all[3].contains('…'), "command should truncate: {:?}", - all[2] + all[3] ); } @@ -798,8 +828,8 @@ mod tests { attached: Default::default(), }; let all = rows(&draw(&sb, 29, 6)); - assert!(all[2].contains("⚠ 1 uncommitted file"), "{:?}", all[2]); - let meta = content(&all[2]); + assert!(all[3].contains("⚠ 1 uncommitted file"), "{:?}", all[3]); + let meta = content(&all[3]); assert!(meta.ends_with('…'), "command must truncate: {meta:?}"); } @@ -817,13 +847,13 @@ mod tests { attached: Default::default(), }; let all = rows(&draw(&sb, 40, 6)); - assert!(all[2].contains("⚠ 1 uncommitted file"), "{:?}", all[2]); + assert!(all[3].contains("⚠ 1 uncommitted file"), "{:?}", all[3]); assert!( - all[2].contains("bash"), + all[3].contains("bash"), "command must survive: {:?}", - all[2] + all[3] ); - assert!(all[2].contains('…'), "detail must truncate: {:?}", all[2]); + assert!(all[3].contains('…'), "detail must truncate: {:?}", all[3]); } /// Reversed video makes a card unreadable against a background; cards @@ -869,27 +899,64 @@ mod tests { let p = palette::active(); assert_eq!( - buf[(0, claude_y as u16)].style().bg, + buf[(1, claude_y as u16)].style().bg, Some(p.selection.bg), "focused card must sit on the selection surface" ); assert_eq!( - buf[(0, claude_y as u16)].style().fg, + buf[(1, claude_y as u16)].style().fg, Some(p.selection.fg), "focused card text must use the selection foreground role" ); assert_eq!( - buf[(0, bash_y as u16)].style().bg, + buf[(1, bash_y as u16)].style().bg, Some(p.card.bg), "unfocused cards must still sit on the card surface" ); // The panel itself (header row) stays on the terminal default. assert!( - matches!(buf[(0, 0)].style().bg, None | Some(Color::Reset)), + matches!(buf[(1, 1)].style().bg, None | Some(Color::Reset)), "panel must keep the terminal default background" ); } + #[test] + fn highlighted_card_is_hollow_when_sidebar_is_unfocused() { + let sb = Sidebar { + sessions: vec![session_named(1, "claude")], + highlight: Some(Uuid::from_u128(1)), + attached: Default::default(), + }; + let buf = draw_with_focus(&sb, 28, 8, false); + let y = (0..buf.area.height) + .find(|&y| (0..buf.area.width).any(|x| buf[(x, y)].symbol() == "c")) + .expect("highlighted session row"); + assert_eq!( + buf[(1, y)].style().bg, + Some(palette::active().card.bg), + "unfocused highlight must not paint the selection surface" + ); + assert!( + (0..buf.area.width).any(|x| buf[(x, y)].symbol() == "▏"), + "unfocused highlight must retain a visible edge marker" + ); + let ordinary = draw_with_focus( + &Sidebar { + sessions: vec![session_named(1, "claude")], + highlight: None, + attached: Default::default(), + }, + 28, + 8, + false, + ); + assert!( + !(0..ordinary.area.height) + .any(|row| (0..ordinary.area.width).any(|x| ordinary[(x, row)].symbol() == "▏")), + "ordinary unhighlighted cards must not carry the cursor marker" + ); + } + /// The card background paints the full card width, not just the glyphs. #[test] fn card_background_covers_the_full_card_width() { @@ -901,7 +968,7 @@ mod tests { }; let buf = draw(&sb, 28, 8); let y = buf.content.iter().position(|c| c.symbol() == "c").unwrap() as u16 / buf.area.width; - for x in 0..27 { + for x in 1..27 { assert_eq!( buf[(x, y)].style().bg, Some(palette::active().card.bg),