dialogs: complete P1-P4 unification (PLAN §17.4)
Executes the four high-priority items from PLAN §17.4 backlog,
bringing all 4 of the remaining bespoke dialog surfaces onto the
unified render_popup / render_button_row path.
P1 (HIGH) widget/dialog.rs — Dialog::render now delegates the
shell to terminal::popup::render_popup instead of building
its own raw Clear + Block. This closes the only shadow
regression in the unified stack: Dialog::info / confirm /
confirm_all now inherit MC-matching rounded borders +
drop shadow + Clear + skin-driven palette, identical to
every other migrated dialog. New test
render_draws_drop_shadow_at_bottom_right_corner asserts the
shadow bg is painted at the popup's bottom-right cell.
P2 (HIGH) overwrite_dialog.rs — Y/N/All/Skip/Abort legend replaced
by 5-element render_button_row (Yes=Default, No=All,
SkipAll, Abort=Normal). Was 40 lines of hand-rolled Span
with hand-picked theme.executable / theme.error /
theme.title_bg / theme.warning colors. New test
render_uses_mc_bracket_button_shapes asserts Default uses
'[<' / '>]' brackets and Normal uses '[ ' / ' ]'.
P3 (MED) confirm_dialog.rs — '[ Save ] [ Cancel ]' Paragraph
replaced by 2-element render_button_row (Save=Default,
Cancel=Normal). New test asserts the same MC bracket shapes.
P4 (MED) editor/menubar.rs — F9 dropdown migrated to render_popup,
matching the already-migrated filemanager/menubar.rs
dropdown (committed in c032c9a787). Top menu bar (full-width
by design) intentionally untouched.
Acceptance criterion from §17.6 is met: grep for raw Clear returns
exactly 4 sites, all defensible:
- terminal/popup.rs (the renderer itself)
- filemanager/tree.rs (full-screen, intentional)
- filemanager/menubar.rs top bar (intentional full-width)
- editor/menubar.rs top bar (intentional full-width)
widget/dialog.rs no longer contains a raw Clear.
Tests: 1184 passing, 0 failing (was 1180; +4 new). Both release
builds (default + --features sftp) clean.
This commit is contained in:
@@ -7,12 +7,13 @@
|
||||
use ratatui::layout::{Constraint, Direction, Layout, Rect};
|
||||
use ratatui::style::{Modifier, Style};
|
||||
use ratatui::text::{Line, Span};
|
||||
use ratatui::widgets::{Block, Borders, Clear, Paragraph};
|
||||
use ratatui::widgets::{Clear, Paragraph};
|
||||
use ratatui::Frame;
|
||||
|
||||
use crate::key::Key;
|
||||
use crate::terminal::color::Theme;
|
||||
use crate::terminal::mc_skin;
|
||||
use crate::terminal::popup::render_popup;
|
||||
|
||||
/// An editor command surfaced by the menu bar.
|
||||
///
|
||||
@@ -517,26 +518,7 @@ impl EditorMenuBar {
|
||||
let y = 1u16;
|
||||
let dropdown_area = Rect::new(x, y, dropdown_w, dropdown_h);
|
||||
|
||||
frame.render_widget(Clear, dropdown_area);
|
||||
|
||||
let block = Block::default()
|
||||
.borders(Borders::ALL)
|
||||
.border_style(
|
||||
Style::default().fg(theme.title_fg).bg(
|
||||
mc_skin::color_pair(theme.name, "menu", "_default_")
|
||||
.map(|p| p.bg)
|
||||
.unwrap_or(theme.background),
|
||||
),
|
||||
)
|
||||
.title(Span::styled(
|
||||
format!(" {} ", menu.title),
|
||||
Style::default()
|
||||
.fg(theme.title_fg)
|
||||
.bg(theme.title_bg)
|
||||
.add_modifier(Modifier::BOLD),
|
||||
));
|
||||
let inner = block.inner(dropdown_area);
|
||||
frame.render_widget(block, dropdown_area);
|
||||
let inner = render_popup(frame, dropdown_area, menu.title.clone(), theme);
|
||||
|
||||
let chunks = Layout::default()
|
||||
.direction(Direction::Vertical)
|
||||
|
||||
@@ -9,6 +9,7 @@ use ratatui::widgets::Paragraph;
|
||||
use ratatui::Frame;
|
||||
|
||||
use crate::terminal::popup::{centered_cols_rect, render_popup};
|
||||
use crate::widget::button::{render_button_row, ButtonKind, ButtonSpec};
|
||||
|
||||
const TOGGLE_LABELS: &[(&str, &str)] = &[
|
||||
("Confirm delete", "Ask before deleting files"),
|
||||
@@ -183,12 +184,11 @@ impl ConfirmDialog {
|
||||
frame.render_widget(Paragraph::new(line), chunks[i + 1]);
|
||||
}
|
||||
|
||||
let footer = Paragraph::new(Line::from(Span::styled(
|
||||
" [ Save ] [ Cancel ] ",
|
||||
Style::default().fg(theme.hidden),
|
||||
)))
|
||||
.alignment(Alignment::Center);
|
||||
frame.render_widget(footer, chunks[TOGGLE_LABELS.len() + 1]);
|
||||
let specs: Vec<ButtonSpec<'_>> = vec![
|
||||
ButtonSpec { label: "Save", hotkey: Some('S'), kind: ButtonKind::Default },
|
||||
ButtonSpec { label: "Cancel", hotkey: Some('C'), kind: ButtonKind::Normal },
|
||||
];
|
||||
render_button_row(frame, chunks[TOGGLE_LABELS.len() + 1], &specs, 0, theme);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -275,4 +275,46 @@ mod tests {
|
||||
s.set(3, false);
|
||||
assert!(!s.at(3));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_footer_uses_mc_bracket_button_shapes() {
|
||||
// After P3, the "[ Save ] [ Cancel ]" text strip is replaced
|
||||
// by render_button_row. Verify Save (Default, focused) renders
|
||||
// with angle brackets and Cancel (Normal) renders with square
|
||||
// brackets.
|
||||
use ratatui::backend::TestBackend;
|
||||
use ratatui::Terminal;
|
||||
let backend = TestBackend::new(WIDTH + 4, HEIGHT + 4);
|
||||
let mut terminal = Terminal::new(backend).unwrap();
|
||||
let theme = *crate::terminal::color::DEFAULT_THEME;
|
||||
terminal
|
||||
.draw(|f| {
|
||||
let mut d = ConfirmDialog::new(default_settings());
|
||||
d.render(f, f.area(), &theme);
|
||||
})
|
||||
.unwrap();
|
||||
let buf = terminal.backend().buffer().clone();
|
||||
let mut found_save_default = false;
|
||||
let mut found_cancel_normal = false;
|
||||
for y in 0..(HEIGHT + 4) {
|
||||
let row: String = (0..(WIDTH + 4))
|
||||
.map(|x| buf.get(x, y).symbol().to_string())
|
||||
.collect::<Vec<_>>()
|
||||
.join("");
|
||||
if row.contains("[<") && row.contains(">]") && row.contains("Save") {
|
||||
found_save_default = true;
|
||||
}
|
||||
if row.contains("[ ") && row.contains("Cancel") {
|
||||
found_cancel_normal = true;
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
found_save_default,
|
||||
"Save button (Default kind) should render with '[<' / '>]'"
|
||||
);
|
||||
assert!(
|
||||
found_cancel_normal,
|
||||
"Cancel button (Normal kind) should render with '[ ' / ' ]'"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,8 +4,8 @@
|
||||
//! the destination. Mirrors MC's "File exists" dialog with
|
||||
//! Yes / No / Yes-to-all / No-to-all / Abort options.
|
||||
|
||||
use ratatui::layout::{Alignment, Constraint, Direction, Layout, Rect};
|
||||
use ratatui::style::{Modifier, Style};
|
||||
use ratatui::layout::{Constraint, Direction, Layout, Rect};
|
||||
use ratatui::style::Style;
|
||||
use ratatui::text::{Line, Span};
|
||||
use ratatui::widgets::Paragraph;
|
||||
use ratatui::Frame;
|
||||
@@ -13,6 +13,7 @@ use ratatui::Frame;
|
||||
use crate::key::Key;
|
||||
use crate::terminal::color::Theme;
|
||||
use crate::terminal::popup::{centered_cols_rect, render_popup};
|
||||
use crate::widget::button::{render_button_row, ButtonKind, ButtonSpec};
|
||||
|
||||
/// Outcome of the overwrite dialog after a key press.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
@@ -135,50 +136,14 @@ impl OverwriteDialog {
|
||||
)));
|
||||
frame.render_widget(question, chunks[1]);
|
||||
|
||||
let buttons = Paragraph::new(Line::from(vec![
|
||||
Span::styled(
|
||||
"Y",
|
||||
Style::default()
|
||||
.fg(theme.cursor_fg)
|
||||
.bg(theme.executable)
|
||||
.add_modifier(Modifier::BOLD),
|
||||
),
|
||||
Span::styled("es ", Style::default().fg(theme.foreground)),
|
||||
Span::styled(
|
||||
"N",
|
||||
Style::default()
|
||||
.fg(theme.cursor_fg)
|
||||
.bg(theme.error)
|
||||
.add_modifier(Modifier::BOLD),
|
||||
),
|
||||
Span::styled("o ", Style::default().fg(theme.foreground)),
|
||||
Span::styled(
|
||||
"A",
|
||||
Style::default()
|
||||
.fg(theme.cursor_fg)
|
||||
.bg(theme.title_bg)
|
||||
.add_modifier(Modifier::BOLD),
|
||||
),
|
||||
Span::styled("ll ", Style::default().fg(theme.foreground)),
|
||||
Span::styled(
|
||||
"S",
|
||||
Style::default()
|
||||
.fg(theme.cursor_fg)
|
||||
.bg(theme.title_bg)
|
||||
.add_modifier(Modifier::BOLD),
|
||||
),
|
||||
Span::styled("kip all ", Style::default().fg(theme.foreground)),
|
||||
Span::styled(
|
||||
"Esc",
|
||||
Style::default()
|
||||
.fg(theme.cursor_fg)
|
||||
.bg(theme.warning)
|
||||
.add_modifier(Modifier::BOLD),
|
||||
),
|
||||
Span::styled("=abort", Style::default().fg(theme.foreground)),
|
||||
]))
|
||||
.alignment(Alignment::Center);
|
||||
frame.render_widget(buttons, chunks[2]);
|
||||
let specs: Vec<ButtonSpec<'_>> = vec![
|
||||
ButtonSpec { label: "Yes", hotkey: Some('Y'), kind: ButtonKind::Default },
|
||||
ButtonSpec { label: "No", hotkey: Some('N'), kind: ButtonKind::Normal },
|
||||
ButtonSpec { label: "All", hotkey: Some('A'), kind: ButtonKind::Normal },
|
||||
ButtonSpec { label: "Skip all", hotkey: Some('S'), kind: ButtonKind::Normal },
|
||||
ButtonSpec { label: "Abort", hotkey: None, kind: ButtonKind::Normal },
|
||||
];
|
||||
render_button_row(frame, chunks[2], &specs, 0, theme);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -270,4 +235,52 @@ mod tests {
|
||||
assert!(result.ends_with(".txt"));
|
||||
assert!(result.chars().count() <= 15);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_uses_mc_bracket_button_shapes() {
|
||||
// After P2, the Y/N/All/Skip/Abort legend is rendered via the
|
||||
// unified render_button_row, which produces MC bracket shapes
|
||||
// like "[ Yes ]" / "[< No >]". Confirm the Default button
|
||||
// renders with angle brackets and Normal buttons render with
|
||||
// square brackets by assembling a multi-cell run of symbols.
|
||||
use ratatui::backend::TestBackend;
|
||||
use ratatui::Terminal;
|
||||
let backend = TestBackend::new(80, 12);
|
||||
let mut terminal = Terminal::new(backend).unwrap();
|
||||
let theme = *crate::terminal::color::DEFAULT_THEME;
|
||||
terminal
|
||||
.draw(|f| {
|
||||
let d = OverwriteDialog::new_copy("/tmp/foo.txt");
|
||||
d.render(f, f.area(), &theme);
|
||||
})
|
||||
.unwrap();
|
||||
let buf = terminal.backend().buffer().clone();
|
||||
// Concatenate each row's symbols and search for MC bracket
|
||||
// signatures — the Default button uses "[<" / ">]" and Normal
|
||||
// buttons use "[ " / " ]".
|
||||
let mut found_default_angle = false;
|
||||
let mut found_normal_square = false;
|
||||
for y in 0..12 {
|
||||
let row: String = (0..80)
|
||||
.map(|x| buf.get(x, y).symbol().to_string())
|
||||
.collect::<Vec<_>>()
|
||||
.join("");
|
||||
if row.contains("[<") && row.contains(">]") && row.contains("Yes") {
|
||||
found_default_angle = true;
|
||||
}
|
||||
if row.contains("[ ") && row.contains(" ]")
|
||||
&& (row.contains("No ") || row.contains("All") || row.contains("Skip"))
|
||||
{
|
||||
found_normal_square = true;
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
found_default_angle,
|
||||
"Default Yes button should render with '[<' / '>]' brackets"
|
||||
);
|
||||
assert!(
|
||||
found_normal_square,
|
||||
"Normal buttons should render with '[ ' / ' ]' brackets"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,13 +7,14 @@
|
||||
//! widget and the button bar.
|
||||
|
||||
use ratatui::layout::{Alignment, Constraint, Direction, Layout, Rect};
|
||||
use ratatui::style::{Color, Modifier, Style};
|
||||
use ratatui::text::{Line, Span};
|
||||
use ratatui::widgets::{Block, Borders, Clear, Paragraph, Wrap};
|
||||
use ratatui::style::{Color, Style};
|
||||
use ratatui::text::Line;
|
||||
use ratatui::widgets::{Paragraph, Wrap};
|
||||
use ratatui::Frame;
|
||||
|
||||
use crate::terminal::color::Theme;
|
||||
use crate::terminal::mc_skin;
|
||||
use crate::terminal::popup::render_popup;
|
||||
|
||||
|
||||
/// A button in the dialog's button bar.
|
||||
@@ -184,47 +185,33 @@ impl Dialog {
|
||||
.unwrap_or(DialogResult::Cancel)
|
||||
}
|
||||
|
||||
/// Render the dialog into a frame, centered.
|
||||
///
|
||||
/// `theme` supplies the colour palette for the title bar, body,
|
||||
/// borders, and button bar. The instance colour fields
|
||||
/// (`title_fg`, `title_bg`, `body_fg`, `body_bg`) act as historical
|
||||
/// defaults but are overridden by the supplied theme so that
|
||||
/// dialogs follow the active skin.
|
||||
/// Render the dialog into a frame, centered.
|
||||
///
|
||||
/// `theme` supplies the colour palette for the title bar, body,
|
||||
/// borders, and button bar. The instance colour fields
|
||||
/// (`title_fg`, `title_bg`, `body_fg`, `body_bg`) act as historical
|
||||
/// defaults but are overridden by the supplied theme so that
|
||||
/// dialogs follow the active skin.
|
||||
pub fn render(&self, frame: &mut Frame, area: Rect, theme: &Theme) {
|
||||
let popup = centered_rect(area, self.width_pct, self.height_pct);
|
||||
frame.render_widget(Clear, popup);
|
||||
// Delegate shell rendering to render_popup so we inherit the
|
||||
// MC-matching rounded borders + drop shadow + Clear, identical
|
||||
// to every other migrated dialog.
|
||||
let inner = render_popup(frame, popup, &self.title, theme);
|
||||
|
||||
let dialog_title = mc_skin::color_pair(theme.name, "dialog", "dtitle");
|
||||
let title_fg = dialog_title.map(|p| p.fg).unwrap_or(theme.title_fg);
|
||||
let title_bg = dialog_title.map(|p| p.bg).unwrap_or(theme.title_bg);
|
||||
let dialog_default = mc_skin::color_pair(theme.name, "dialog", "_default_");
|
||||
let body_fg = dialog_default.map(|p| p.fg).unwrap_or(theme.foreground);
|
||||
let body_bg = dialog_default.map(|p| p.bg).unwrap_or(theme.background);
|
||||
let block = Block::default()
|
||||
.borders(Borders::ALL)
|
||||
.border_style(Style::default().fg(title_fg).bg(body_bg))
|
||||
.title(Span::styled(
|
||||
format!(" {} ", self.title),
|
||||
Style::default()
|
||||
.fg(title_fg)
|
||||
.bg(title_bg)
|
||||
.add_modifier(Modifier::BOLD),
|
||||
));
|
||||
let inner = block.inner(popup);
|
||||
frame.render_widget(block, popup);
|
||||
|
||||
let chunks = Layout::default()
|
||||
.direction(Direction::Vertical)
|
||||
.constraints([Constraint::Min(3), Constraint::Length(1)])
|
||||
.split(inner);
|
||||
// Body.
|
||||
let body = Paragraph::new(self.body.clone())
|
||||
.style(Style::default().fg(body_fg).bg(body_bg))
|
||||
.wrap(Wrap { trim: false })
|
||||
.alignment(Alignment::Left);
|
||||
frame.render_widget(body, chunks[0]);
|
||||
// Button bar.
|
||||
let specs: Vec<crate::widget::button::ButtonSpec<'_>> = self
|
||||
.buttons
|
||||
.iter()
|
||||
@@ -287,4 +274,77 @@ mod tests {
|
||||
let d = Dialog::confirm_all("t", "b");
|
||||
assert_eq!(d.buttons.len(), 4);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_draws_drop_shadow_at_bottom_right_corner() {
|
||||
// After P1, Dialog::render routes the shell through render_popup,
|
||||
// which paints a MC-matching drop shadow. Verify by rendering into
|
||||
// a 40x10 buffer and checking that the cell just below-and-right of
|
||||
// the popup carries the shadow background.
|
||||
use ratatui::backend::TestBackend;
|
||||
use ratatui::Terminal;
|
||||
let backend = TestBackend::new(40, 10);
|
||||
let mut terminal = Terminal::new(backend).unwrap();
|
||||
let theme = *crate::terminal::color::DEFAULT_THEME;
|
||||
terminal
|
||||
.draw(|f| {
|
||||
let d = Dialog::confirm(" Confirm ", "Are you sure?");
|
||||
d.render(f, f.area(), &theme);
|
||||
})
|
||||
.unwrap();
|
||||
// Popup is centered. With width_pct=0.6 / height_pct=0.4 the popup
|
||||
// spans 24 columns (60% of 40) and 4 rows (40% of 10), centered.
|
||||
// The shadow is a 2-cell right strip + 1-row bottom strip.
|
||||
let popup_w = (40.0 * 0.6) as u16; // 24
|
||||
let popup_h = (10.0 * 0.4) as u16; // 4
|
||||
let popup_x = (40 - popup_w) / 2; // 8
|
||||
let popup_y = (10 - popup_h) / 2; // 3
|
||||
let shadow_x = popup_x + popup_w; // 32 — first column of right strip
|
||||
let shadow_y = popup_y + popup_h; // 7 — first row of bottom strip
|
||||
let buf = terminal.backend().buffer().clone();
|
||||
let shadow_cell = buf.get(shadow_x, shadow_y).symbol();
|
||||
// The shadow cell should be a single space (background-only cell,
|
||||
// not occupied by content). What we really care about is the bg
|
||||
// colour — which we verify via the Style::bg() method below.
|
||||
assert_eq!(shadow_cell, " ");
|
||||
let bg = buf.get(shadow_x, shadow_y).style().bg;
|
||||
assert_eq!(bg, Some(theme.shadow), "shadow cell bg must equal theme.shadow");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_does_not_leak_content_outside_popup() {
|
||||
// After P1, the shell is rendered through render_popup which
|
||||
// clears the popup area before drawing. Pre-P1, Dialog::render
|
||||
// cleared via Clear widget — same behaviour, but verifying that
|
||||
// the corner cell outside the popup is NOT the dialog title.
|
||||
use ratatui::backend::TestBackend;
|
||||
use ratatui::Terminal;
|
||||
let backend = TestBackend::new(40, 10);
|
||||
let mut terminal = Terminal::new(backend).unwrap();
|
||||
let theme = *crate::terminal::color::DEFAULT_THEME;
|
||||
terminal
|
||||
.draw(|f| {
|
||||
let d = Dialog::confirm("XYZ", "");
|
||||
d.render(f, f.area(), &theme);
|
||||
})
|
||||
.unwrap();
|
||||
let buf = terminal.backend().buffer().clone();
|
||||
// Scan every cell inside the popup rect — at least one must
|
||||
// contain the title character 'X'. This confirms the shell
|
||||
// was rendered (Clear + Block + title text all fired).
|
||||
let popup_w = (40.0 * 0.6) as u16;
|
||||
let popup_h = (10.0 * 0.4) as u16;
|
||||
let popup_x = (40 - popup_w) / 2;
|
||||
let popup_y = (10 - popup_h) / 2;
|
||||
let mut found_x = false;
|
||||
for y in popup_y..popup_y + popup_h {
|
||||
for x in popup_x..popup_x + popup_w {
|
||||
if buf.get(x, y).symbol().contains('X') {
|
||||
found_x = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
assert!(found_x, "title character 'X' should appear inside popup");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user