Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 119 additions & 0 deletions docs/tui-ux-accessibility-review.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
# TUI UI/UX, Responsiveness & Accessibility Review

A comprehensive review of the coven-code terminal UI (`src-rust/crates/tui/`) across
three dimensions — **responsive rendering (all terminal sizes)**, **accessibility**,
and **UI/UX consistency / seamless integration** — with the fixes applied in this
pass and prioritized follow-ups.

"Screen size" for a TUI means terminal **columns × rows**: tiny (e.g. 20×6), narrow
(~60 cols), short (few rows), and very wide/tall.

---

## Summary

The codebase already has solid accessibility *infrastructure* — a `theme_colors.rs`
with a deuteranopia (colorblind) palette and WCAG-contrast unit tests, effort levels
encoded by **shape** not just color (`figures.rs`), and a small-terminal fallback for
the welcome box. The gaps were: a **live mojibake rendering bug**, selection/focus
conveyed by background color alone, one genuinely color-only status, a handful of
unguarded layout subtractions that can underflow on tiny terminals, and a duplicated
accent color literal that undercuts theming.

---

## Fixed in this pass

### Accessibility

1. **Mojibake in rendered strings (High — live bug on every terminal).**
`render.rs` contained double-encoded UTF-8 (UTF-8 bytes misread as CP1252 then
re-encoded). The statusline segment separator rendered as literal `â"‚` instead of
`│`, the context warning as `âš ` instead of `⚠`, and the bridge glyph as `ðŸ"—`
instead of `🔗`. Fixed all 12 occurrences (5 rendered + 7 comments) via a safe
CP1252 round-trip that only touches CP1252-encodable runs, leaving legitimate
multibyte glyphs (`✓ 🌿 ─ │ ⚠`) untouched.
Comment on lines +31 to +35

2. **Focus/selection survives NO_COLOR & monochrome (High).** Selected list rows in
`dialog_select.rs` and `model_picker.rs` were distinguished **only** by a highlight
background. Added a `> ` caret marker + `Modifier::BOLD` so the focused item is
still identifiable when the background can't render (NO_COLOR, monochrome terminal,
low-contrast theme, colorblind users). `model_picker` reuses its existing 3-char
leading column, so there's no layout shift. (`ask_user_dialog.rs` already did this
correctly with a `▶ ` prefix + bold.)

3. **Context-window level is no longer color-only (High).** The statusline context
gauge showed `Nk/Mk (P%)` colored green/yellow/red — the *number* was visible but
the *danger level* was carried by color alone. Added a shape prefix (`○` ok / `◐`
warning / `●` critical, matching the effort-level shape convention) so colorblind
and monochrome users can read the level.

### Responsiveness (all screen sizes)

4. **Underflow-safe layout math (Medium).** Six unguarded `rect.height - N` /
`rect.width - N` subtractions could underflow and panic on tiny terminals. Converted
to `saturating_sub` in `ask_user_dialog.rs` (×2), `prompt_input.rs`, `stats_dialog.rs`,
and `tasks_overlay.rs`. (`diff_viewer.rs:859` was already guarded by `inner.width > 1`
and left as-is; `stats_dialog.rs:614` is bounded by its loop and left as-is.)

### UI/UX consistency

5. **Accent color consolidated (Medium).** The violet accent `Color::Rgb(139,92,246)`
was copy-pasted **21 times across 11 files**. A single source of truth already
existed — `overlays::COVEN_CODE_ACCENT` — so 18 inline literals were replaced with
it (leaving the `overlays.rs` definition and the `theme_colors.rs` palette values).
This is the foundation for making the accent theme-aware.

---

## Verified NOT an issue (checked, no change needed)

- **PR badge & agent-progress status** were flagged as possibly color-only, but both
render the status as **text** (`[approved]`, `working`/`done`/`error`) — color only
reinforces. No change.
- **Welcome box** already collapses to a single status line below a minimum size
(`render.rs` `render_welcome_box`), and the familiar switcher caps its popup to the
available area with `.min(...).max(4)`.

---

## Prioritized follow-ups (not done here — larger/riskier)

These are documented for a future pass; each is a broader change than this review's
surgical scope.

1. **Honor `NO_COLOR` (High).** The env var is never read anywhere in the workspace.
A startup check that forces a monochrome style path (drop `.fg()/.bg()`, rely on
`BOLD`/`REVERSED`) would make the app usable on strictly monochrome setups. The
focus-marker work above is a prerequisite that's now in place.

