tui: tweaks to dialog display

codex-rs/tui/src/bottom_pane/approval_overlay.rs

@@ -12,6 +12,7 @@ use crate::diff_render::DiffSummary;
 use crate::exec_command::strip_bash_lc_and_escape;
 use crate::history_cell;
 use crate::key_hint;
+use crate::key_hint::KeyBinding;
 use crate::render::highlight::highlight_bash_to_lines;
 use crate::render::renderable::ColumnRenderable;
 use crate::render::renderable::Renderable;
@@ -94,8 +95,14 @@ impl ApprovalOverlay {
         header: Box<dyn Renderable>,
     ) -> (Vec<ApprovalOption>, SelectionViewParams) {
         let (options, title) = match &variant {
-            ApprovalVariant::Exec { .. } => (exec_options(), "Allow command?".to_string()),
-            ApprovalVariant::ApplyPatch { .. } => (patch_options(), "Apply changes?".to_string()),
+            ApprovalVariant::Exec { .. } => (
+                exec_options(),
+                "Would you like to run the following command?".to_string(),
+            ),
+            ApprovalVariant::ApplyPatch { .. } => (
+                patch_options(),
+                "Would you like to make the following edits?".to_string(),
+            ),
         };
 
         let header = Box::new(ColumnRenderable::new([
@@ -108,11 +115,9 @@ impl ApprovalOverlay {
             .iter()
             .map(|opt| SelectionItem {
                 name: opt.label.clone(),
-                description: Some(opt.description.clone()),
-                is_current: false,
-                actions: Vec::new(),
+                display_shortcut: opt.display_shortcut,
                 dismiss_on_select: false,
-                search_value: None,
+                ..Default::default()
             })
             .collect();
 
@@ -197,28 +202,18 @@ impl ApprovalOverlay {
                     false
                 }
             }
-            KeyEvent {
-                kind: KeyEventKind::Press,
-                code: KeyCode::Char(c),
-                modifiers,
-                ..
-            } if !modifiers.contains(KeyModifiers::CONTROL)
-                && !modifiers.contains(KeyModifiers::ALT) =>
-            {
-                let lower = c.to_ascii_lowercase();
-                match self
+            e => {
+                if let Some(idx) = self
                     .options
                     .iter()
-                    .position(|opt| opt.shortcut.map(|s| s == lower).unwrap_or(false))
+                    .position(|opt| opt.shortcuts().any(|s| s.is_press(*e)))
                 {
-                    Some(idx) => {
-                        self.apply_selection(idx);
-                        true
-                    }
-                    None => false,
+                    self.apply_selection(idx);
+                    true
+                } else {
+                    false
                 }
             }
-            _ => false,
         }
     }
 }
@@ -299,7 +294,7 @@ impl From<ApprovalRequest> for ApprovalRequestState {
                 if let Some(reason) = reason
                     && !reason.is_empty()
                 {
-                    header.push(reason.italic().into());
+                    header.push(Line::from(vec!["Reason: ".into(), reason.italic()]));
                     header.push(Line::from(""));
                 }
                 let full_cmd = strip_bash_lc_and_escape(&command);
@@ -347,48 +342,55 @@ enum ApprovalVariant {
 #[derive(Clone)]
 struct ApprovalOption {
     label: String,
-    description: String,
     decision: ReviewDecision,
-    shortcut: Option<char>,
+    display_shortcut: Option<KeyBinding>,
+    additional_shortcuts: Vec<KeyBinding>,
+}
+
+impl ApprovalOption {
+    fn shortcuts(&self) -> impl Iterator<Item = KeyBinding> + '_ {
+        self.display_shortcut
+            .into_iter()
+            .chain(self.additional_shortcuts.iter().copied())
+    }
 }
 
 fn exec_options() -> Vec<ApprovalOption> {
     vec![
         ApprovalOption {
-            label: "Approve and run now".to_string(),
-            description: "Run this command one time".to_string(),
+            label: "Yes, proceed".to_string(),
             decision: ReviewDecision::Approved,
-            shortcut: Some('y'),
+            display_shortcut: None,
+            additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))],
         },
         ApprovalOption {
-            label: "Always approve this session".to_string(),
-            description: "Automatically approve this command for the rest of the session"
-                .to_string(),
+            label: "Yes, and don't ask again for this command".to_string(),
             decision: ReviewDecision::ApprovedForSession,
-            shortcut: Some('a'),
+            display_shortcut: None,
+            additional_shortcuts: vec![key_hint::plain(KeyCode::Char('a'))],
         },
         ApprovalOption {
-            label: "Cancel".to_string(),
-            description: "Do not run the command".to_string(),
+            label: "No, and tell Codex what to do differently".to_string(),
             decision: ReviewDecision::Abort,
-            shortcut: Some('n'),
+            display_shortcut: Some(key_hint::plain(KeyCode::Esc)),
+            additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))],
         },
     ]
 }
 
 fn patch_options() -> Vec<ApprovalOption> {
     vec![
         ApprovalOption {
-            label: "Approve".to_string(),
-            description: "Apply the proposed changes".to_string(),
+            label: "Yes, proceed".to_string(),
             decision: ReviewDecision::Approved,
-            shortcut: Some('y'),
+            display_shortcut: None,
+            additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))],
         },
         ApprovalOption {
-            label: "Cancel".to_string(),
-            description: "Do not apply the changes".to_string(),
+            label: "No, and tell Codex what to do differently".to_string(),
             decision: ReviewDecision::Abort,
-            shortcut: Some('n'),
+            display_shortcut: Some(key_hint::plain(KeyCode::Esc)),
+            additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))],
         },
     ]
 }

