mirror of
https://github.com/openai/codex.git
synced 2026-11-12 21:20:34 +00:00
Refactor TUI sidebar layout and focus handling around panels (#61167)
## What changed Rename `SidebarLayout::Chooser` to `SidebarLayout::Panel` and use panel helpers for geometry, transcript visibility, focus routing, and shortcut hints. Retain `chooser_focus()` for chooser-specific logic. When changing panel focus, reuse the pane's visible content instead of explicitly selecting `SidebarContent::Chooser`. Update existing sidebar tests to use the renamed geometry helper. GitOrigin-RevId: ee6180bad691da3a76bd23835077aeb11c7d969d
This commit is contained in:
@@ -866,7 +866,7 @@ impl App {
|
||||
self.transcript_view
|
||||
.sync_patch_approval(self.chat_widget.current_patch_approval());
|
||||
// A chooser action must never decide a modal that has not been painted yet.
|
||||
if (self.conversation_frame.chooser_area().is_some()
|
||||
if (self.conversation_frame.panel_area().is_some()
|
||||
|| self.sidebar().is_some_and(|pane| pane.close.is_some()))
|
||||
&& self.chat_widget.has_active_modal()
|
||||
&& matches!(
|
||||
@@ -1353,7 +1353,7 @@ impl App {
|
||||
let frozen = self.chat_widget.is_external_writer_view();
|
||||
let visible = self.current_displayed_thread_id().filter(|&id| {
|
||||
tui.is_terminal_focused()
|
||||
&& !self.conversation_frame.chooser_covers_transcript()
|
||||
&& !self.conversation_frame.panel_covers_transcript()
|
||||
&& matches!(self.overlay, None | Some(Overlay::Transcript(_)))
|
||||
&& self.chat_widget.no_modal_or_popup_active()
|
||||
&& self.initial_history_replay_buffer.is_none()
|
||||
|
||||
@@ -56,20 +56,20 @@ impl ConversationFrame {
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn chooser_covers_transcript(&self) -> bool {
|
||||
matches!(self.sidebar, Some(sidebar::SidebarLayout::Chooser { main, .. }) if main.is_empty())
|
||||
pub(super) fn panel_covers_transcript(&self) -> bool {
|
||||
matches!(self.sidebar, Some(sidebar::SidebarLayout::Panel { main, .. }) if main.is_empty())
|
||||
}
|
||||
|
||||
pub(super) fn chooser_area(&self) -> Option<Rect> {
|
||||
pub(super) fn panel_area(&self) -> Option<Rect> {
|
||||
match self.sidebar {
|
||||
Some(sidebar::SidebarLayout::Chooser { sidebar, .. }) => Some(sidebar),
|
||||
Some(sidebar::SidebarLayout::Panel { sidebar, .. }) => Some(sidebar),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn sidebar_tab_strip(&self) -> Option<Rect> {
|
||||
self.chat_strip.or_else(|| {
|
||||
self.chooser_area()
|
||||
self.panel_area()
|
||||
.map(|area| side_pane::tab_and_body(area)[0])
|
||||
})
|
||||
}
|
||||
@@ -269,7 +269,7 @@ impl App {
|
||||
Some(sidebar::SidebarLayout::Chat(panes)) => {
|
||||
(Some(panes.focused), Some(panes.tab_strip))
|
||||
}
|
||||
Some(sidebar::SidebarLayout::Chooser { main, .. }) => {
|
||||
Some(sidebar::SidebarLayout::Panel { main, .. }) => {
|
||||
((!main.is_empty()).then_some(*main), None)
|
||||
}
|
||||
None if chat == TabState::Active
|
||||
@@ -342,7 +342,7 @@ impl App {
|
||||
);
|
||||
}
|
||||
}
|
||||
Some(sidebar::SidebarLayout::Chooser { sidebar, focus, .. }) => {
|
||||
Some(sidebar::SidebarLayout::Panel { sidebar, focus, .. }) => {
|
||||
let focused = *focus == PaneFocus::Sidebar;
|
||||
let resize_hint = self.pane_resize.map(|_| side_pane::RESIZE_HINT);
|
||||
if let Some(creation) = self.sidebar().and_then(|pane| pane.creation.as_ref()) {
|
||||
@@ -457,7 +457,7 @@ impl App {
|
||||
let viewport = frame.area();
|
||||
let column = self
|
||||
.conversation_frame
|
||||
.chooser_area()
|
||||
.panel_area()
|
||||
.or(strip)
|
||||
.unwrap_or(viewport);
|
||||
let area = Rect {
|
||||
@@ -584,7 +584,7 @@ impl App {
|
||||
let max_composer_height = side_pane::composer_height_limit(area.height);
|
||||
let unfocused = matches!(
|
||||
self.conversation_frame.sidebar,
|
||||
Some(sidebar::SidebarLayout::Chooser {
|
||||
Some(sidebar::SidebarLayout::Panel {
|
||||
focus: PaneFocus::Sidebar,
|
||||
..
|
||||
})
|
||||
@@ -874,7 +874,7 @@ impl App {
|
||||
}
|
||||
if self
|
||||
.conversation_frame
|
||||
.chooser_area()
|
||||
.panel_area()
|
||||
.is_some_and(|area| area.contains((mouse.column, mouse.row).into()))
|
||||
{
|
||||
return Ok(true);
|
||||
|
||||
@@ -304,7 +304,7 @@ impl App {
|
||||
&& (self.chat_widget.no_modal_or_popup_active()
|
||||
// Replaying the parent may reopen a completion for its parked draft.
|
||||
// The focused chooser hides it and owns the keystrokes.
|
||||
|| (self.chooser_focus() == Some(sidebar::PaneFocus::Sidebar)
|
||||
|| (self.panel_focus() == Some(sidebar::PaneFocus::Sidebar)
|
||||
&& self.sidebar_chooser_available()
|
||||
&& self.chat_widget.completion_can_yield_focus()))
|
||||
}
|
||||
|
||||
@@ -155,7 +155,7 @@ impl App {
|
||||
if tui.shortcut_hud_contains(*mouse) {
|
||||
return false;
|
||||
}
|
||||
let chooser = frame.chooser_area();
|
||||
let chooser = frame.panel_area();
|
||||
let pane = self.sidebar();
|
||||
if ((matches!(mouse.kind, MouseEventKind::Down(_))
|
||||
&& chooser.is_some_and(|area| area.contains(point)))
|
||||
@@ -212,7 +212,7 @@ impl App {
|
||||
tui.frame_requester().schedule_frame();
|
||||
let Some(area) = self
|
||||
.conversation_frame
|
||||
.chooser_area()
|
||||
.panel_area()
|
||||
.filter(|area| area.contains((mouse.column, mouse.row).into()))
|
||||
else {
|
||||
self.sidebar_mut()
|
||||
@@ -257,7 +257,7 @@ impl App {
|
||||
let Some(list_action) = self.keymap.list.action_for(*key) else {
|
||||
return true;
|
||||
};
|
||||
let area = self.conversation_frame.chooser_area();
|
||||
let area = self.conversation_frame.panel_area();
|
||||
let pane = self.sidebar_mut();
|
||||
if new_chat == sidebar_chooser::NewChatState::WaitingForMainMessage
|
||||
&& list_action != ListAction::Cancel
|
||||
@@ -302,7 +302,7 @@ impl App {
|
||||
// Fast keyboard navigation is safe until an asynchronous update changes its target.
|
||||
if activation.is_some()
|
||||
&& open
|
||||
&& frame.chooser_area().is_some()
|
||||
&& frame.panel_area().is_some()
|
||||
&& self.sidebar().is_some_and(|pane| {
|
||||
let current = pane.input_state();
|
||||
pane.creation.is_none()
|
||||
@@ -449,8 +449,9 @@ impl App {
|
||||
bindings.focus_main_composer.is_pressed(key) && self.main_composer_shortcut_available();
|
||||
let sidebar = bindings.focus_sidebar_content.is_pressed(key);
|
||||
let tabs = bindings.focus_sidebar_tabs.is_pressed(key) && self.sidebar_chooser_available();
|
||||
let panel_focus = self.panel_focus();
|
||||
let hide = bindings.hide_pane.is_pressed(key)
|
||||
|| (self.chooser_focus() == Some(PaneFocus::Sidebar)
|
||||
|| (panel_focus == Some(PaneFocus::Sidebar)
|
||||
&& crate::key_hint::ctrl(KeyCode::Char('c')).is_press(key));
|
||||
let width_change = if bindings.move_divider_left.is_pressed(key) {
|
||||
2
|
||||
@@ -493,9 +494,8 @@ impl App {
|
||||
return false;
|
||||
}
|
||||
let child = self.switchable_side_child();
|
||||
let chooser = self.chooser_focus();
|
||||
if child.is_none()
|
||||
&& ((hide && chooser != Some(PaneFocus::Sidebar))
|
||||
&& ((hide && panel_focus != Some(PaneFocus::Sidebar))
|
||||
|| (sidebar && !self.sidebar_chooser_available()))
|
||||
{
|
||||
return false;
|
||||
@@ -531,8 +531,8 @@ impl App {
|
||||
self.zoomed_thread = (self.zoomed_thread != Some(target)).then_some(target);
|
||||
tui.frame_requester().schedule_frame();
|
||||
Ok(())
|
||||
} else if chooser.is_some() {
|
||||
if hide || (sidebar && chooser == Some(PaneFocus::Sidebar)) {
|
||||
} else if panel_focus.is_some() {
|
||||
if hide || (sidebar && panel_focus == Some(PaneFocus::Sidebar)) {
|
||||
self.focus_current_parent_view(tui);
|
||||
self.sidebar_mut().hide();
|
||||
} else {
|
||||
@@ -544,7 +544,10 @@ impl App {
|
||||
} else {
|
||||
PaneFocus::Sidebar
|
||||
};
|
||||
self.sidebar_mut().show(SidebarContent::Chooser, focus);
|
||||
let pane = self.sidebar_mut();
|
||||
if let Some(content) = pane.visible_content() {
|
||||
pane.show(content, focus);
|
||||
}
|
||||
}
|
||||
tui.frame_requester().schedule_frame();
|
||||
Ok(())
|
||||
@@ -579,7 +582,7 @@ impl App {
|
||||
}
|
||||
let view_changed = leaves_transcript
|
||||
|| self.current_displayed_thread_id() != previous_thread
|
||||
|| ((main || hide) && chooser == Some(PaneFocus::Sidebar));
|
||||
|| ((main || hide) && panel_focus == Some(PaneFocus::Sidebar));
|
||||
let suppression = if view_changed && !direct_tab_switch {
|
||||
ActivationKeySuppression::ViewTransition
|
||||
} else {
|
||||
|
||||
@@ -97,7 +97,7 @@ pub(super) struct SidePaneLayout {
|
||||
pub(super) fn layout(app: &App, tui: &Tui, area: Rect) -> Option<SidePaneLayout> {
|
||||
match visible_split(app, tui, area)? {
|
||||
SidebarLayout::Chat(panes) => Some(panes),
|
||||
SidebarLayout::Chooser { .. } => None,
|
||||
SidebarLayout::Panel { .. } => None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -119,7 +119,7 @@ pub(super) fn split_layout(app: &App, tui: &Tui, area: Rect) -> Option<SidebarLa
|
||||
sidebar::content_layout(
|
||||
app,
|
||||
content,
|
||||
app.chooser_focus().unwrap_or(PaneFocus::Main),
|
||||
app.panel_focus().unwrap_or(PaneFocus::Main),
|
||||
sidebar_areas(app, tui, area, /*fraction*/ 1.0)?,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -286,9 +286,9 @@ impl App {
|
||||
) {
|
||||
let area = if self.sidebar_parent() == Some(parent) && self.chooser_focus().is_some() {
|
||||
let screen: Rect = tui.terminal.last_known_screen_size.into();
|
||||
self.conversation_frame.chooser_area().or_else(|| {
|
||||
self.conversation_frame.panel_area().or_else(|| {
|
||||
match side_pane::split_layout(self, tui, screen) {
|
||||
Some(SidebarLayout::Chooser { sidebar, .. }) => Some(sidebar),
|
||||
Some(SidebarLayout::Panel { sidebar, .. }) => Some(sidebar),
|
||||
_ if screen.width < 100 && self.chooser_focus() == Some(PaneFocus::Sidebar) => {
|
||||
Some(screen)
|
||||
}
|
||||
@@ -408,15 +408,23 @@ impl App {
|
||||
.filter(|_| pane.content == SidebarContent::Chooser)
|
||||
}
|
||||
|
||||
pub(super) fn panel_focus(&self) -> Option<PaneFocus> {
|
||||
let pane = self.sidebar()?;
|
||||
match pane.content {
|
||||
SidebarContent::Chooser => pane.focus,
|
||||
SidebarContent::Chat(_) => None,
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn sidebar_has_focus(&self) -> bool {
|
||||
self.active_side_parent_thread_id().is_some()
|
||||
|| self.chooser_focus() == Some(PaneFocus::Sidebar)
|
||||
|| self.panel_focus() == Some(PaneFocus::Sidebar)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(PartialEq)]
|
||||
pub(super) enum SidebarLayout {
|
||||
Chooser {
|
||||
Panel {
|
||||
main: Rect,
|
||||
sidebar: Rect,
|
||||
focus: PaneFocus,
|
||||
@@ -427,7 +435,7 @@ pub(super) enum SidebarLayout {
|
||||
impl SidebarLayout {
|
||||
pub(super) fn divider_area(&self) -> Option<Rect> {
|
||||
let (x, pane) = match self {
|
||||
Self::Chooser { main, .. } => (main.right(), *main),
|
||||
Self::Panel { main, .. } => (main.right(), *main),
|
||||
Self::Chat(panes) => (panes.divider, panes.focused.union(panes.tab_strip)),
|
||||
};
|
||||
(!pane.is_empty()).then(|| Rect::new(x, pane.y, side_pane::GAP, pane.height))
|
||||
@@ -476,7 +484,7 @@ pub(super) fn content_layout(
|
||||
match content {
|
||||
SidebarContent::Chooser => (app.sidebar_chooser_available()
|
||||
&& (focus == PaneFocus::Main || app.chat_widget.completion_can_yield_focus()))
|
||||
.then_some(SidebarLayout::Chooser {
|
||||
.then_some(SidebarLayout::Panel {
|
||||
main,
|
||||
sidebar,
|
||||
focus,
|
||||
|
||||
@@ -4578,7 +4578,7 @@ async fn handle_start_side_seeds_navigation_before_thread_started() -> Result<()
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n")
|
||||
);
|
||||
let sidebar_x = app.conversation_frame.chooser_area().unwrap().x;
|
||||
let sidebar_x = app.conversation_frame.panel_area().unwrap().x;
|
||||
app.local_settings.tui.animations = true;
|
||||
keys!(KeyCode::Enter.into());
|
||||
assert!(!tui.has_deferred_thread_switch_clear());
|
||||
@@ -5552,7 +5552,7 @@ async fn handle_start_side_seeds_navigation_before_thread_started() -> Result<()
|
||||
|
||||
keys!(prefix, chooser_key, prefix, KeyCode::Char('=').into());
|
||||
side_frame(&mut app, &mut tui, even)?;
|
||||
assert_eq!(app.conversation_frame.chooser_area().unwrap().width, 100);
|
||||
assert_eq!(app.conversation_frame.panel_area().unwrap().width, 100);
|
||||
keys!(prefix, KeyCode::Char('r').into(), KeyCode::Left.into());
|
||||
let frame = side_frame(&mut app, &mut tui, even)?;
|
||||
let (_, hint) = frame
|
||||
@@ -5586,12 +5586,12 @@ async fn handle_start_side_seeds_navigation_before_thread_started() -> Result<()
|
||||
assert_eq!(strip.divider_area(), Some(Rect::new(target, 0, 1, 1)));
|
||||
keys!(prefix, chooser_key);
|
||||
side_frame(&mut app, &mut tui, even)?;
|
||||
assert!(app.conversation_frame.chooser_area().is_some());
|
||||
assert!(app.conversation_frame.panel_area().is_some());
|
||||
assert_eq!(divider_at(&tui, 4), target);
|
||||
|
||||
// Closing from the chooser must not switch to the tab before confirmation.
|
||||
let chooser_focus = app.chooser_focus();
|
||||
let chooser = app.conversation_frame.chooser_area().unwrap();
|
||||
let chooser = app.conversation_frame.panel_area().unwrap();
|
||||
let close_x = (chooser.x..chooser.right())
|
||||
.rfind(|&x| tui.terminal.previous_buffer()[(x, chooser.y)].symbol() == "×")
|
||||
.expect("close button on the second tab");
|
||||
|
||||
@@ -1261,7 +1261,7 @@ async fn sidebar_chooser_resize_survives_startup_thread_assignment() -> Result<(
|
||||
}
|
||||
app.activate_thread_channel(ThreadId::new()).await;
|
||||
let frame = super::side_frame(&mut app, &mut tui, size)?;
|
||||
assert_eq!(app.conversation_frame.chooser_area().unwrap().width, 55);
|
||||
assert_eq!(app.conversation_frame.panel_area().unwrap().width, 55);
|
||||
let (_, footer) = frame.lines().last().unwrap().split_once('│').unwrap();
|
||||
insta::assert_snapshot!(footer.trim(), @"←/→ resize · Enter/Esc done");
|
||||
assert!(
|
||||
@@ -1344,7 +1344,7 @@ async fn sidebar_chooser_preserves_the_parent_draft_and_starts_a_side_chat() ->
|
||||
for (elapsed, width) in [0, 90, 90].into_iter().zip(widths) {
|
||||
tokio::time::advance(std::time::Duration::from_millis(elapsed)).await;
|
||||
super::side_frame(&mut app, &mut tui, size)?;
|
||||
assert_eq!(app.conversation_frame.chooser_area().unwrap().width, width);
|
||||
assert_eq!(app.conversation_frame.panel_area().unwrap().width, width);
|
||||
if key == KeyCode::Esc && elapsed == 90 && width == 20 {
|
||||
let divider = app.conversation_frame.divider_area().unwrap();
|
||||
for kind in [
|
||||
@@ -1405,7 +1405,7 @@ async fn sidebar_chooser_preserves_the_parent_draft_and_starts_a_side_chat() ->
|
||||
// Navigation can request a fresh backend sample between keystrokes.
|
||||
tui.terminal = terminal_at(narrow);
|
||||
super::side_frame(&mut app, &mut tui, narrow)?;
|
||||
assert!(app.conversation_frame.chooser_area().is_none());
|
||||
assert!(app.conversation_frame.panel_area().is_none());
|
||||
let prefix = KeyEvent::from(KeyCode::F(12));
|
||||
assert_eq!(app.route_key_chord_event(&mut tui, prefix), Some(prefix));
|
||||
for code in [KeyCode::End, KeyCode::Char('8'), KeyCode::End] {
|
||||
@@ -1443,7 +1443,7 @@ async fn sidebar_chooser_preserves_the_parent_draft_and_starts_a_side_chat() ->
|
||||
super::side_frame(&mut app, &mut tui, size)?;
|
||||
tokio::time::advance(std::time::Duration::from_millis(90)).await;
|
||||
super::side_frame(&mut app, &mut tui, size)?;
|
||||
assert_eq!(app.conversation_frame.chooser_area().unwrap().width, 20);
|
||||
assert_eq!(app.conversation_frame.panel_area().unwrap().width, 20);
|
||||
}
|
||||
}
|
||||
let first_prompt = crate::chatwidget::UserMessage {
|
||||
|
||||
@@ -25,7 +25,7 @@ impl App {
|
||||
return pending.guide(&self.keymap.chords, |_| true);
|
||||
}
|
||||
let side = self.sidebar_has_focus();
|
||||
let can_switch = self.chooser_focus().is_some() || self.switchable_side_child().is_some();
|
||||
let can_switch = self.panel_focus().is_some() || self.switchable_side_child().is_some();
|
||||
let can_size = super::side_pane::split_layout(self, tui, area).is_some();
|
||||
let can_resize = can_size && side && self.zoomed_thread.is_none();
|
||||
let mut groups = [
|
||||
|
||||
Reference in New Issue
Block a user