From 182000999c882a4424475981f1790e2311b9777c Mon Sep 17 00:00:00 2001 From: Matthew Zeng Date: Sat, 24 Jan 2026 21:50:35 -0800 Subject: [PATCH] Raise welcome animation breakpoint to 37 rows (#9778) ### Motivation - The large ASCII welcome animation can push onboarding content below the fold on default-height terminals, making the CLI appear unresponsive; raising the breakpoint prevents that. - The existing test measured an arbitrary row count rather than asserting the welcome line position relative to the animation frame, which made the intent unclear. ### Description - Increase `MIN_ANIMATION_HEIGHT` from `20` to `37` in `codex-rs/tui/src/onboarding/welcome.rs` so the animation is skipped unless there is enough vertical space. - Replace the brittle measurement logic in the welcome render test with a `row_containing` helper and assert the welcome row equals the frame height plus the spacer line (`frame_lines + 1`). - Add a regression test `welcome_skips_animation_below_height_breakpoint` that verifies the animation is not rendered when the viewport height is one row below the breakpoint. ### Testing - Ran formatting with `~/.cargo/bin/just fmt` which completed successfully. - Ran unit tests for the crate with `cargo test -p codex-tui --lib` and they passed (unit test suite succeeded). - Ran `cargo test -p codex-tui` which reported a failing integration test in this environment because the test cannot locate the `codex` binary, so full crate tests are blocked here (environment limitation). ------ [Codex Task](https://chatgpt.com/codex/tasks/task_i_6973b0a710d4832c9ff36fac26eb1519) --- .../tui/src/onboarding/onboarding_screen.rs | 3 + codex-rs/tui/src/onboarding/welcome.rs | 59 ++++++++++++------- 2 files changed, 41 insertions(+), 21 deletions(-) diff --git a/codex-rs/tui/src/onboarding/onboarding_screen.rs b/codex-rs/tui/src/onboarding/onboarding_screen.rs index 7e076ae16..78a7f0ce8 100644 --- a/codex-rs/tui/src/onboarding/onboarding_screen.rs +++ b/codex-rs/tui/src/onboarding/onboarding_screen.rs @@ -311,6 +311,9 @@ impl WidgetRef for &OnboardingScreen { } let scratch_area = Rect::new(0, 0, width, max_h); let mut scratch = Buffer::empty(scratch_area); + if let Step::Welcome(widget) = step { + widget.update_layout_area(scratch_area); + } step.render_ref(scratch_area, &mut scratch); let h = used_rows(&scratch, width, max_h).min(max_h); if h > 0 { diff --git a/codex-rs/tui/src/onboarding/welcome.rs b/codex-rs/tui/src/onboarding/welcome.rs index 8ff5e8198..bdf4f9140 100644 --- a/codex-rs/tui/src/onboarding/welcome.rs +++ b/codex-rs/tui/src/onboarding/welcome.rs @@ -11,6 +11,7 @@ use ratatui::widgets::Clear; use ratatui::widgets::Paragraph; use ratatui::widgets::WidgetRef; use ratatui::widgets::Wrap; +use std::cell::Cell; use crate::ascii_animation::AsciiAnimation; use crate::onboarding::onboarding_screen::KeyboardHandler; @@ -19,13 +20,14 @@ use crate::tui::FrameRequester; use super::onboarding_screen::StepState; -const MIN_ANIMATION_HEIGHT: u16 = 20; +const MIN_ANIMATION_HEIGHT: u16 = 37; const MIN_ANIMATION_WIDTH: u16 = 60; pub(crate) struct WelcomeWidget { pub is_logged_in: bool, animation: AsciiAnimation, animations_enabled: bool, + layout_area: Cell>, } impl KeyboardHandler for WelcomeWidget { @@ -53,8 +55,13 @@ impl WelcomeWidget { is_logged_in, animation: AsciiAnimation::new(request_frame), animations_enabled, + layout_area: Cell::new(None), } } + + pub(crate) fn update_layout_area(&self, area: Rect) { + self.layout_area.set(Some(area)); + } } impl WidgetRef for &WelcomeWidget { @@ -64,12 +71,14 @@ impl WidgetRef for &WelcomeWidget { self.animation.schedule_next_frame(); } + let layout_area = self.layout_area.get().unwrap_or(area); // Skip the animation entirely when the viewport is too small so we don't clip frames. - let show_animation = - area.height >= MIN_ANIMATION_HEIGHT && area.width >= MIN_ANIMATION_WIDTH; + let show_animation = self.animations_enabled + && layout_area.height >= MIN_ANIMATION_HEIGHT + && layout_area.width >= MIN_ANIMATION_WIDTH; let mut lines: Vec = Vec::new(); - if show_animation && self.animations_enabled { + if show_animation { let frame = self.animation.current_frame(); lines.extend(frame.lines().map(Into::into)); lines.push("".into()); @@ -99,6 +108,7 @@ impl StepStateProvider for WelcomeWidget { #[cfg(test)] mod tests { use super::*; + use pretty_assertions::assert_eq; use ratatui::buffer::Buffer; use ratatui::layout::Rect; @@ -106,31 +116,37 @@ mod tests { static VARIANT_B: [&str; 1] = ["frame-b"]; static VARIANTS: [&[&str]; 2] = [&VARIANT_A, &VARIANT_B]; + fn row_containing(buf: &Buffer, needle: &str) -> Option { + (0..buf.area.height).find(|&y| { + let mut row = String::new(); + for x in 0..buf.area.width { + row.push_str(buf[(x, y)].symbol()); + } + row.contains(needle) + }) + } + #[test] fn welcome_renders_animation_on_first_draw() { let widget = WelcomeWidget::new(false, FrameRequester::test_dummy(), true); let area = Rect::new(0, 0, MIN_ANIMATION_WIDTH, MIN_ANIMATION_HEIGHT); let mut buf = Buffer::empty(area); + let frame_lines = widget.animation.current_frame().lines().count() as u16; (&widget).render(area, &mut buf); - let mut found = false; - let mut last_non_empty: Option = None; - for y in 0..area.height { - for x in 0..area.width { - if !buf[(x, y)].symbol().trim().is_empty() { - found = true; - last_non_empty = Some(y); - break; - } - } - } + let welcome_row = row_containing(&buf, "Welcome"); + assert_eq!(welcome_row, Some(frame_lines + 1)); + } - assert!(found, "expected welcome animation to render characters"); - let measured_rows = last_non_empty.map(|v| v + 2).unwrap_or(0); - assert!( - measured_rows >= MIN_ANIMATION_HEIGHT, - "expected measurement to report at least {MIN_ANIMATION_HEIGHT} rows, got {measured_rows}" - ); + #[test] + fn welcome_skips_animation_below_height_breakpoint() { + let widget = WelcomeWidget::new(false, FrameRequester::test_dummy(), true); + let area = Rect::new(0, 0, MIN_ANIMATION_WIDTH, MIN_ANIMATION_HEIGHT - 1); + let mut buf = Buffer::empty(area); + (&widget).render(area, &mut buf); + + let welcome_row = row_containing(&buf, "Welcome"); + assert_eq!(welcome_row, Some(0)); } #[test] @@ -139,6 +155,7 @@ mod tests { is_logged_in: false, animation: AsciiAnimation::with_variants(FrameRequester::test_dummy(), &VARIANTS, 0), animations_enabled: true, + layout_area: Cell::new(None), }; let before = widget.animation.current_frame();