From 84d771b88ac11fe8ce0a91f8411d2739dd350a11 Mon Sep 17 00:00:00 2001 From: Eric Curtin Date: Sat, 6 Jun 2026 13:32:54 +0100 Subject: [PATCH] refactor(tui): extract shared draw_text_field and draw_confirm_popup helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The openshell-tui UI module had two private draw_text_field functions with near-identical logic in create_sandbox.rs and create_provider.rs (differing only in cursor glyph and whether a gap row is shown), and four confirm-dialog rendering blocks duplicated across global_settings.rs and sandbox_settings.rs. Extract both patterns into module-level helpers in ui/mod.rs: - draw_text_field: accepts cursor (&str) and show_gap (bool) to cover both callers; create_sandbox uses '█'/true, create_provider uses '_'/false. - draw_confirm_popup: takes the already-built lines vec, a border style, a width, and the enclosing area, and owns the Clear + Block + Paragraph rendering that was duplicated in all four confirm functions. Remove the now-redundant imports (Clear, Paragraph) from the settings files and drop the local use super::centered_popup as centered_rect aliases that are no longer needed. Signed-off-by: Eric Curtin --- .../openshell-tui/src/ui/create_provider.rs | 46 ++--------- crates/openshell-tui/src/ui/create_sandbox.rs | 55 +++---------- .../openshell-tui/src/ui/global_settings.rs | 27 +------ crates/openshell-tui/src/ui/mod.rs | 80 +++++++++++++++++++ .../openshell-tui/src/ui/sandbox_settings.rs | 26 +----- 5 files changed, 101 insertions(+), 133 deletions(-) diff --git a/crates/openshell-tui/src/ui/create_provider.rs b/crates/openshell-tui/src/ui/create_provider.rs index 94e545ad2..a0896aa46 100644 --- a/crates/openshell-tui/src/ui/create_provider.rs +++ b/crates/openshell-tui/src/ui/create_provider.rs @@ -279,7 +279,7 @@ fn draw_enter_key( // Name field. let name_placeholder = format!("optional (defaults to {selected_type})"); - draw_text_field( + super::draw_text_field( frame, "Name", &form.name, @@ -287,6 +287,8 @@ fn draw_enter_key( form.key_field == ProviderKeyField::Name, chunks[idx], t, + "_", + false, ); idx += 1; @@ -295,7 +297,7 @@ fn draw_enter_key( if form.is_generic { // Env var name field. - draw_text_field( + super::draw_text_field( frame, "Env var name", &form.generic_env_name, @@ -303,6 +305,8 @@ fn draw_enter_key( form.key_field == ProviderKeyField::EnvVarName, chunks[idx], t, + "_", + false, ); idx += 1; @@ -773,44 +777,6 @@ pub fn draw_update(frame: &mut Frame<'_>, app: &App, area: Rect) { // Helpers // --------------------------------------------------------------------------- -fn draw_text_field( - frame: &mut Frame<'_>, - label: &str, - value: &str, - placeholder: &str, - focused: bool, - area: Rect, - theme: &crate::theme::Theme, -) { - let t = theme; - let chunks = Layout::default() - .direction(Direction::Vertical) - .constraints([ - Constraint::Length(1), // label - Constraint::Length(1), // input - ]) - .split(area); - - let label_style = if focused { t.accent_bold } else { t.text }; - let mut label_spans = vec![Span::styled(format!("{label}:"), label_style)]; - if !placeholder.is_empty() { - label_spans.push(Span::styled(format!(" {placeholder}"), t.muted)); - } - frame.render_widget(Paragraph::new(Line::from(label_spans)), chunks[0]); - - let display = if value.is_empty() && !focused { - Line::from(Span::styled(" -", t.muted)) - } else if focused { - Line::from(vec![ - Span::styled(format!(" {value}"), t.accent), - Span::styled("_", t.accent), - ]) - } else { - Line::from(Span::styled(format!(" {value}"), t.text)) - }; - frame.render_widget(Paragraph::new(display), chunks[1]); -} - fn draw_secret_field( frame: &mut Frame<'_>, label: &str, diff --git a/crates/openshell-tui/src/ui/create_sandbox.rs b/crates/openshell-tui/src/ui/create_sandbox.rs index a120036cf..154a94e7e 100644 --- a/crates/openshell-tui/src/ui/create_sandbox.rs +++ b/crates/openshell-tui/src/ui/create_sandbox.rs @@ -75,7 +75,7 @@ fn draw_form(frame: &mut Frame<'_>, app: &App, area: Rect) { .split(inner); // --- Name --- - draw_text_field( + super::draw_text_field( frame, "Name", &form.name, @@ -83,10 +83,12 @@ fn draw_form(frame: &mut Frame<'_>, app: &App, area: Rect) { form.focused_field == CreateFormField::Name, chunks[0], t, + "█", + true, ); // --- Image --- - draw_text_field( + super::draw_text_field( frame, "Image", &form.image, @@ -94,10 +96,12 @@ fn draw_form(frame: &mut Frame<'_>, app: &App, area: Rect) { form.focused_field == CreateFormField::Image, chunks[1], t, + "█", + true, ); // --- Command --- - draw_text_field( + super::draw_text_field( frame, "Command", &form.command, @@ -105,6 +109,8 @@ fn draw_form(frame: &mut Frame<'_>, app: &App, area: Rect) { form.focused_field == CreateFormField::Command, chunks[2], t, + "█", + true, ); // --- Providers label --- @@ -377,46 +383,3 @@ pub fn render_chase( Line::from(spans) } - -// --------------------------------------------------------------------------- -// Form helpers -// --------------------------------------------------------------------------- - -fn draw_text_field( - frame: &mut Frame<'_>, - label: &str, - value: &str, - placeholder: &str, - focused: bool, - area: Rect, - theme: &crate::theme::Theme, -) { - let t = theme; - let chunks = Layout::default() - .direction(Direction::Vertical) - .constraints([ - Constraint::Length(1), // label - Constraint::Length(1), // input - Constraint::Length(1), // gap - ]) - .split(area); - - let label_style = if focused { t.accent_bold } else { t.text }; - let mut label_spans = vec![Span::styled(format!("{label}:"), label_style)]; - if !placeholder.is_empty() { - label_spans.push(Span::styled(format!(" {placeholder}"), t.muted)); - } - frame.render_widget(Paragraph::new(Line::from(label_spans)), chunks[0]); - - let display = if value.is_empty() && !focused { - Line::from(Span::styled(" -", t.muted)) - } else if focused { - Line::from(vec![ - Span::styled(format!(" {value}"), t.accent), - Span::styled("█", t.accent), - ]) - } else { - Line::from(Span::styled(format!(" {value}"), t.text)) - }; - frame.render_widget(Paragraph::new(display), chunks[1]); -} diff --git a/crates/openshell-tui/src/ui/global_settings.rs b/crates/openshell-tui/src/ui/global_settings.rs index fa47f225a..f20364009 100644 --- a/crates/openshell-tui/src/ui/global_settings.rs +++ b/crates/openshell-tui/src/ui/global_settings.rs @@ -4,7 +4,7 @@ use ratatui::Frame; use ratatui::layout::{Constraint, Rect}; use ratatui::text::{Line, Span}; -use ratatui::widgets::{Block, Borders, Cell, Clear, Padding, Paragraph, Row, Table}; +use ratatui::widgets::{Block, Borders, Cell, Padding, Row, Table}; use super::draw_empty_message; @@ -133,10 +133,6 @@ fn draw_confirm_set(frame: &mut Frame<'_>, app: &App, idx: usize, area: Rect) { }; let new_value = app.setting_edit.as_ref().map_or("-", |e| e.input.as_str()); - // 7 content lines + 2 border rows = 9 outer height. - let popup = centered_rect(60, 9, area); - frame.render_widget(Clear, popup); - let lines = vec![ Line::from(Span::styled(" Confirm Global Setting Change ", t.heading)), Line::from(""), @@ -161,12 +157,7 @@ fn draw_confirm_set(frame: &mut Frame<'_>, app: &App, idx: usize, area: Rect) { ]), ]; - let block = Block::default() - .borders(Borders::ALL) - .border_style(t.border_focused) - .padding(Padding::horizontal(1)); - - frame.render_widget(Paragraph::new(lines).block(block), popup); + super::draw_confirm_popup(frame, lines, t.border_focused, 60, area); } fn draw_confirm_delete(frame: &mut Frame<'_>, app: &App, idx: usize, area: Rect) { @@ -197,17 +188,5 @@ fn draw_confirm_delete(frame: &mut Frame<'_>, app: &App, idx: usize, area: Rect) ]), ]; - // content lines + 2 for border - let popup_height = u16::try_from(lines.len() + 2).unwrap_or(u16::MAX); - let popup = centered_rect(60, popup_height, area); - frame.render_widget(Clear, popup); - - let block = Block::default() - .borders(Borders::ALL) - .border_style(t.status_err) - .padding(Padding::horizontal(1)); - - frame.render_widget(Paragraph::new(lines).block(block), popup); + super::draw_confirm_popup(frame, lines, t.status_err, 60, area); } - -use super::centered_popup as centered_rect; diff --git a/crates/openshell-tui/src/ui/mod.rs b/crates/openshell-tui/src/ui/mod.rs index c8affc7fb..f44b9eef1 100644 --- a/crates/openshell-tui/src/ui/mod.rs +++ b/crates/openshell-tui/src/ui/mod.rs @@ -569,6 +569,86 @@ fn draw_setting_edit_overlay( frame.render_widget(Paragraph::new(lines).block(block), popup); } +/// Draw a labeled text input field. +/// +/// `cursor` is the cursor character shown when `focused` (e.g. `"█"` or `"_"`). +/// `show_gap` appends an extra blank row below the input line. +#[allow(clippy::too_many_arguments)] +fn draw_text_field( + frame: &mut Frame<'_>, + label: &str, + value: &str, + placeholder: &str, + focused: bool, + area: Rect, + theme: &Theme, + cursor: &str, + show_gap: bool, +) { + let t = theme; + let chunks = if show_gap { + Layout::default() + .direction(Direction::Vertical) + .constraints([ + Constraint::Length(1), // label + Constraint::Length(1), // input + Constraint::Length(1), // gap + ]) + .split(area) + } else { + Layout::default() + .direction(Direction::Vertical) + .constraints([ + Constraint::Length(1), // label + Constraint::Length(1), // input + ]) + .split(area) + }; + + let label_style = if focused { t.accent_bold } else { t.text }; + let mut label_spans = vec![Span::styled(format!("{label}:"), label_style)]; + if !placeholder.is_empty() { + label_spans.push(Span::styled(format!(" {placeholder}"), t.muted)); + } + frame.render_widget(Paragraph::new(Line::from(label_spans)), chunks[0]); + + let display = if value.is_empty() && !focused { + Line::from(Span::styled(" -", t.muted)) + } else if focused { + Line::from(vec![ + Span::styled(format!(" {value}"), t.accent), + Span::styled(cursor, t.accent), + ]) + } else { + Line::from(Span::styled(format!(" {value}"), t.text)) + }; + frame.render_widget(Paragraph::new(display), chunks[1]); +} + +/// Render a bordered confirmation popup centred on `area`. +/// +/// `lines` is the popup body; height is derived from the line count. +/// Use `t.border_focused` for confirmations and `t.status_err` for destructive +/// actions. +fn draw_confirm_popup( + frame: &mut Frame<'_>, + lines: Vec>, + border_style: Style, + width: u16, + area: Rect, +) { + let popup_height = u16::try_from(lines.len() + 2).unwrap_or(u16::MAX); + let popup = centered_popup(width, popup_height, area); + frame.render_widget(Clear, popup); + + let block = Block::default() + .borders(Borders::ALL) + .border_style(border_style) + .padding(Padding::horizontal(1)); + + frame.render_widget(Paragraph::new(lines).block(block), popup); +} + /// Center a popup rectangle within `area` using percentage-based width and /// an absolute height (in rows). pub fn centered_popup(percent_x: u16, height: u16, area: Rect) -> Rect { diff --git a/crates/openshell-tui/src/ui/sandbox_settings.rs b/crates/openshell-tui/src/ui/sandbox_settings.rs index ef03c89be..a15e1417a 100644 --- a/crates/openshell-tui/src/ui/sandbox_settings.rs +++ b/crates/openshell-tui/src/ui/sandbox_settings.rs @@ -4,7 +4,7 @@ use ratatui::Frame; use ratatui::layout::{Constraint, Rect}; use ratatui::text::{Line, Span}; -use ratatui::widgets::{Block, Borders, Cell, Clear, Padding, Paragraph, Row, Table}; +use ratatui::widgets::{Block, Borders, Cell, Padding, Row, Table}; use super::draw_empty_message; use crate::app::{App, SandboxPolicyTab, SettingScope}; @@ -166,16 +166,7 @@ fn draw_confirm_set(frame: &mut Frame<'_>, app: &App, idx: usize, area: Rect) { ]), ]; - let popup_height = u16::try_from(lines.len() + 2).unwrap_or(u16::MAX); - let popup = centered_rect(60, popup_height, area); - frame.render_widget(Clear, popup); - - let block = Block::default() - .borders(Borders::ALL) - .border_style(t.border_focused) - .padding(Padding::horizontal(1)); - - frame.render_widget(Paragraph::new(lines).block(block), popup); + super::draw_confirm_popup(frame, lines, t.border_focused, 60, area); } fn draw_confirm_delete(frame: &mut Frame<'_>, app: &App, idx: usize, area: Rect) { @@ -204,16 +195,5 @@ fn draw_confirm_delete(frame: &mut Frame<'_>, app: &App, idx: usize, area: Rect) ]), ]; - let popup_height = u16::try_from(lines.len() + 2).unwrap_or(u16::MAX); - let popup = centered_rect(55, popup_height, area); - frame.render_widget(Clear, popup); - - let block = Block::default() - .borders(Borders::ALL) - .border_style(t.status_err) - .padding(Padding::horizontal(1)); - - frame.render_widget(Paragraph::new(lines).block(block), popup); + super::draw_confirm_popup(frame, lines, t.status_err, 55, area); } - -use super::centered_popup as centered_rect;