From 5ea69dbc084ed3b4f0c308b1a6313eec8a05a494 Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Fri, 29 May 2026 22:07:32 +0000 Subject: [PATCH] fix: widen undo dialog and polish its summary line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The undo/redo confirmation dialog capped at 60 columns and wrapped even a short insert on wide terminals, showed a lowercase "snapshot taken", a raw ISO-8601 timestamp, and lowercase yes/no labels. Grow the dialog to fit its longest line (bounded 34–100), capitalise Snapshot/Yes/No, and render the snapshot timestamp in local time, human-formatted (24 May 2026, 11:00) via a new chrono dependency (clock feature only; English month names). Yes/No capitalisation also applies to the rebuild-confirm dialog. --- Cargo.lock | 51 +++++ Cargo.toml | 4 + docs/requirements.md | 6 +- src/friendly/strings/en-US.yaml | 6 +- ...ui__tests__rebuild_confirm_modal_dark.snap | 3 +- src/ui.rs | 184 ++++++++++++++++-- 6 files changed, 235 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index aa6fba7..9dd46bf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -23,6 +23,15 @@ version = "0.2.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "683d7910e743518b0e34f1186f92494becacb047c7b6bf616c96772180fef923" +[[package]] +name = "android_system_properties" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "819e7219dbd41043ac279b19830f2efc897156490d7fd6ea916720117ee66311" +dependencies = [ + "libc", +] + [[package]] name = "anyhow" version = "1.0.102" @@ -155,6 +164,17 @@ dependencies = [ "rand_core 0.10.1", ] +[[package]] +name = "chrono" +version = "0.4.44" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c673075a2e0e5f4a1dde27ce9dee1ea4558c7ffe648f576438a20ca1d2acc4b0" +dependencies = [ + "iana-time-zone", + "num-traits", + "windows-link", +] + [[package]] name = "compact_str" version = "0.9.0" @@ -189,6 +209,12 @@ dependencies = [ "unicode-segmentation", ] +[[package]] +name = "core-foundation-sys" +version = "0.8.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" + [[package]] name = "cpufeatures" version = "0.2.17" @@ -675,6 +701,30 @@ version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" +[[package]] +name = "iana-time-zone" +version = "0.1.65" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e31bc9ad994ba00e440a8aa5c9ef0ec67d5cb5e5cb0cc7f8b744a35b389cc470" +dependencies = [ + "android_system_properties", + "core-foundation-sys", + "iana-time-zone-haiku", + "js-sys", + "log", + "wasm-bindgen", + "windows-core", +] + +[[package]] +name = "iana-time-zone-haiku" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f31827a206f56af32e590ba56d5d2d085f558508192593743f16b2306495269f" +dependencies = [ + "cc", +] + [[package]] name = "id-arena" version = "2.3.0" @@ -1370,6 +1420,7 @@ version = "0.1.0" dependencies = [ "anyhow", "base64", + "chrono", "crossterm", "csv", "directories", diff --git a/Cargo.toml b/Cargo.toml index 6af328a..76d88e3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,6 +11,10 @@ publish = false [dependencies] anyhow = "1.0.102" base64 = "0.22.1" +# `clock` brings local-timezone support (UTC → machine local) for +# the undo-dialog snapshot timestamp (issue #13). No locale feature: +# month names stay English; `unstable-locales` is deliberately avoided. +chrono = { version = "0.4.44", default-features = false, features = ["clock"] } crossterm = { version = "0.29.0", features = ["event-stream"] } csv = "1.4.0" directories = "6.0.0" diff --git a/docs/requirements.md b/docs/requirements.md index 57661ee..4d60d77 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -453,7 +453,11 @@ handoff-14 cleanup; 449 after B2/C2.) undone / re-applied (`Y` confirms). *(Implemented 2026-05-24: `undo` / `redo` app commands, `Modal::UndoConfirm`, runtime prepare→confirm→restore→refresh; `--no-undo` reports undo is off, - empty stacks report "nothing to undo/redo".)* + empty stacks report "nothing to undo/redo". UX polish 2026-05-29, + issue #13: the confirm dialog grows to fit its summary on one row, + capitalises `Snapshot` / `Yes` / `No`, and renders the snapshot + timestamp in local time, human-formatted (`24 May 2026, 11:00`) via + the new `chrono` dependency.)* - [x] **U3** `history.log` records every submitted command in append-only form, tagged with its outcome (Iteration 2; broadened by ADR-0034). Format: `||` diff --git a/src/friendly/strings/en-US.yaml b/src/friendly/strings/en-US.yaml index ee0b254..36ffa64 100644 --- a/src/friendly/strings/en-US.yaml +++ b/src/friendly/strings/en-US.yaml @@ -763,7 +763,7 @@ modal: redo_confirm_title: "Redo last undone change" undo_confirm_command: "This will undo:" redo_confirm_command: "This will re-apply:" - undo_confirm_when: "snapshot taken {timestamp}" + undo_confirm_when: "Snapshot taken {timestamp}" undo_confirm_prompt: "Restore that earlier state?" undo_cancelled: "undo cancelled" redo_cancelled: "redo cancelled" @@ -807,8 +807,8 @@ shortcut: submit: "submit" confirm: "confirm" cancel: "cancel" - yes: "yes" - no: "no" + yes: "Yes" + no: "No" load: "load" select: "select" browse_path: "browse path" diff --git a/src/snapshots/rdbms_playground__ui__tests__rebuild_confirm_modal_dark.snap b/src/snapshots/rdbms_playground__ui__tests__rebuild_confirm_modal_dark.snap index 082c09e..f34c9e6 100644 --- a/src/snapshots/rdbms_playground__ui__tests__rebuild_confirm_modal_dark.snap +++ b/src/snapshots/rdbms_playground__ui__tests__rebuild_confirm_modal_dark.snap @@ -1,5 +1,6 @@ --- source: src/ui.rs +assertion_line: 1469 expression: snapshot --- ╭ Tables ──────────────────╮╭ Output ──────────────────────────────────────────╮ @@ -16,7 +17,7 @@ expression: snapshot │ │ │ │ │ │Continue? │ │ │ │ │ │ -│ │[Y] yes [N] no Esc cancel │ │ +│ │[Y] Yes [N] No Esc cancel │ │ │ ╰──────────────────────────────────────────────────────────╯─────────╯ │ │╭ SIMPLE ──────────────────────────────────────────╮ │ ││ │ diff --git a/src/ui.rs b/src/ui.rs index aa429e6..5b804a5 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -317,6 +317,52 @@ fn render_rebuild_confirm(summary: &str, theme: &Theme, frame: &mut Frame<'_>, a frame.render_widget(paragraph, dialog_area); } +/// Format a stored ISO-8601 UTC timestamp for display in a +/// confirmation dialog (issue #13): parse it, convert to the +/// machine's local timezone, and render a fixed human-friendly +/// form (`24 May 2026, 11:00`). Month names stay English — no +/// locale feature. Falls back to the raw input if it can't be +/// parsed; this is defensive only, since stored values are always +/// `utc_iso8601_now()` output. +fn format_snapshot_timestamp(iso: &str) -> String { + chrono::DateTime::parse_from_rfc3339(iso) + .map(|dt| format_local_datetime(dt.with_timezone(&chrono::Local))) + .unwrap_or_else(|_| iso.to_string()) +} + +/// Render a timezone-aware datetime in the fixed display form. +/// Split out from [`format_snapshot_timestamp`] so the format can +/// be unit-tested deterministically with a fixed offset (the +/// `Local` conversion itself is machine-dependent). +fn format_local_datetime(dt: chrono::DateTime) -> String +where + Tz: chrono::TimeZone, + Tz::Offset: std::fmt::Display, +{ + dt.format("%-d %b %Y, %H:%M").to_string() +} + +/// Preferred outer width (columns) for the undo/redo confirm +/// dialog (issue #13): wide enough to hold the longest content +/// line on a single row, clamped to sane bounds and the available +/// area so a short insert no longer wraps on roomy terminals. +fn undo_dialog_width( + content_widths: impl IntoIterator, + area_width: u16, +) -> u16 { + /// Floor — comfortably fits the button row plus borders. + const MIN: u16 = 34; + /// Ceiling for outlier (ultra-wide) terminals. + const MAX: u16 = 100; + let widest = content_widths.into_iter().max().unwrap_or(0); + // +4: left/right border (2) + one padding column each side (2). + let preferred = + u16::try_from(widest).unwrap_or(u16::MAX).saturating_add(4); + let upper = area_width.min(MAX); + let lower = MIN.min(upper); + preferred.clamp(lower, upper) +} + /// `undo` / `redo` confirmation modal (ADR-0006 Amendment 1). Names /// the command that will be undone / re-applied and when its /// snapshot was taken, then prompts `Y` / `N`. @@ -326,19 +372,48 @@ fn render_undo_confirm( frame: &mut Frame<'_>, area: Rect, ) { - let dialog_w = area.width.clamp(20, 60); - let inner_w = dialog_w.saturating_sub(4) as usize; - let intro = if m.is_redo { crate::t!("modal.redo_confirm_command") } else { crate::t!("modal.undo_confirm_command") }; - let mut body_lines: Vec = wrap_lines(&format!("{intro} {}", m.command), inner_w); - body_lines.extend(wrap_lines( - &crate::t!("modal.undo_confirm_when", timestamp = m.timestamp), - inner_w, - )); + let title = if m.is_redo { + crate::t!("modal.redo_confirm_title") + } else { + crate::t!("modal.undo_confirm_title") + }; + let intro_line = format!("{intro} {}", m.command); + // Local-time, human-formatted snapshot stamp (issue #13). + let when_display = format_snapshot_timestamp(&m.timestamp); + let when_line = + crate::t!("modal.undo_confirm_when", timestamp = when_display); + let prompt = crate::t!("modal.undo_confirm_prompt"); + // Reconstruct the button row purely to measure its width — the + // styled spans are built below. Keep this in sync with them. + let buttons_measure = format!( + "[Y] {} [N] {} Esc {}", + crate::t!("shortcut.yes"), + crate::t!("shortcut.no"), + crate::t!("shortcut.cancel"), + ); + + // Grow the dialog to fit the longest content line on one row + // (issue #13). The title sits in the border, so it needs two + // extra columns for the surrounding spaces. + let dialog_w = undo_dialog_width( + [ + title.chars().count() + 2, + intro_line.chars().count(), + when_line.chars().count(), + prompt.chars().count(), + buttons_measure.chars().count(), + ], + area.width, + ); + let inner_w = dialog_w.saturating_sub(4) as usize; + + let mut body_lines: Vec = wrap_lines(&intro_line, inner_w); + body_lines.extend(wrap_lines(&when_line, inner_w)); let body_height = body_lines.len() as u16; // Title row + blank + body + blank + prompt + blank + keys + borders (2). let dialog_h = body_height.saturating_add(7).min(area.height); @@ -354,11 +429,6 @@ fn render_undo_confirm( frame.render_widget(ratatui::widgets::Clear, dialog_area); - let title = if m.is_redo { - crate::t!("modal.redo_confirm_title") - } else { - crate::t!("modal.undo_confirm_title") - }; let title_style = Style::default().fg(theme.fg).add_modifier(Modifier::BOLD); let block = Block::default() .borders(Borders::ALL) @@ -376,7 +446,7 @@ fn render_undo_confirm( text_lines.push(Line::from(line)); } text_lines.push(Line::from("")); - text_lines.push(Line::from(crate::t!("modal.undo_confirm_prompt"))); + text_lines.push(Line::from(prompt)); text_lines.push(Line::from("")); text_lines.push(Line::from(vec![ Span::styled("[Y]", Style::default().fg(theme.fg).add_modifier(Modifier::BOLD)), @@ -1399,6 +1469,92 @@ mod tests { insta::assert_snapshot!("rebuild_confirm_modal_dark", snapshot); } + // ---- Issue #13: undo confirm dialog ------------------------- + + #[test] + fn format_local_datetime_renders_fixed_human_form() { + // Deterministic: a fixed offset (not Local) so the output + // does not depend on the test machine's timezone. + let dt = chrono::DateTime::parse_from_rfc3339("2026-05-24T11:05:00+02:00") + .expect("valid rfc3339"); + assert_eq!(format_local_datetime(dt), "24 May 2026, 11:05"); + // Single-digit day: no leading zero on the day, but zero- + // padded hour. + let dt = chrono::DateTime::parse_from_rfc3339("2026-05-04T09:05:00+00:00") + .expect("valid rfc3339"); + assert_eq!(format_local_datetime(dt), "4 May 2026, 09:05"); + } + + #[test] + fn format_snapshot_timestamp_drops_machine_syntax() { + // The stored UTC string is reformatted: no 'T'/'Z' machine + // syntax survives, and the year is preserved. (Day/month + // can shift across the date line depending on local TZ, so + // we assert only the stable parts.) + let out = format_snapshot_timestamp("2026-07-24T10:00:00Z"); + assert!(!out.contains('T'), "no date/time 'T' separator: {out}"); + assert!(!out.contains('Z'), "no UTC 'Z' suffix: {out}"); + assert!(out.contains("2026"), "year preserved: {out}"); + } + + #[test] + fn format_snapshot_timestamp_falls_back_on_garbage() { + assert_eq!(format_snapshot_timestamp("not a timestamp"), "not a timestamp"); + } + + #[test] + fn undo_dialog_width_grows_to_fit_and_clamps() { + // Grows to the widest line + 4 (borders + padding). + assert_eq!(undo_dialog_width([50usize], 120), 54); + // Floors at MIN (34) for tiny content. + assert_eq!(undo_dialog_width([3usize], 120), 34); + // Caps at MAX (100) for absurdly long content. + assert_eq!(undo_dialog_width([400usize], 120), 100); + // Never exceeds the available area, and never panics when + // the area is narrower than MIN. + assert_eq!(undo_dialog_width([50usize], 40), 40); + assert_eq!(undo_dialog_width([50usize], 10), 10); + } + + #[test] + fn undo_modal_command_does_not_wrap_on_wide_terminal() { + use crate::app::{Modal, UndoConfirmModal}; + let mut app = App::new(); + app.modal = Some(Modal::UndoConfirm(UndoConfirmModal { + command: "insert into Customers values (1, 'Oliver Sturm')".to_string(), + timestamp: "2026-05-24T10:00:00Z".to_string(), + is_redo: false, + })); + let theme = Theme::dark(); + let out = render_to_string(&mut app, &theme, 120, 30); + assert!( + out.lines().any(|l| l.contains( + "This will undo: insert into Customers values (1, 'Oliver Sturm')" + )), + "command must sit on one row on a wide terminal:\n{out}" + ); + } + + #[test] + fn undo_modal_uses_capitalized_labels_and_formatted_time() { + use crate::app::{Modal, UndoConfirmModal}; + let mut app = App::new(); + app.modal = Some(Modal::UndoConfirm(UndoConfirmModal { + command: "delete from T where id = 1".to_string(), + timestamp: "2026-05-24T10:00:00Z".to_string(), + is_redo: false, + })); + let theme = Theme::dark(); + let out = render_to_string(&mut app, &theme, 120, 30); + assert!(out.contains("Snapshot taken"), "capitalized Snapshot:\n{out}"); + assert!(out.contains("[Y] Yes"), "capitalized Yes:\n{out}"); + assert!(out.contains("[N] No"), "capitalized No:\n{out}"); + assert!( + !out.contains("2026-05-24T10:00:00Z"), + "raw ISO timestamp must not appear:\n{out}" + ); + } + #[test] fn populated_with_table_snapshot() { // Items panel lists tables; output panel shows the