feat: Add view stack to BottomPane (#4026)

Adds a "View Stack" to the bottom pane to allow for pushing/popping
bottom panels.

`esc` will go back instead of dismissing.

Benefit: We retain the "selection state" of a parent panel (e.g. the
review panel).
This commit is contained in:
dedrisian-oai
2025-09-22 11:29:39 -07:00
committed by GitHub
parent d2940bd4c3
commit 8daba53808
9 changed files with 70 additions and 109 deletions

View File

@@ -363,9 +363,6 @@ impl App {
AppEvent::OpenReviewCustomPrompt => {
self.chat_widget.show_review_custom_prompt();
}
AppEvent::OpenReviewPopup => {
self.chat_widget.open_review_popup();
}
}
Ok(true)
}

View File

@@ -76,7 +76,4 @@ pub(crate) enum AppEvent {
/// Open the custom prompt option from the review popup.
OpenReviewCustomPrompt,
/// Open the top-level review presets popup.
OpenReviewPopup,
}

View File

@@ -7,7 +7,6 @@ use crate::app_event_sender::AppEventSender;
use crate::user_approval_widget::ApprovalRequest;
use crate::user_approval_widget::UserApprovalWidget;
use super::BottomPane;
use super::BottomPaneView;
use super::CancellationEvent;
@@ -42,12 +41,12 @@ impl ApprovalModalView {
}
impl BottomPaneView for ApprovalModalView {
fn handle_key_event(&mut self, _pane: &mut BottomPane, key_event: KeyEvent) {
fn handle_key_event(&mut self, key_event: KeyEvent) {
self.current.handle_key_event(key_event);
self.maybe_advance();
}
fn on_ctrl_c(&mut self, _pane: &mut BottomPane) -> CancellationEvent {
fn on_ctrl_c(&mut self) -> CancellationEvent {
self.current.on_ctrl_c();
self.queue.clear();
CancellationEvent::Handled
@@ -75,6 +74,7 @@ impl BottomPaneView for ApprovalModalView {
mod tests {
use super::*;
use crate::app_event::AppEvent;
use crate::bottom_pane::BottomPane;
use tokio::sync::mpsc::unbounded_channel;
fn make_exec_request() -> ApprovalRequest {
@@ -94,7 +94,8 @@ mod tests {
view.enqueue_request(make_exec_request());
let (tx2, _rx2) = unbounded_channel::<AppEvent>();
let mut pane = BottomPane::new(super::super::BottomPaneParams {
// Why do we have this?
let _pane = BottomPane::new(super::super::BottomPaneParams {
app_event_tx: AppEventSender::new(tx2),
frame_requester: crate::tui::FrameRequester::test_dummy(),
has_input_focus: true,
@@ -102,7 +103,7 @@ mod tests {
placeholder_text: "Ask Codex to do anything".to_string(),
disable_paste_burst: false,
});
assert_eq!(CancellationEvent::Handled, view.on_ctrl_c(&mut pane));
assert_eq!(CancellationEvent::Handled, view.on_ctrl_c());
assert!(view.queue.is_empty());
assert!(view.current.is_complete());
assert!(view.is_complete());

View File

@@ -3,14 +3,13 @@ use crossterm::event::KeyEvent;
use ratatui::buffer::Buffer;
use ratatui::layout::Rect;
use super::BottomPane;
use super::CancellationEvent;
/// Trait implemented by every view that can be shown in the bottom pane.
pub(crate) trait BottomPaneView {
/// Handle a key event while the view is active. A redraw is always
/// scheduled after this call.
fn handle_key_event(&mut self, _pane: &mut BottomPane, _key_event: KeyEvent) {}
fn handle_key_event(&mut self, _key_event: KeyEvent) {}
/// Return `true` if the view has finished and should be removed.
fn is_complete(&self) -> bool {
@@ -18,7 +17,7 @@ pub(crate) trait BottomPaneView {
}
/// Handle Ctrl-C while this view is active.
fn on_ctrl_c(&mut self, _pane: &mut BottomPane) -> CancellationEvent {
fn on_ctrl_c(&mut self) -> CancellationEvent {
CancellationEvent::NotHandled
}
@@ -30,7 +29,7 @@ pub(crate) trait BottomPaneView {
/// Optional paste handler. Return true if the view modified its state and
/// needs a redraw.
fn handle_paste(&mut self, _pane: &mut BottomPane, _pasted: String) -> bool {
fn handle_paste(&mut self, _pasted: String) -> bool {
false
}

View File

@@ -13,8 +13,6 @@ use ratatui::widgets::Widget;
use std::cell::RefCell;
use super::popup_consts::STANDARD_POPUP_HINT_LINE;
use crate::app_event_sender::AppEventSender;
use crate::bottom_pane::SelectionAction;
use super::CancellationEvent;
use super::bottom_pane_view::BottomPaneView;
@@ -30,8 +28,6 @@ pub(crate) struct CustomPromptView {
placeholder: String,
context_label: Option<String>,
on_submit: PromptSubmitted,
app_event_tx: AppEventSender,
on_escape: Option<SelectionAction>,
// UI state
textarea: TextArea,
@@ -44,8 +40,6 @@ impl CustomPromptView {
title: String,
placeholder: String,
context_label: Option<String>,
app_event_tx: AppEventSender,
on_escape: Option<SelectionAction>,
on_submit: PromptSubmitted,
) -> Self {
Self {
@@ -53,8 +47,6 @@ impl CustomPromptView {
placeholder,
context_label,
on_submit,
app_event_tx,
on_escape,
textarea: TextArea::new(),
textarea_state: RefCell::new(TextAreaState::default()),
complete: false,
@@ -63,12 +55,12 @@ impl CustomPromptView {
}
impl BottomPaneView for CustomPromptView {
fn handle_key_event(&mut self, _pane: &mut super::BottomPane, key_event: KeyEvent) {
fn handle_key_event(&mut self, key_event: KeyEvent) {
match key_event {
KeyEvent {
code: KeyCode::Esc, ..
} => {
self.on_ctrl_c(_pane);
self.on_ctrl_c();
}
KeyEvent {
code: KeyCode::Enter,
@@ -93,11 +85,8 @@ impl BottomPaneView for CustomPromptView {
}
}
fn on_ctrl_c(&mut self, _pane: &mut super::BottomPane) -> CancellationEvent {
fn on_ctrl_c(&mut self) -> CancellationEvent {
self.complete = true;
if let Some(cb) = &self.on_escape {
cb(&self.app_event_tx);
}
CancellationEvent::Handled
}
@@ -212,7 +201,7 @@ impl BottomPaneView for CustomPromptView {
}
}
fn handle_paste(&mut self, _pane: &mut super::BottomPane, pasted: String) -> bool {
fn handle_paste(&mut self, pasted: String) -> bool {
if pasted.is_empty() {
return false;
}

View File

@@ -11,7 +11,6 @@ use ratatui::widgets::Widget;
use crate::app_event_sender::AppEventSender;
use super::BottomPane;
use super::CancellationEvent;
use super::bottom_pane_view::BottomPaneView;
use super::popup_consts::MAX_POPUP_ROWS;
@@ -40,7 +39,6 @@ pub(crate) struct SelectionViewParams {
pub items: Vec<SelectionItem>,
pub is_searchable: bool,
pub search_placeholder: Option<String>,
pub on_escape: Option<SelectionAction>,
}
pub(crate) struct ListSelectionView {
@@ -51,7 +49,6 @@ pub(crate) struct ListSelectionView {
state: ScrollState,
complete: bool,
app_event_tx: AppEventSender,
on_escape: Option<SelectionAction>,
is_searchable: bool,
search_query: String,
search_placeholder: Option<String>,
@@ -77,7 +74,6 @@ impl ListSelectionView {
state: ScrollState::new(),
complete: false,
app_event_tx,
on_escape: params.on_escape,
is_searchable: params.is_searchable,
search_query: String::new(),
search_placeholder: if params.is_searchable {
@@ -221,7 +217,7 @@ impl ListSelectionView {
}
impl BottomPaneView for ListSelectionView {
fn handle_key_event(&mut self, _pane: &mut BottomPane, key_event: KeyEvent) {
fn handle_key_event(&mut self, key_event: KeyEvent) {
match key_event {
KeyEvent {
code: KeyCode::Up, ..
@@ -240,7 +236,7 @@ impl BottomPaneView for ListSelectionView {
KeyEvent {
code: KeyCode::Esc, ..
} => {
self.on_ctrl_c(_pane);
self.on_ctrl_c();
}
KeyEvent {
code: KeyCode::Char(c),
@@ -266,11 +262,8 @@ impl BottomPaneView for ListSelectionView {
self.complete
}
fn on_ctrl_c(&mut self, _pane: &mut BottomPane) -> CancellationEvent {
fn on_ctrl_c(&mut self) -> CancellationEvent {
self.complete = true;
if let Some(cb) = &self.on_escape {
cb(&self.app_event_tx);
}
CancellationEvent::Handled
}

View File

@@ -7,6 +7,7 @@ use crate::user_approval_widget::ApprovalRequest;
use bottom_pane_view::BottomPaneView;
use codex_core::protocol::TokenUsageInfo;
use codex_file_search::FileMatch;
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use ratatui::buffer::Buffer;
use ratatui::layout::Constraint;
@@ -51,8 +52,8 @@ pub(crate) struct BottomPane {
/// input state is retained when the view is closed.
composer: ChatComposer,
/// If present, this is displayed instead of the `composer` (e.g. modals).
active_view: Option<Box<dyn BottomPaneView>>,
/// Stack of views displayed instead of the composer (e.g. popups/modals).
view_stack: Vec<Box<dyn BottomPaneView>>,
app_event_tx: AppEventSender,
frame_requester: FrameRequester,
@@ -89,7 +90,7 @@ impl BottomPane {
params.placeholder_text,
params.disable_paste_burst,
),
active_view: None,
view_stack: Vec::new(),
app_event_tx: params.app_event_tx,
frame_requester: params.frame_requester,
has_input_focus: params.has_input_focus,
@@ -101,12 +102,21 @@ impl BottomPane {
}
}
fn active_view(&self) -> Option<&dyn BottomPaneView> {
self.view_stack.last().map(|view| view.as_ref())
}
fn push_view(&mut self, view: Box<dyn BottomPaneView>) {
self.view_stack.push(view);
self.request_redraw();
}
pub fn desired_height(&self, width: u16) -> u16 {
// Always reserve one blank row above the pane for visual spacing.
let top_margin = 1;
// Base height depends on whether a modal/overlay is active.
let base = match self.active_view.as_ref() {
let base = match self.active_view().as_ref() {
Some(view) => view.desired_height(width),
None => self.composer.desired_height(width).saturating_add(
self.status
@@ -133,7 +143,7 @@ impl BottomPane {
width: area.width,
height: area.height - top_margin - bottom_margin,
};
match self.active_view.as_ref() {
match self.active_view() {
Some(_) => [Rect::ZERO, area],
None => {
let status_height = self
@@ -151,7 +161,7 @@ impl BottomPane {
// In these states the textarea is not interactable, so we should not
// show its caret.
let [_, content] = self.layout(area);
if let Some(view) = self.active_view.as_ref() {
if let Some(view) = self.active_view() {
view.cursor_pos(content)
} else {
self.composer.cursor_pos(content)
@@ -160,12 +170,20 @@ impl BottomPane {
/// Forward a key event to the active view or the composer.
pub fn handle_key_event(&mut self, key_event: KeyEvent) -> InputResult {
if let Some(mut view) = self.active_view.take() {
view.handle_key_event(self, key_event);
if !view.is_complete() {
self.active_view = Some(view);
} else {
// If a modal/view is active, handle it here; otherwise forward to composer.
if let Some(view) = self.view_stack.last_mut() {
if key_event.code == KeyCode::Esc
&& matches!(view.on_ctrl_c(), CancellationEvent::Handled)
&& view.is_complete()
{
self.view_stack.pop();
self.on_active_view_complete();
} else {
view.handle_key_event(key_event);
if view.is_complete() {
self.view_stack.clear();
self.on_active_view_complete();
}
}
self.request_redraw();
InputResult::None
@@ -195,43 +213,31 @@ impl BottomPane {
/// Handle Ctrl-C in the bottom pane. If a modal view is active it gets a
/// chance to consume the event (e.g. to dismiss itself).
pub(crate) fn on_ctrl_c(&mut self) -> CancellationEvent {
let mut view = match self.active_view.take() {
Some(view) => view,
None => {
return if self.composer_is_empty() {
CancellationEvent::NotHandled
} else {
self.set_composer_text(String::new());
self.show_ctrl_c_quit_hint();
CancellationEvent::Handled
};
}
};
let event = view.on_ctrl_c(self);
match event {
CancellationEvent::Handled => {
if !view.is_complete() {
self.active_view = Some(view);
} else {
if let Some(view) = self.view_stack.last_mut() {
let event = view.on_ctrl_c();
if matches!(event, CancellationEvent::Handled) {
if view.is_complete() {
self.view_stack.pop();
self.on_active_view_complete();
}
self.show_ctrl_c_quit_hint();
}
CancellationEvent::NotHandled => {
self.active_view = Some(view);
}
event
} else if self.composer_is_empty() {
CancellationEvent::NotHandled
} else {
self.view_stack.pop();
self.set_composer_text(String::new());
self.show_ctrl_c_quit_hint();
CancellationEvent::Handled
}
event
}
pub fn handle_paste(&mut self, pasted: String) {
if let Some(mut view) = self.active_view.take() {
let needs_redraw = view.handle_paste(self, pasted);
if let Some(view) = self.view_stack.last_mut() {
let needs_redraw = view.handle_paste(pasted);
if view.is_complete() {
self.on_active_view_complete();
} else {
self.active_view = Some(view);
}
if needs_redraw {
self.request_redraw();
@@ -332,7 +338,7 @@ impl BottomPane {
/// Show a generic list selection view with the provided items.
pub(crate) fn show_selection_view(&mut self, params: list_selection_view::SelectionViewParams) {
let view = list_selection_view::ListSelectionView::new(params, self.app_event_tx.clone());
self.active_view = Some(Box::new(view));
self.push_view(Box::new(view));
}
/// Update the queued messages shown under the status header.
@@ -362,7 +368,7 @@ impl BottomPane {
/// overlays or popups and not running a task. This is the safe context to
/// use Esc-Esc for backtracking from the main view.
pub(crate) fn is_normal_backtrack_mode(&self) -> bool {
!self.is_task_running && self.active_view.is_none() && !self.composer.popup_active()
!self.is_task_running && self.view_stack.is_empty() && !self.composer.popup_active()
}
/// Update the *context-window remaining* indicator in the composer. This
@@ -373,13 +379,12 @@ impl BottomPane {
}
pub(crate) fn show_view(&mut self, view: Box<dyn BottomPaneView>) {
self.active_view = Some(view);
self.request_redraw();
self.push_view(view);
}
/// Called when the agent requests user approval.
pub fn push_approval_request(&mut self, request: ApprovalRequest) {
let request = if let Some(view) = self.active_view.as_mut() {
let request = if let Some(view) = self.view_stack.last_mut() {
match view.try_consume_approval_request(request) {
Some(request) => request,
None => {
@@ -394,8 +399,7 @@ impl BottomPane {
// Otherwise create a new approval modal overlay.
let modal = ApprovalModalView::new(request, self.app_event_tx.clone());
self.pause_status_timer_for_modal();
self.active_view = Some(Box::new(modal));
self.request_redraw()
self.push_view(Box::new(modal));
}
fn on_active_view_complete(&mut self) {
@@ -464,7 +468,7 @@ impl BottomPane {
height: u32,
format_label: &str,
) {
if self.active_view.is_none() {
if self.view_stack.is_empty() {
self.composer
.attach_image(path, width, height, format_label);
self.request_redraw();
@@ -481,7 +485,7 @@ impl WidgetRef for &BottomPane {
let [status_area, content] = self.layout(area);
// When a modal view is active, it owns the whole content area.
if let Some(view) = &self.active_view {
if let Some(view) = self.active_view() {
view.render(content, buf);
} else {
// No active modal:
@@ -591,7 +595,7 @@ mod tests {
// After denial, since the task is still running, the status indicator should be
// visible above the composer. The modal should be gone.
assert!(
pane.active_view.is_none(),
pane.view_stack.is_empty(),
"no active modal view after denial"
);

View File

@@ -1645,7 +1645,6 @@ impl ChatWidget {
title: "Select a review preset".into(),
footer_hint: Some(STANDARD_POPUP_HINT_LINE.to_string()),
items,
on_escape: None,
..Default::default()
});
}
@@ -1684,7 +1683,6 @@ impl ChatWidget {
items,
is_searchable: true,
search_placeholder: Some("Type to search branches".to_string()),
on_escape: Some(Box::new(|tx| tx.send(AppEvent::OpenReviewPopup))),
..Default::default()
});
}
@@ -1726,7 +1724,6 @@ impl ChatWidget {
items,
is_searchable: true,
search_placeholder: Some("Type to search commits".to_string()),
on_escape: Some(Box::new(|tx| tx.send(AppEvent::OpenReviewPopup))),
..Default::default()
});
}
@@ -1737,8 +1734,6 @@ impl ChatWidget {
"Custom review instructions".to_string(),
"Type instructions and press Enter".to_string(),
None,
self.app_event_tx.clone(),
Some(Box::new(|tx| tx.send(AppEvent::OpenReviewPopup))),
Box::new(move |prompt: String| {
let trimmed = prompt.trim().to_string();
if trimmed.is_empty() {

View File

@@ -851,7 +851,7 @@ fn interrupt_exec_marks_failed_snapshot() {
/// parent popup, pressing Esc again dismisses all panels (back to normal mode).
#[test]
fn review_custom_prompt_escape_navigates_back_then_dismisses() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual();
let (mut chat, _rx, _op_rx) = make_chatwidget_manual();
// Open the Review presets parent popup.
chat.open_review_popup();
@@ -868,13 +868,6 @@ fn review_custom_prompt_escape_navigates_back_then_dismisses() {
// Esc once: child view closes, parent (review presets) remains.
chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE));
// Process emitted app events to reopen the parent review popup.
while let Ok(ev) = rx.try_recv() {
if let AppEvent::OpenReviewPopup = ev {
chat.open_review_popup();
break;
}
}
let header = render_bottom_first_row(&chat, 60);
assert!(
header.contains("Select a review preset"),
@@ -893,7 +886,7 @@ fn review_custom_prompt_escape_navigates_back_then_dismisses() {
/// parent popup, pressing Esc again dismisses all panels (back to normal mode).
#[tokio::test(flavor = "current_thread")]
async fn review_branch_picker_escape_navigates_back_then_dismisses() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual();
let (mut chat, _rx, _op_rx) = make_chatwidget_manual();
// Open the Review presets parent popup.
chat.open_review_popup();
@@ -911,13 +904,6 @@ async fn review_branch_picker_escape_navigates_back_then_dismisses() {
// Esc once: child view closes, parent remains.
chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE));
// Process emitted app events to reopen the parent review popup.
while let Ok(ev) = rx.try_recv() {
if let AppEvent::OpenReviewPopup = ev {
chat.open_review_popup();
break;
}
}
let header = render_bottom_first_row(&chat, 60);
assert!(
header.contains("Select a review preset"),