2. **Wire the theme system into the main render path (High).** `theme_colors.rs`
`ColorPalette` / `get_error_color` / `get_success_color` have **no consumers**
outside the file; only the diff viewer is theme-aware (`diff_viewer.rs:40`). The
~53 hardcoded `Rgb` and ~60 `DarkGray` in `render.rs` bypass the selected theme, so
the deuteranopia theme currently has no effect on the main UI. Route them through
`ColorPalette::for_theme(&theme_name)`, replicating the diff-viewer pattern.

3. **Raise the worst contrast offenders above ~4.5:1 (High).** Near-invisible dim text:
`onboarding_dialog.rs` separators (`Rgb(45,45,55)` ≈ 1.45:1), `model_picker.rs:758`
(`Rgb(40,40,45)` ≈ 1.35:1). Move toward the AA-tested `COVEN_CODE_MUTED`
(`Rgb(161,161,170)` ≈ 7.7:1). (Verify the stats-chart empty-block dimness is
intentional before changing it.)

4. **Replace pervasive `Color::DarkGray` with `COVEN_CODE_MUTED` (Medium).** `DarkGray`
maps to ANSI bright-black, whose RGB is terminal-theme-dependent and often fails
contrast. A defined muted constant is both consistent and covered by the existing
WCAG tests.

5. **Unicode capability gate + ASCII fallbacks (Medium).** `figures.rs` has rich
glyphs with only a Windows/Unix fallback for one symbol and no `LANG`/`LC_*`
detection. Provide ASCII alternates for glyphs and box-drawing separators for
ASCII-only / `LANG=C` terminals and log captures.

---

## Test coverage