codex-rs/tui/src/bottom_pane/command_popup.rs

@@ -173,6 +173,7 @@ impl CommandPopup {
                     name,
                     match_indices: indices.map(|v| v.into_iter().map(|i| i + 1).collect()),
                     is_current: false,
+                    display_shortcut: None,
                     description: Some(description),
                 }
             })

codex-rs/tui/src/bottom_pane/file_search_popup.rs

@@ -130,6 +130,7 @@ impl WidgetRef for &FileSearchPopup {
                         .as_ref()
                         .map(|v| v.iter().map(|&i| i as usize).collect()),
                     is_current: false,
+                    display_shortcut: None,
                     description: None,
                 })
                 .collect()

codex-rs/tui/src/bottom_pane/list_selection_view.rs

@@ -1,6 +1,7 @@
 use crossterm::event::KeyCode;
 use crossterm::event::KeyEvent;
 use crossterm::event::KeyModifiers;
+use itertools::Itertools as _;
 use ratatui::buffer::Buffer;
 use ratatui::layout::Constraint;
 use ratatui::layout::Layout;
@@ -13,6 +14,7 @@ use ratatui::widgets::Paragraph;
 use ratatui::widgets::Widget;
 
 use crate::app_event_sender::AppEventSender;
+use crate::key_hint::KeyBinding;
 use crate::render::Insets;
 use crate::render::RectExt as _;
 use crate::render::renderable::ColumnRenderable;
@@ -31,8 +33,10 @@ use super::selection_popup_common::render_rows;
 /// One selectable item in the generic selection list.
 pub(crate) type SelectionAction = Box<dyn Fn(&AppEventSender) + Send + Sync>;
 
