mirror of
https://github.com/openai/codex.git
synced 2026-04-24 22:54:54 +00:00
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)
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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<Option<Rect>>,
|
||||
}
|
||||
|
||||
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<Line> = 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<u16> {
|
||||
(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<u16> = 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();
|
||||
|
||||
Reference in New Issue
Block a user