mirror of
https://github.com/openai/codex.git
synced 2026-10-21 09:54:56 +00:00
Keep selection adjacent when hiding tasks in the agents overview (#45978)
## Why Hiding the selected task can reset selection to the current task or first row, interrupting navigation through the list. ## What changed Select the next visible task before hiding the selected task, falling back to the previous task at the end of the list. Follow the active grouping and search filter. Preserve the existing rename cancellation behavior when hiding a rename target so its draft cannot transfer to a neighboring task. ## Testing Add regression tests and a selection snapshot covering project, status, and model grouping, filtered and unfiltered lists, custom hide bindings, hiding all visible tasks, and hiding a task with an unsubmitted rename draft. GitOrigin-RevId: e5ca6b2a9955622bcf60e68621322b3abd801a7f
This commit is contained in:
@@ -108,6 +108,172 @@ async fn lifecycle_shortcuts_target_filtered_task_in_any_state() {
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn hiding_tasks_keeps_selection_adjacent_in_display_order() -> Result<()> {
|
||||
let (mut app, mut rx, _op_rx) = crate::app::tests::make_test_app_with_channels().await;
|
||||
let mut app_server = crate::start_embedded_app_server_for_picker(&app.config).await?;
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
let threads = (1..=4)
|
||||
.map(|index| {
|
||||
let mut thread = overview_thread(
|
||||
ThreadId::from_u128(index),
|
||||
/*parent_thread_id*/ None,
|
||||
if index == 2 { "Other" } else { "Task" },
|
||||
ThreadStatus::Idle,
|
||||
);
|
||||
thread.name = Some(format!("{} {index}", thread.preview));
|
||||
thread.cwd = test_path_buf(&format!("/tmp/project-{index}")).abs();
|
||||
thread.model = Some(format!("model-{index}"));
|
||||
thread.updated_at = index as i64;
|
||||
thread
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
app.primary_thread_id = Some(ThreadId::from_u128(/*value*/ 1));
|
||||
app.agents_overview.threads = threads
|
||||
.iter()
|
||||
.map(|thread| {
|
||||
(
|
||||
ThreadId::from_string(&thread.id).unwrap(),
|
||||
Some(thread.clone()),
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
let mut selections = Vec::new();
|
||||
for grouping in [
|
||||
AgentsOverviewGrouping::Project,
|
||||
AgentsOverviewGrouping::Status,
|
||||
AgentsOverviewGrouping::Model,
|
||||
] {
|
||||
for filtered in [false, true] {
|
||||
app.agents_overview.hidden_threads.clear();
|
||||
app.agents_overview.view_state.lock().unwrap().grouping = grouping;
|
||||
let mut keymap = TuiKeymap::default();
|
||||
let hide_key = if filtered {
|
||||
KeyCode::F(7)
|
||||
} else {
|
||||
KeyCode::Char('h')
|
||||
};
|
||||
if filtered {
|
||||
keymap.agents.hide = Some(KeybindingsSpec::One(KeybindingSpec("f7".into())));
|
||||
}
|
||||
app.keymap = RuntimeKeymap::from_config(&keymap).unwrap();
|
||||
let mut view =
|
||||
app.agents_overview_view(threads.clone(), Some(ThreadId::from_u128(/*value*/ 3)));
|
||||
view.handle_key_event(KeyCode::Esc.into());
|
||||
if filtered {
|
||||
view.handle_key_event(KeyCode::Char('f').into());
|
||||
view.handle_paste("Task".into());
|
||||
// Search selects the first match; move back to Task 3.
|
||||
view.handle_key_event(KeyCode::Down.into());
|
||||
}
|
||||
app.agents_overview.visible_thread_ids = view.thread_ids();
|
||||
app.chat_widget.show_bottom_pane_view(Box::new(view));
|
||||
let expected = match (grouping, filtered) {
|
||||
(AgentsOverviewGrouping::Status, false) => vec![3, 2, 1, 4],
|
||||
(AgentsOverviewGrouping::Status, true) => vec![3, 1, 4],
|
||||
(AgentsOverviewGrouping::Project | AgentsOverviewGrouping::Model, false) => {
|
||||
vec![3, 4, 2, 1]
|
||||
}
|
||||
(AgentsOverviewGrouping::Project | AgentsOverviewGrouping::Model, true) => {
|
||||
vec![3, 4, 1]
|
||||
}
|
||||
};
|
||||
for index in expected {
|
||||
let selected = app
|
||||
.chat_widget
|
||||
.selected_index_for_present_view(AGENTS_OVERVIEW_VIEW_ID)
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
app.agents_overview.visible_thread_ids[selected],
|
||||
ThreadId::from_u128(index),
|
||||
"{grouping:?}, filtered={filtered}"
|
||||
);
|
||||
let rendered = render_bottom_popup(&app.chat_widget, /*width*/ 100);
|
||||
let selected_row = rendered.lines().find(|line| line.contains('›')).unwrap();
|
||||
selections.push(format!(
|
||||
"{grouping:?}, filtered={filtered}: {}",
|
||||
selected_row.split('│').next().unwrap().trim()
|
||||
));
|
||||
app.chat_widget.handle_key_event(hide_key.into());
|
||||
let hide = std::iter::from_fn(|| rx.try_recv().ok())
|
||||
.find(|event| matches!(event, AppEvent::HideAgentsOverviewThread { .. }))
|
||||
.expect("hide shortcut emits an event");
|
||||
assert!(matches!(
|
||||
&hide,
|
||||
AppEvent::HideAgentsOverviewThread { thread_id }
|
||||
if *thread_id == ThreadId::from_u128(index)
|
||||
));
|
||||
Box::pin(app.handle_event(&mut tui, &mut app_server, hide)).await?;
|
||||
}
|
||||
let selected = app
|
||||
.chat_widget
|
||||
.selected_index_for_present_view(AGENTS_OVERVIEW_VIEW_ID)
|
||||
.unwrap();
|
||||
assert_eq!(app.agents_overview.visible_thread_ids.get(selected), None);
|
||||
app.chat_widget.handle_key_event(hide_key.into());
|
||||
assert!(
|
||||
!std::iter::from_fn(|| rx.try_recv().ok())
|
||||
.any(|event| matches!(event, AppEvent::HideAgentsOverviewThread { .. }))
|
||||
);
|
||||
}
|
||||
}
|
||||
insta::assert_snapshot!(selections.join("\n"));
|
||||
app_server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn hiding_rename_target_does_not_transfer_draft_to_neighbor() -> Result<()> {
|
||||
let (mut app, mut rx, _op_rx) = crate::app::tests::make_test_app_with_channels().await;
|
||||
let mut app_server = crate::start_embedded_app_server_for_picker(&app.config).await?;
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
let mut keymap = TuiKeymap::default();
|
||||
keymap.agents.hide = Some(KeybindingsSpec::One(KeybindingSpec("f7".into())));
|
||||
app.keymap = RuntimeKeymap::from_config(&keymap).unwrap();
|
||||
let threads = ["Rename target", "Neighbor"]
|
||||
.into_iter()
|
||||
.map(|name| {
|
||||
overview_thread(
|
||||
ThreadId::new(),
|
||||
/*parent_thread_id*/ None,
|
||||
name,
|
||||
ThreadStatus::Idle,
|
||||
)
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let target = ThreadId::from_string(&threads[0].id)?;
|
||||
app.agents_overview.threads = threads
|
||||
.iter()
|
||||
.map(|thread| {
|
||||
(
|
||||
ThreadId::from_string(&thread.id).unwrap(),
|
||||
Some(thread.clone()),
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
let mut view = app.agents_overview_view(threads, Some(target));
|
||||
view.handle_key_event(KeyCode::Char('r').into());
|
||||
view.handle_paste("Unsubmitted title".into());
|
||||
app.agents_overview.visible_thread_ids = view.thread_ids();
|
||||
app.chat_widget.show_bottom_pane_view(Box::new(view));
|
||||
app.chat_widget.handle_key_event(KeyCode::F(7).into());
|
||||
let hide = std::iter::from_fn(|| rx.try_recv().ok())
|
||||
.find(|event| matches!(event, AppEvent::HideAgentsOverviewThread { .. }))
|
||||
.expect("hide shortcut emits an event");
|
||||
Box::pin(app.handle_event(&mut tui, &mut app_server, hide)).await?;
|
||||
{
|
||||
let state = app.agents_overview.view_state.lock().unwrap();
|
||||
assert_eq!((state.renaming, state.input.as_str()), (false, ""));
|
||||
}
|
||||
app.chat_widget.handle_key_event(KeyCode::Enter.into());
|
||||
assert!(
|
||||
!std::iter::from_fn(|| rx.try_recv().ok())
|
||||
.any(|event| matches!(event, AppEvent::RenameAgentsOverviewThread { .. }))
|
||||
);
|
||||
app_server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn hidden_task_stays_hidden_through_activity_and_seed_until_explicit_resume() -> Result<()> {
|
||||
let (mut app, mut rx, _op_rx) = crate::app::tests::make_test_app_with_channels().await;
|
||||
|
||||
@@ -655,9 +655,19 @@ impl BottomPaneView for AgentsOverviewView {
|
||||
}
|
||||
if self.agents_keymap.hide.is_pressed(key) {
|
||||
if let Some(row) = self.selected_row() {
|
||||
self.app_event_tx.send(AppEvent::HideAgentsOverviewThread {
|
||||
thread_id: row.thread_id,
|
||||
});
|
||||
let thread_id = row.thread_id;
|
||||
let visible = self.visible_indices();
|
||||
if !self.state().renaming
|
||||
&& let Some(position) = visible.iter().position(|index| *index == self.selected)
|
||||
&& let Some(next) = visible
|
||||
.get(position + 1)
|
||||
.or_else(|| visible.get(position.saturating_sub(1)))
|
||||
{
|
||||
// Preserve a neighboring row when hiding rebuilds the view.
|
||||
self.selected = *next;
|
||||
}
|
||||
self.app_event_tx
|
||||
.send(AppEvent::HideAgentsOverviewThread { thread_id });
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
+26
@@ -0,0 +1,26 @@
|
||||
---
|
||||
source: tui/src/app/agents_overview_actions_tests.rs
|
||||
assertion_line: 211
|
||||
expression: "selections.join(\"\\n\")"
|
||||
---
|
||||
Project, filtered=false: › ○ Task 3 Ready
|
||||
Project, filtered=false: › ○ Task 4 Ready
|
||||
Project, filtered=false: › ○ Other 2 Ready
|
||||
Project, filtered=false: › ○ Task 1 current Ready
|
||||
Project, filtered=true: › ○ Task 3 Ready
|
||||
Project, filtered=true: › ○ Task 4 Ready
|
||||
Project, filtered=true: › ○ Task 1 current Ready
|
||||
Status, filtered=false: › ○ Task 3
|
||||
Status, filtered=false: › ○ Other 2
|
||||
Status, filtered=false: › ○ Task 1 current
|
||||
Status, filtered=false: › ○ Task 4
|
||||
Status, filtered=true: › ○ Task 3
|
||||
Status, filtered=true: › ○ Task 1 current
|
||||
Status, filtered=true: › ○ Task 4
|
||||
Model, filtered=false: › ○ Task 3 Ready
|
||||
Model, filtered=false: › ○ Task 4 Ready
|
||||
Model, filtered=false: › ○ Other 2 Ready
|
||||
Model, filtered=false: › ○ Task 1 current Ready
|
||||
Model, filtered=true: › ○ Task 3 Ready
|
||||
Model, filtered=true: › ○ Task 4 Ready
|
||||
Model, filtered=true: › ○ Task 1 current Ready
|
||||
Reference in New Issue
Block a user