Compare commits

...

1 Commits

Author SHA1 Message Date
easong-openai
dbbded2c42 fix approval display 2025-07-30 19:27:47 -07:00
5 changed files with 85 additions and 6 deletions

View File

@@ -344,10 +344,16 @@ impl App<'_> {
}
let size = terminal.size()?;
// Determine how tall the bottom pane should be, then clamp it so it never
// exceeds the terminal height and also never grows beyond 10 rows. This keeps
// the dynamically resizable bottom pane from taking over the screen while
// still respecting very small terminals.
let desired_height = match &self.app_state {
AppState::Chat { widget } => widget.desired_height(),
AppState::GitWarning { .. } => 10,
};
let max_allowed_height = size.height.min(10);
let desired_height = desired_height.min(max_allowed_height);
let mut area = terminal.viewport_area;
area.height = desired_height;
area.width = size.width;

View File

@@ -65,12 +65,22 @@ impl<'a> BottomPaneView<'a> for ApprovalModalView<'a> {
self.enqueue_request(req);
None
}
fn min_desired_height(&self) -> u16 {
// Ensure there is enough room for all approval options (select mode)
// plus the surrounding border drawn by the modal.
(super::super::user_approval_widget::APPROVAL_OPTION_LABELS.len() as u16) + 2
}
}
#[cfg(test)]
mod tests {
#![allow(clippy::unwrap_used)]
use super::*;
use crate::app_event::AppEvent;
use crate::user_approval_widget::APPROVAL_OPTION_LABELS;
use ratatui::Terminal;
use ratatui::backend::TestBackend;
use std::path::PathBuf;
use std::sync::mpsc::channel;
@@ -101,4 +111,26 @@ mod tests {
assert!(view.current.is_complete());
assert!(view.is_complete());
}
#[test]
fn renders_all_select_options_with_min_height() {
let (tx_raw, _rx) = channel::<AppEvent>();
let tx = AppEventSender::new(tx_raw);
let view = ApprovalModalView::new(make_exec_request(), tx);
// Use the view's minimum desired height to render.
let min_h = (APPROVAL_OPTION_LABELS.len() as u16) + 2; // options + border
let mut terminal = Terminal::new(TestBackend::new(80, min_h)).unwrap();
terminal
.draw(|f| {
view.render(f.area(), f.buffer_mut());
})
.unwrap();
// Ensure every option label is present in the buffer output.
let dump = format!("{:?}", terminal.backend());
for label in APPROVAL_OPTION_LABELS {
assert!(dump.contains(label), "missing option: {label}");
}
}
}

View File

@@ -49,4 +49,11 @@ pub(crate) trait BottomPaneView<'a> {
) -> Option<ApprovalRequest> {
Some(request)
}
/// Minimal height (in terminal rows) this view needs to be usable. The
/// bottom pane will ensure at least this many rows are allocated while the
/// view is active, even if the composer would normally request less.
fn min_desired_height(&self) -> u16 {
3
}
}

View File

@@ -65,7 +65,12 @@ impl BottomPane<'_> {
}
pub fn desired_height(&self) -> u16 {
self.composer.desired_height()
let composer_height = self.composer.desired_height();
if let Some(view) = &self.active_view {
composer_height.max(view.min_desired_height())
} else {
composer_height
}
}
/// Forward a key event to the active view or the composer.
@@ -276,6 +281,7 @@ impl WidgetRef for &BottomPane<'_> {
mod tests {
use super::*;
use crate::app_event::AppEvent;
use crate::user_approval_widget::APPROVAL_OPTION_LABELS;
use std::path::PathBuf;
use std::sync::mpsc::channel;
@@ -301,4 +307,23 @@ mod tests {
assert!(pane.ctrl_c_quit_hint_visible());
assert_eq!(CancellationEvent::Ignored, pane.on_ctrl_c());
}
#[test]
fn approval_modal_has_min_desired_height() {
let (tx_raw, _rx) = channel::<AppEvent>();
let tx = AppEventSender::new(tx_raw);
let mut pane = BottomPane::new(BottomPaneParams {
app_event_tx: tx,
has_input_focus: true,
});
// With no overlay, desired height is just the composer height (>= 3 typical).
let base = pane.desired_height();
assert!(base >= 1);
// Push an approval request desired height should be at least enough
// to show all options plus the modal border.
pane.push_approval_request(exec_request());
let min_modal = (APPROVAL_OPTION_LABELS.len() as u16) + 2;
assert!(pane.desired_height() >= min_modal);
}
}

View File

@@ -55,34 +55,43 @@ struct SelectOption {
enters_input_mode: bool,
}
// User-visible labels for approval options, exported for tests and layout.
pub(crate) const APPROVAL_OPTION_LABELS: &[&str] = &[
"Yes (y)",
"Yes, always approve this exact command for this session (a)",
"Edit or give feedback (e)",
"No, and keep going (n)",
"No, and stop for now (esc)",
];
// keep in same order as in the TS implementation
const SELECT_OPTIONS: &[SelectOption] = &[
SelectOption {
label: "Yes (y)",
label: APPROVAL_OPTION_LABELS[0],
decision: Some(ReviewDecision::Approved),
enters_input_mode: false,
},
SelectOption {
label: "Yes, always approve this exact command for this session (a)",
label: APPROVAL_OPTION_LABELS[1],
decision: Some(ReviewDecision::ApprovedForSession),
enters_input_mode: false,
},
SelectOption {
label: "Edit or give feedback (e)",
label: APPROVAL_OPTION_LABELS[2],
decision: None,
enters_input_mode: true,
},
SelectOption {
label: "No, and keep going (n)",
label: APPROVAL_OPTION_LABELS[3],
decision: Some(ReviewDecision::Denied),
enters_input_mode: false,
},
SelectOption {
label: "No, and stop for now (esc)",
label: APPROVAL_OPTION_LABELS[4],
decision: Some(ReviewDecision::Abort),
enters_input_mode: false,