mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Enforce animations = false for screen readers (#20564)
## Why Issue #20489 calls out that animated TUI affordances can be noisy for screen-reader users. Codex already has `tui.animations = false` as a reduced-motion setting, but some live activity rows render spinner-style prefixes in that mode. These were relatively recent regressions. We have also regressed this pattern more than once by adding new spinner/shimmer callsites that do not think through the reduced-motion path, so this PR adds a small guardrail while fixing the current surfaces. ## What changed - Omit the live status-row spinner when animations are disabled, so the row starts with stable text like `Working (...)`. - Render running hook headers without the spinner prefix when animations are disabled, while preserving shimmer/spinner behavior when animations are enabled. - Centralize TUI activity indicators in `tui/src/motion.rs`, with explicit reduced-motion choices for hidden prefixes, static bullets, and plain shimmer-text fallbacks. - Route existing spinner/shimmer callsites through the central motion helper, including exec rows, MCP/web-search/loading rows, hook rows, plugin loading, and onboarding loading text. - Add a source-scan regression test that rejects direct `spinner(...)` or `shimmer_spans(...)` usage outside the central module and primitive definition. - Add focused coverage that reduced-motion active exec rows are stable, status rows start without a spinner, running hooks omit the spinner, and MCP inventory loading stays stable. - Update the one affected status-indicator snapshot; the existing detail tree prefix remains unchanged. ## Verification - `cargo test -p codex-tui`
This commit is contained in:
@@ -9,4 +9,3 @@ pub(crate) use render::OutputLinesParams;
|
||||
pub(crate) use render::TOOL_CALL_MAX_LINES;
|
||||
pub(crate) use render::new_active_exec_command;
|
||||
pub(crate) use render::output_lines;
|
||||
pub(crate) use render::spinner;
|
||||
|
||||
@@ -5,10 +5,12 @@ use super::model::ExecCall;
|
||||
use super::model::ExecCell;
|
||||
use crate::exec_command::strip_bash_lc_and_escape;
|
||||
use crate::history_cell::HistoryCell;
|
||||
use crate::motion::MotionMode;
|
||||
use crate::motion::ReducedMotionIndicator;
|
||||
use crate::motion::activity_indicator;
|
||||
use crate::render::highlight::highlight_bash_to_lines;
|
||||
use crate::render::line_utils::prefix_lines;
|
||||
use crate::render::line_utils::push_owned_lines;
|
||||
use crate::shimmer::shimmer_spans;
|
||||
use crate::wrapping::RtOptions;
|
||||
use crate::wrapping::adaptive_wrap_line;
|
||||
use crate::wrapping::adaptive_wrap_lines;
|
||||
@@ -180,20 +182,13 @@ pub(crate) fn output_lines(
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn spinner(start_time: Option<Instant>, animations_enabled: bool) -> Span<'static> {
|
||||
if !animations_enabled {
|
||||
return "•".dim();
|
||||
}
|
||||
let elapsed = start_time.map(|st| st.elapsed()).unwrap_or_default();
|
||||
if supports_color::on_cached(supports_color::Stream::Stdout)
|
||||
.map(|level| level.has_16m)
|
||||
.unwrap_or(false)
|
||||
{
|
||||
shimmer_spans("•")[0].clone()
|
||||
} else {
|
||||
let blink_on = (elapsed.as_millis() / 600).is_multiple_of(2);
|
||||
if blink_on { "•".into() } else { "◦".dim() }
|
||||
}
|
||||
fn activity_marker(start_time: Option<Instant>, animations_enabled: bool) -> Span<'static> {
|
||||
activity_indicator(
|
||||
start_time,
|
||||
MotionMode::from_animations_enabled(animations_enabled),
|
||||
ReducedMotionIndicator::StaticBullet,
|
||||
)
|
||||
.unwrap_or_else(|| "•".dim())
|
||||
}
|
||||
|
||||
impl HistoryCell for ExecCell {
|
||||
@@ -263,7 +258,7 @@ impl ExecCell {
|
||||
let mut out: Vec<Line<'static>> = Vec::new();
|
||||
out.push(Line::from(vec![
|
||||
if self.is_active() {
|
||||
spinner(self.active_start_time(), self.animations_enabled())
|
||||
activity_marker(self.active_start_time(), self.animations_enabled())
|
||||
} else {
|
||||
"•".dim()
|
||||
},
|
||||
@@ -371,7 +366,7 @@ impl ExecCell {
|
||||
let bullet = match success {
|
||||
Some(true) => "•".green().bold(),
|
||||
Some(false) => "•".red().bold(),
|
||||
None => spinner(call.start_time, self.animations_enabled()),
|
||||
None => activity_marker(call.start_time, self.animations_enabled()),
|
||||
};
|
||||
let is_interaction = call.is_unified_exec_interaction();
|
||||
let title = if is_interaction {
|
||||
@@ -957,6 +952,35 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn active_command_without_animations_is_stable() {
|
||||
let call = ExecCall {
|
||||
call_id: "call-id".to_string(),
|
||||
command: vec!["bash".into(), "-lc".into(), "echo done".into()],
|
||||
parsed: Vec::new(),
|
||||
output: None,
|
||||
source: ExecCommandSource::Agent,
|
||||
start_time: Some(Instant::now()),
|
||||
duration: None,
|
||||
interaction_input: None,
|
||||
};
|
||||
|
||||
let cell = ExecCell::new(call, /*animations_enabled*/ false);
|
||||
let first: Vec<String> = cell
|
||||
.command_display_lines(/*width*/ 80)
|
||||
.iter()
|
||||
.map(render_line_text)
|
||||
.collect();
|
||||
let second: Vec<String> = cell
|
||||
.command_display_lines(/*width*/ 80)
|
||||
.iter()
|
||||
.map(render_line_text)
|
||||
.collect();
|
||||
|
||||
assert_eq!(first, second);
|
||||
assert_eq!(first, vec!["• Running echo done".to_string()]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exploring_display_does_not_split_long_url_like_search_query() {
|
||||
let url_like = "example.test/api/v1/projects/alpha-team/releases/2026-02-17/builds/1234567890/artifacts/reports/performance/summary/detail/with/a/very/long/path";
|
||||
|
||||
Reference in New Issue
Block a user