- `cargo test` across touched crates (see PR).
- `scripts/tui-tests/` interactive tmux suite run at multiple terminal sizes
(small / narrow / default) to confirm no rendering regressions.
6 changes: 3 additions & 3 deletions src-rust/crates/tui/src/agents_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1236,7 +1236,7 @@ fn render_agents_list(state: &AgentsMenuState, area: Rect, buf: &mut Buffer) {
lines.push(Line::from(vec![Span::styled(
" Coven Familiars",
Style::default()
.fg(Color::Rgb(139, 92, 246)) // violet-500
.fg(crate::overlays::COVEN_CODE_ACCENT) // violet-500
.add_modifier(Modifier::BOLD),
)]));
}
Expand Down Expand Up @@ -1326,7 +1326,7 @@ fn render_agent_detail(def: &AgentDefinition, area: Rect, buf: &mut Buffer) {
// Source badge — colour-coded for familiar vs user.
let source_style = if is_familiar {
Style::default()
.fg(Color::Rgb(139, 92, 246))
.fg(crate::overlays::COVEN_CODE_ACCENT)
.add_modifier(Modifier::BOLD)
} else {
Style::default().fg(COVEN_CODE_MUTED)
Expand Down Expand Up @@ -1428,7 +1428,7 @@ fn render_agent_detail(def: &AgentDefinition, area: Rect, buf: &mut Buffer) {
lines.push(Line::default());
lines.push(Line::from(vec![Span::styled(
" ✨ Coven Familiar — read-only. Create a workspace override to customise this familiar.",
Style::default().fg(Color::Rgb(139, 92, 246)),
Style::default().fg(crate::overlays::COVEN_CODE_ACCENT),
)]));
}

Expand Down
26 changes: 9 additions & 17 deletions src-rust/crates/tui/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1288,7 +1288,7 @@ Move immutable borrow out of scope first, then take mutable. Good good good afte
}

/// Accent color for build mode (default pink).
pub const ACCENT_BUILD: Color = Color::Rgb(139, 92, 246);
pub const ACCENT_BUILD: Color = crate::overlays::COVEN_CODE_ACCENT;
/// Accent color for plan mode (blue).
pub const ACCENT_PLAN: Color = Color::Rgb(66, 135, 245);
/// Accent color for explore mode (amber).
Expand Down Expand Up @@ -3418,25 +3418,17 @@ impl App {
KeyCode::Esc | KeyCode::F(2) => {
self.close_familiar_switcher();
}
KeyCode::Down => {
if len > 0 {
self.familiar_switcher_idx = (self.familiar_switcher_idx + 1) % len;
}
KeyCode::Down if len > 0 => {
self.familiar_switcher_idx = (self.familiar_switcher_idx + 1) % len;
}
KeyCode::Up => {
if len > 0 {
self.familiar_switcher_idx = (self.familiar_switcher_idx + len - 1) % len;
}
KeyCode::Up if len > 0 => {
self.familiar_switcher_idx = (self.familiar_switcher_idx + len - 1) % len;
}
KeyCode::Char('n') if ctrl => {
if len > 0 {
self.familiar_switcher_idx = (self.familiar_switcher_idx + 1) % len;
}
KeyCode::Char('n') if ctrl && len > 0 => {
self.familiar_switcher_idx = (self.familiar_switcher_idx + 1) % len;
}
KeyCode::Char('p') if ctrl => {
if len > 0 {
self.familiar_switcher_idx = (self.familiar_switcher_idx + len - 1) % len;
}
KeyCode::Char('p') if ctrl && len > 0 => {
self.familiar_switcher_idx = (self.familiar_switcher_idx + len - 1) % len;
}
KeyCode::Backspace => {
self.familiar_switcher_filter.pop();
Expand Down
4 changes: 2 additions & 2 deletions src-rust/crates/tui/src/ask_user_dialog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ pub fn render_ask_user_dialog(state: &AskUserDialogState, area: Rect, buf: &mut
// Option rows
if let Some(ref opts) = state.options {
for (i, opt) in opts.iter().enumerate() {
if row >= inner.y + inner.height - 2 {
if row >= inner.y + inner.height.saturating_sub(2) {
break;
}
let is_sel = !state.in_custom_input && state.selected_idx == i;
Expand Down Expand Up @@ -344,7 +344,7 @@ pub fn render_ask_user_dialog(state: &AskUserDialogState, area: Rect, buf: &mut
}

// Custom input row
if row < inner.y + inner.height - 1 {
if row < inner.y + inner.height.saturating_sub(1) {
let is_sel = state.in_custom_input || state.options.is_none();
let prefix = if is_sel { "❯ " } else { " " };
let cursor = if is_sel { "█" } else { "" };
Expand Down
2 changes: 1 addition & 1 deletion src-rust/crates/tui/src/device_auth_dialog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ pub fn render_device_auth_dialog(frame: &mut Frame, state: &DeviceAuthDialogStat
return;
}

let pink = Color::Rgb(139, 92, 246);
let pink = crate::overlays::COVEN_CODE_ACCENT;
let dim = Color::Rgb(90, 90, 90);
let dialog_bg = COVEN_CODE_PANEL_BG;
let green = Color::Rgb(80, 200, 120);
Expand Down
16 changes: 12 additions & 4 deletions src-rust/crates/tui/src/dialog_select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,9 @@ pub fn render_dialog_select(frame: &mut Frame, state: &DialogSelectState, area:

let dim = Color::Rgb(90, 90, 90);
let dialog_bg = COVEN_CODE_PANEL_BG;
let highlight_bg = Color::Rgb(139, 92, 246); // pink highlight bar
let highlight_bg = crate::overlays::COVEN_CODE_ACCENT; // pink highlight bar
let highlight_fg = Color::White;
let category_fg = Color::Rgb(139, 92, 246); // pink category names
let category_fg = crate::overlays::COVEN_CODE_ACCENT; // pink category names

// ── Darken the entire background ──
render_dark_overlay(frame, area);
Expand Down Expand Up @@ -315,9 +315,17 @@ pub fn render_dialog_select(frame: &mut Frame, state: &DialogSelectState, area:
(Color::White, dialog_bg)
};

// A leading marker + bold keeps the selected row distinguishable even
// when the highlight background can't render (NO_COLOR, monochrome, or
// a low-contrast terminal theme).
let marker = if is_selected { "> " } else { " " };
let mut title_style = Style::default().fg(item_fg).bg(item_bg);
if is_selected {
title_style = title_style.add_modifier(Modifier::BOLD);
}
let mut spans = vec![Span::styled(
format!(" {}", item.title),
Style::default().fg(item_fg).bg(item_bg),
format!("{}{}", marker, item.title),
title_style,
)];

// Auth hint in parens, dimmed
Expand Down
2 changes: 1 addition & 1 deletion src-rust/crates/tui/src/key_input_dialog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ pub fn render_key_input_dialog(frame: &mut Frame, state: &KeyInputDialogState, a
return;
}

let pink = Color::Rgb(139, 92, 246);
let pink = crate::overlays::COVEN_CODE_ACCENT;
let dim = Color::Rgb(90, 90, 90);
let dialog_bg = COVEN_CODE_PANEL_BG;

Expand Down
24 changes: 16 additions & 8 deletions src-rust/crates/tui/src/model_picker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -744,10 +744,10 @@ pub fn render_model_picker(state: &ModelPickerState, area: Rect, buf: &mut Buffe
use ratatui::prelude::Stylize;
use ratatui::widgets::Widget;

let _pink = Color::Rgb(139, 92, 246);
let _pink = crate::overlays::COVEN_CODE_ACCENT;
let dim = Color::Rgb(90, 90, 90);
let dialog_bg = COVEN_CODE_PANEL_BG;
let highlight_bg = Color::Rgb(139, 92, 246);
let highlight_bg = crate::overlays::COVEN_CODE_ACCENT;
let highlight_fg = Color::White;

// ── Dark overlay ──
Expand Down Expand Up @@ -893,20 +893,28 @@ pub fn render_model_picker(state: &ModelPickerState, area: Rect, buf: &mut Buffe

let mut spans: Vec<Span<'static>> = Vec::new();

// Current model indicator
// Leading column: the current-model dot takes priority; otherwise a
// selection caret so focus is visible without relying on the
// highlight background (NO_COLOR / monochrome / low-contrast themes).
if model.is_current {
spans.push(Span::styled(
" \u{25cf} ",
Style::default().fg(Color::Green).bg(bg),
));
} else if is_selected {
spans.push(Span::styled(
" > ",
Style::default().fg(fg).bg(bg).add_modifier(Modifier::BOLD),
));
} else {
spans.push(Span::styled(" ", Style::default().bg(bg)));
}

spans.push(Span::styled(
model.display_name.clone(),
Style::default().fg(fg).bg(bg),
));
let mut name_style = Style::default().fg(fg).bg(bg);
if is_selected {
name_style = name_style.add_modifier(Modifier::BOLD);
}
spans.push(Span::styled(model.display_name.clone(), name_style));

// Effort indicator
if supports_effort && is_selected {
Expand Down Expand Up @@ -978,7 +986,7 @@ pub fn render_model_picker(state: &ModelPickerState, area: Rect, buf: &mut Buffe
footer_spans.push(Span::raw(" "));
footer_spans.push(Span::styled(
" /connect",
Style::default().fg(Color::Rgb(139, 92, 246)),
Style::default().fg(crate::overlays::COVEN_CODE_ACCENT),
));
footer_spans.push(Span::styled(" providers", Style::default().fg(dim)));
Paragraph::new(Line::from(footer_spans))
Expand Down
6 changes: 3 additions & 3 deletions src-rust/crates/tui/src/onboarding_dialog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ const PROVIDER_ENTRIES: &[ProviderEntry] = &[

fn render_provider_setup_page(frame: &mut Frame, area: Rect) {
// Theme pink — matches the header and mascot
let pink = Color::Rgb(139, 92, 246);
let pink = crate::overlays::COVEN_CODE_ACCENT;
let dim = Color::Rgb(100, 100, 100);
let esc_red = Color::Red;

Expand Down Expand Up @@ -276,7 +276,7 @@ fn render_provider_setup_page(frame: &mut Frame, area: Rect) {
fn render_welcome_page(frame: &mut Frame, state: &OnboardingDialogState, area: Rect) {
use crate::overlays::{render_dark_overlay, render_dialog_bg, COVEN_CODE_PANEL_BG};

let pink = Color::Rgb(139, 92, 246);
let pink = crate::overlays::COVEN_CODE_ACCENT;
let dim = Color::Rgb(90, 90, 90);
let text = Color::Rgb(210, 210, 215);

Expand Down Expand Up @@ -365,7 +365,7 @@ fn render_welcome_page(frame: &mut Frame, state: &OnboardingDialogState, area: R
fn render_keybindings_page(frame: &mut Frame, state: &OnboardingDialogState, area: Rect) {
use crate::overlays::{render_dark_overlay, render_dialog_bg, COVEN_CODE_PANEL_BG};

let pink = Color::Rgb(139, 92, 246);
let pink = crate::overlays::COVEN_CODE_ACCENT;
let dim = Color::Rgb(90, 90, 90);
let text = Color::Rgb(210, 210, 215);

Expand Down
4 changes: 2 additions & 2 deletions src-rust/crates/tui/src/prompt_input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use ratatui::{
};
use unicode_width::{UnicodeWidthChar, UnicodeWidthStr};

const CLAUDE_ORANGE: Color = Color::Rgb(139, 92, 246);
const CLAUDE_ORANGE: Color = crate::overlays::COVEN_CODE_ACCENT;
const PROMPT_POINTER: &str = "\u{276f}";

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -3550,7 +3550,7 @@ pub fn render_prompt_input(
x: area.x,
y: area.y + 1,
width: area.width,
height: area.height - 1,
height: area.height.saturating_sub(1),
};
(rest, Some(pill_y))
} else {
Expand Down
Loading