+#[derive(Default)]
 pub(crate) struct SelectionItem {
     pub name: String,
+    pub display_shortcut: Option<KeyBinding>,
     pub description: Option<String>,
     pub is_current: bool,
     pub actions: Vec<SelectionAction>,
@@ -135,18 +139,10 @@ impl ListSelectionView {
             self.filtered_indices = self
                 .items
                 .iter()
-                .enumerate()
-                .filter_map(|(idx, item)| {
-                    let matches = if let Some(search_value) = &item.search_value {
-                        search_value.to_lowercase().contains(&query_lower)
-                    } else {
-                        let mut matches = item.name.to_lowercase().contains(&query_lower);
-                        if !matches && let Some(desc) = &item.description {
-                            matches = desc.to_lowercase().contains(&query_lower);
-                        }
-                        matches
-                    };
-                    matches.then_some(idx)
+                .positions(|item| {
+                    item.search_value
+                        .as_ref()
+                        .is_some_and(|v| v.to_lowercase().contains(&query_lower))
                 })
                 .collect();
         } else {
@@ -200,6 +196,7 @@ impl ListSelectionView {
                     };
                     GenericDisplayRow {
                         name: display_name,
+                        display_shortcut: item.display_shortcut,
                         match_indices: None,
                         is_current: item.is_current,
                         description: item.description.clone(),
@@ -329,7 +326,8 @@ impl Renderable for ListSelectionView {
 
         let rows_height = measure_rows_height(&rows, &self.state, MAX_POPUP_ROWS, width);
 
-        let mut height = self.header.desired_height(width);
+        // Subtract 4 for the padding on the left and right of the header.
+        let mut height = self.header.desired_height(width.saturating_sub(4));
         height = height.saturating_add(rows_height + 3);
         if self.is_searchable {
             height = height.saturating_add(1);
@@ -355,7 +353,10 @@ impl Renderable for ListSelectionView {
             .style(user_message_style(terminal_palette::default_bg()))
             .render(content_area, buf);
 
-        let header_height = self.header.desired_height(content_area.width);
+        let header_height = self
+            .header
+            // Subtract 4 for the padding on the left and right of the header.
+            .desired_height(content_area.width.saturating_sub(4));
         let rows = self.build_rows();
         let rows_height =
             measure_rows_height(&rows, &self.state, MAX_POPUP_ROWS, content_area.width);
@@ -438,17 +439,15 @@ mod tests {
                 name: "Read Only".to_string(),
                 description: Some("Codex can read files".to_string()),
                 is_current: true,
-                actions: vec![],
                 dismiss_on_select: true,
-                search_value: None,
+                ..Default::default()
             },
             SelectionItem {
                 name: "Full Access".to_string(),
                 description: Some("Codex can edit files".to_string()),
                 is_current: false,
-                actions: vec![],
                 dismiss_on_select: true,
-                search_value: None,
+                ..Default::default()
             },
         ];
         ListSelectionView::new(
@@ -510,9 +509,8 @@ mod tests {
             name: "Read Only".to_string(),
             description: Some("Codex can read files".to_string()),
             is_current: false,
-            actions: vec![],
             dismiss_on_select: true,
-            search_value: None,
+            ..Default::default()
         }];
         let mut view = ListSelectionView::new(
             SelectionViewParams {

codex-rs/tui/src/bottom_pane/selection_popup_common.rs

@@ -9,11 +9,14 @@ use ratatui::text::Span;
 use ratatui::widgets::Widget;
 use unicode_width::UnicodeWidthChar;
 
+use crate::key_hint::KeyBinding;
+
 use super::scroll_state::ScrollState;
 
 /// A generic representation of a display row for selection popups.
 pub(crate) struct GenericDisplayRow {
     pub name: String,
+    pub display_shortcut: Option<KeyBinding>,
     pub match_indices: Option<Vec<usize>>, // indices to bold (char positions)
     pub is_current: bool,
     pub description: Option<String>, // optional grey text after the name
@@ -92,6 +95,10 @@ fn build_full_line(row: &GenericDisplayRow, desc_col: usize) -> Line<'static> {
 
     let this_name_width = Line::from(name_spans.clone()).width();
     let mut full_spans: Vec<Span> = name_spans;
+    if let Some(display_shortcut) = row.display_shortcut {
+        full_spans.push(" ".into());
+        full_spans.push(display_shortcut.into());
+    }
     if let Some(desc) = row.description.as_ref() {
         let gap = desc_col.saturating_sub(this_name_width);
         if gap > 0 {
@@ -155,6 +162,7 @@ pub(crate) fn render_rows(
         let GenericDisplayRow {
             name,
             match_indices,
+            display_shortcut,
             is_current: _is_current,
             description,
         } = row;
@@ -163,6 +171,7 @@ pub(crate) fn render_rows(
             &GenericDisplayRow {
                 name: name.clone(),
                 match_indices: match_indices.clone(),
+                display_shortcut: *display_shortcut,
                 is_current: *_is_current,
                 description: description.clone(),
             },