Skip to content

Commit 9817b43

Browse files
nornagon-openaiHolovkat
authored andcommitted
tui: refactor ChatWidget and BottomPane to use Renderables (#5565)
- introduce RenderableItem to support both owned and borrowed children in composite Renderables - refactor some of our gnarlier manual layouts, BottomPane and ChatWidget, to use ColumnRenderable - Renderable and friends now handle cursor_pos()
1 parent 9b4e0a8 commit 9817b43

21 files changed

+147
-255
lines changed

codex-rs/tui/src/app.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ use crate::index_worker::IndexWorker;
3030
use crate::memory_manager::run_memory_manager;
3131
use crate::pager_overlay::Overlay;
3232
use crate::render::highlight::highlight_bash_to_lines;
33+
use crate::render::renderable::Renderable;
3334
use crate::resume_picker::ResumeSelection;
3435
use crate::tui;
3536
use crate::tui::TuiEvent;
@@ -1229,7 +1230,7 @@ impl App {
12291230
tui.draw(
12301231
self.chat_widget.desired_height(tui.terminal.size()?.width),
12311232
|frame| {
1232-
frame.render_widget_ref(&self.chat_widget, frame.area());
1233+
self.chat_widget.render(frame.area(), frame.buffer);
12331234
if let Some((x, y)) = self.chat_widget.cursor_pos(frame.area()) {
12341235
frame.set_cursor_position((x, y));
12351236
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -261,10 +261,6 @@ impl BottomPaneView for ApprovalOverlay {
261261
self.enqueue_request(request);
262262
None
263263
}
264-
265-
fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> {
266-
self.list.cursor_pos(area)
267-
}
268264
}
269265

270266
impl Renderable for ApprovalOverlay {
@@ -275,6 +271,10 @@ impl Renderable for ApprovalOverlay {
275271
fn render(&self, area: Rect, buf: &mut Buffer) {
276272
self.list.render(area, buf);
277273
}
274+
275+
fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> {
276+
self.list.cursor_pos(area)
277+
}
278278
}
279279

280280
struct ApprovalRequestState {

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use crate::bottom_pane::ApprovalRequest;
22
use crate::render::renderable::Renderable;
33
use crossterm::event::KeyEvent;
4-
use ratatui::layout::Rect;
54

65
use super::CancellationEvent;
76

@@ -27,11 +26,6 @@ pub(crate) trait BottomPaneView: Renderable {
2726
false
2827
}
2928

30-
/// Cursor position when this view is active.
31-
fn cursor_pos(&self, _area: Rect) -> Option<(u16, u16)> {
32-
None
33-
}
34-
3529
/// Try to handle approval request; return the original value if not
3630
/// consumed.
3731
fn try_consume_approval_request(

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,12 @@ impl BottomPaneView for FeedbackNoteView {
163163
self.textarea.insert_str(&pasted);
164164
true
165165
}
166+
}
167+
168+
impl Renderable for FeedbackNoteView {
169+
fn desired_height(&self, width: u16) -> u16 {
170+
1u16 + self.input_height(width) + 3u16
171+
}
166172

167173
fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> {
168174
if area.height < 2 || area.width <= 2 {
@@ -182,12 +188,6 @@ impl BottomPaneView for FeedbackNoteView {
182188
let state = *self.textarea_state.borrow();
183189
self.textarea.cursor_pos_with_state(textarea_rect, state)
184190
}
185-
}
186-
187-
impl Renderable for FeedbackNoteView {
188-
fn desired_height(&self, width: u16) -> u16 {
189-
1u16 + self.input_height(width) + 3u16
190-
}
191191

192192
fn render(&self, area: Rect, buf: &mut Buffer) {
193193
if area.height == 0 || area.width == 0 {

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

Lines changed: 38 additions & 154 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,16 @@ use std::path::PathBuf;
33

44
use crate::app_event_sender::AppEventSender;
55
use crate::bottom_pane::queued_user_messages::QueuedUserMessages;
6-
use crate::render::Insets;
7-
use crate::render::RectExt;
8-
use crate::render::renderable::Renderable as _;
6+
use crate::render::renderable::FlexRenderable;
7+
use crate::render::renderable::Renderable;
8+
use crate::render::renderable::RenderableItem;
99
use crate::tui::FrameRequester;
1010
use bottom_pane_view::BottomPaneView;
1111
use codex_file_search::FileMatch;
1212
use crossterm::event::KeyCode;
1313
use crossterm::event::KeyEvent;
1414
use ratatui::buffer::Buffer;
15-
use ratatui::layout::Constraint;
16-
use ratatui::layout::Layout;
1715
use ratatui::layout::Rect;
18-
use ratatui::widgets::WidgetRef;
1916
use std::time::Duration;
2017

2118
mod approval_overlay;
@@ -126,77 +123,6 @@ impl BottomPane {
126123
self.request_redraw();
127124
}
128125

129-
pub fn desired_height(&self, width: u16) -> u16 {
130-
let top_margin = 1;
131-
132-
// Base height depends on whether a modal/overlay is active.
133-
let base = match self.active_view().as_ref() {
134-
Some(view) => view.desired_height(width),
135-
None => {
136-
let status_height = self
137-
.status
138-
.as_ref()
139-
.map_or(0, |status| status.desired_height(width));
140-
let queue_height = self.queued_user_messages.desired_height(width);
141-
let spacing_height = if status_height == 0 && queue_height == 0 {
142-
0
143-
} else {
144-
1
145-
};
146-
self.composer
147-
.desired_height(width)
148-
.saturating_add(spacing_height)
149-
.saturating_add(status_height)
150-
.saturating_add(queue_height)
151-
}
152-
};
153-
// Account for bottom padding rows. Top spacing is handled in layout().
154-
base.saturating_add(top_margin)
155-
}
156-
157-
fn layout(&self, area: Rect) -> [Rect; 2] {
158-
// At small heights, bottom pane takes the entire height.
159-
let top_margin = if area.height <= 1 { 0 } else { 1 };
160-
161-
let area = area.inset(Insets::tlbr(top_margin, 0, 0, 0));
162-
if self.active_view().is_some() {
163-
return [Rect::ZERO, area];
164-
}
165-
let has_queue = !self.queued_user_messages.messages.is_empty();
166-
let mut status_height = self
167-
.status
168-
.as_ref()
169-
.map_or(0, |status| status.desired_height(area.width))
170-
.min(area.height.saturating_sub(1));
171-
if has_queue && status_height > 1 {
172-
status_height = status_height.saturating_sub(1);
173-
}
174-
let combined_height = status_height
175-
.saturating_add(self.queued_user_messages.desired_height(area.width))
176-
.min(area.height.saturating_sub(1));
177-
178-
let [status_area, _, content_area] = Layout::vertical([
179-
Constraint::Length(combined_height),
180-
Constraint::Length(if combined_height == 0 { 0 } else { 1 }),
181-
Constraint::Min(1),
182-
])
183-
.areas(area);
184-
[status_area, content_area]
185-
}
186-
187-
pub fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> {
188-
// Hide the cursor whenever an overlay view is active (e.g. the
189-
// status indicator shown while a task is running, or approval modal).
190-
// In these states the textarea is not interactable, so we should not
191-
// show its caret.
192-
let [_, content] = self.layout(area);
193-
if let Some(view) = self.active_view() {
194-
view.cursor_pos(content)
195-
} else {
196-
self.composer.cursor_pos(content)
197-
}
198-
}
199-
200126
/// Forward a key event to the active view or the composer.
201127
pub fn handle_key_event(&mut self, key_event: KeyEvent) -> InputResult {
202128
// If a modal/view is active, handle it here; otherwise forward to composer.
@@ -564,42 +490,39 @@ impl BottomPane {
564490
pub(crate) fn take_recent_submission_images(&mut self) -> Vec<PathBuf> {
565491
self.composer.take_recent_submission_images()
566492
}
567-
}
568-
569-
impl WidgetRef for &BottomPane {
570-
fn render_ref(&self, area: Rect, buf: &mut Buffer) {
571-
let [top_area, content_area] = self.layout(area);
572493

573-
// When a modal view is active, it owns the whole content area.
494+
fn as_renderable(&'_ self) -> RenderableItem<'_> {
574495
if let Some(view) = self.active_view() {
575-
view.render(content_area, buf);
496+
RenderableItem::Borrowed(view)
576497
} else {
577-
let status_height = self
578-
.status
579-
.as_ref()
580-
.map(|status| status.desired_height(top_area.width).min(top_area.height))
581-
.unwrap_or(0);
582-
if let Some(status) = &self.status
583-
&& status_height > 0
584-
{
585-
status.render_ref(top_area, buf);
498+
let mut flex = FlexRenderable::new();
499+
if let Some(status) = &self.status {
500+
flex.push(0, RenderableItem::Borrowed(status));
586501
}
587-
588-
let queue_area = Rect {
589-
x: top_area.x,
590-
y: top_area.y.saturating_add(status_height),
591-
width: top_area.width,
592-
height: top_area.height.saturating_sub(status_height),
593-
};
594-
if queue_area.height > 0 {
595-
self.queued_user_messages.render(queue_area, buf);
502+
flex.push(1, RenderableItem::Borrowed(&self.queued_user_messages));
503+
if self.status.is_some() || !self.queued_user_messages.messages.is_empty() {
504+
flex.push(0, RenderableItem::Owned("".into()));
596505
}
597-
598-
self.composer.render_ref(content_area, buf);
506+
let mut flex2 = FlexRenderable::new();
507+
flex2.push(1, RenderableItem::Owned(flex.into()));
508+
flex2.push(0, RenderableItem::Borrowed(&self.composer));
509+
RenderableItem::Owned(Box::new(flex2))
599510
}
600511
}
601512
}
602513

514+
impl Renderable for BottomPane {
515+
fn render(&self, area: Rect, buf: &mut Buffer) {
516+
self.as_renderable().render(area, buf);
517+
}
518+
fn desired_height(&self, width: u16) -> u16 {
519+
self.as_renderable().desired_height(width)
520+
}
521+
fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> {
522+
self.as_renderable().cursor_pos(area)
523+
}
524+
}
525+
603526
#[cfg(test)]
604527
mod tests {
605528
use super::*;
@@ -623,7 +546,7 @@ mod tests {
623546

624547
fn render_snapshot(pane: &BottomPane, area: Rect) -> String {
625548
let mut buf = Buffer::empty(area);
626-
(&pane).render_ref(area, &mut buf);
549+
pane.render(area, &mut buf);
627550
snapshot_buffer(&buf)
628551
}
629552

@@ -675,7 +598,7 @@ mod tests {
675598
// Render and verify the top row does not include an overlay.
676599
let area = Rect::new(0, 0, 60, 6);
677600
let mut buf = Buffer::empty(area);
678-
(&pane).render_ref(area, &mut buf);
601+
pane.render(area, &mut buf);
679602

680603
let mut r0 = String::new();
681604
for x in 0..area.width {
@@ -689,7 +612,7 @@ mod tests {
689612

690613
#[test]
691614
fn composer_shown_after_denied_while_task_running() {
692-
let (tx_raw, rx) = unbounded_channel::<AppEvent>();
615+
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
693616
let tx = AppEventSender::new(tx_raw);
694617
let mut pane = BottomPane::new(BottomPaneParams {
695618
app_event_tx: tx,
@@ -724,14 +647,14 @@ mod tests {
724647
std::thread::sleep(Duration::from_millis(120));
725648
let area = Rect::new(0, 0, 40, 6);
726649
let mut buf = Buffer::empty(area);
727-
(&pane).render_ref(area, &mut buf);
728-
let mut row1 = String::new();
650+
pane.render(area, &mut buf);
651+
let mut row0 = String::new();
729652
for x in 0..area.width {
730-
row1.push(buf[(x, 1)].symbol().chars().next().unwrap_or(' '));
653+
row0.push(buf[(x, 0)].symbol().chars().next().unwrap_or(' '));
731654
}
732655
assert!(
733-
row1.contains("Working"),
734-
"expected Working header after denial on row 1: {row1:?}"
656+
row0.contains("Working"),
657+
"expected Working header after denial on row 0: {row0:?}"
735658
);
736659

737660
// Composer placeholder should be visible somewhere below.
@@ -750,9 +673,6 @@ mod tests {
750673
found_composer,
751674
"expected composer visible under status line"
752675
);
753-
754-
// Drain the channel to avoid unused warnings.
755-
drop(rx);
756676
}
757677

758678
#[test]
@@ -774,16 +694,10 @@ mod tests {
774694
// Use a height that allows the status line to be visible above the composer.
775695
let area = Rect::new(0, 0, 40, 6);
776696
let mut buf = Buffer::empty(area);
777-
(&pane).render_ref(area, &mut buf);
697+
pane.render(area, &mut buf);
778698

779-
let mut row0 = String::new();
780-
for x in 0..area.width {
781-
row0.push(buf[(x, 1)].symbol().chars().next().unwrap_or(' '));
782-
}
783-
assert!(
784-
row0.contains("Working"),
785-
"expected Working header: {row0:?}"
786-
);
699+
let bufs = snapshot_buffer(&buf);
700+
assert!(bufs.contains("• Working"), "expected Working header");
787701
}
788702

789703
#[test]
@@ -815,36 +729,6 @@ mod tests {
815729
);
816730
}
817731

818-
#[test]
819-
fn status_hidden_when_height_too_small() {
820-
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
821-
let tx = AppEventSender::new(tx_raw);
822-
let mut pane = BottomPane::new(BottomPaneParams {
823-
app_event_tx: tx,
824-
frame_requester: FrameRequester::test_dummy(),
825-
has_input_focus: true,
826-
enhanced_keys_supported: false,
827-
placeholder_text: "Ask Codex to do anything".to_string(),
828-
disable_paste_burst: false,
829-
});
830-
831-
pane.set_task_running(true);
832-
833-
// Height=2 → composer takes the full space; status collapses when there is no room.
834-
let area2 = Rect::new(0, 0, 20, 2);
835-
assert_snapshot!(
836-
"status_hidden_when_height_too_small_height_2",
837-
render_snapshot(&pane, area2)
838-
);
839-
840-
// Height=1 → no padding; single row is the composer (status hidden).
841-
let area1 = Rect::new(0, 0, 20, 1);
842-
assert_snapshot!(
843-
"status_hidden_when_height_too_small_height_1",
844-
render_snapshot(&pane, area1)
845-
);
846-
}
847-
848732
#[test]
849733
fn queued_messages_visible_when_status_hidden_snapshot() {
850734
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();

codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__queued_messages_visible_when_status_hidden_snapshot.snap

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
source: tui/src/bottom_pane/mod.rs
33
expression: "render_snapshot(&pane, area)"
44
---
5-
65
Queued follow-up question
76
+edit
87

codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_and_composer_fill_height_without_bottom_padding.snap

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
source: tui/src/bottom_pane/mod.rs
33
expression: "render_snapshot(&pane, area)"
44
---
5-
65
Working (0sesc to interru
76

87

codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_and_queued_messages_snapshot.snap

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
source: tui/src/bottom_pane/mod.rs
33
expression: "render_snapshot(&pane, area)"
44
---
5-
65
Working (0sesc to interrupt)
76
Queued follow-up question
87
+edit

codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_hidden_when_height_too_small_height_2.snap

Lines changed: 0 additions & 6 deletions
This file was deleted.

0 commit comments

Comments
 (0)