diff --git a/crates/ui/src/composer/components/pickers.rs b/crates/ui/src/composer/components/pickers.rs index 3b3315a9..e4ee38b8 100644 --- a/crates/ui/src/composer/components/pickers.rs +++ b/crates/ui/src/composer/components/pickers.rs @@ -231,6 +231,7 @@ impl Composer { ); crate::material::overlay_popover(("model-picker-popover", self.model_picker_token)) + .track_focus(&self.model_search.read(cx).focus_handle(cx)) .anchor(Anchor::BottomLeft) .when(self.compact, |popover| { popover.bottom_sheet(crate::tr!("mobile.model")) @@ -729,6 +730,7 @@ fn render_model_pane( let popover_key = popover.clone(); let pane = h_flex() + .key_context("ModelPicker") .when(!composer.read(cx).compact, |pane| pane.w(px(360.))) .when(composer.read(cx).compact, |pane| pane.w_full()) .h(px(360.)) diff --git a/crates/ui/src/run.rs b/crates/ui/src/run.rs index bba3a324..59bc0de4 100644 --- a/crates/ui/src/run.rs +++ b/crates/ui/src/run.rs @@ -127,6 +127,7 @@ pub fn run_shell( .expect("failed to register bundled application fonts"); theme::init_with_json(&options.theme_json, cx); crate::markdown::init(cx); + crate::shortcut::init(cx); // Global ⌘K / Ctrl-K opens/closes the command palette (handled by // AppShell). `secondary` is gpui's platform modifier: command on macOS, // control on Windows/Linux — where a literal `cmd-` binding would mean the diff --git a/crates/ui/src/shell.rs b/crates/ui/src/shell.rs index b4354e27..4f7ff1aa 100644 --- a/crates/ui/src/shell.rs +++ b/crates/ui/src/shell.rs @@ -995,6 +995,25 @@ fn current_shell(cx: &App) -> Option> { cx.try_global::()?.shell.upgrade() } +/// App-level dispatch also reaches the shell when no child has keyboard focus. +pub(crate) fn navigate_thread(action: &crate::shortcut::NavigateThread, cx: &mut App) { + let Some(shell) = current_shell(cx) else { + return; + }; + shell.update(cx, |shell, cx| { + if let Some(attachment) = &shell.attachment + && attachment + .sidebar + .update(cx, |sidebar, cx| sidebar.navigate_thread(action, cx)) + { + shell.window_state.update(cx, |state, cx| { + state.close_palette(cx); + state.go(Destination::Thread, cx); + }); + } + }); +} + /// Point this window at another host. pub(crate) fn switch_current(target: AttachmentTarget, window: &mut Window, cx: &mut App) { let Some(shell) = current_shell(cx) else { @@ -3626,6 +3645,127 @@ mod tests { shell.read_with(cx, |shell, _| shell.store().expect("attached")) } + #[gpui::test] + fn thread_shortcuts_follow_list_order_and_leave_model_picker_numbers_alone( + cx: &mut TestAppContext, + ) { + cx.update(crate::theme::init); + cx.update(crate::shortcut::init); + let root = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("../../tmp") + .join(format!( + "thread-shortcuts-{}", + tcode_services::store::now_millis() + )); + let disk = tcode_services::store::SessionStore::open_at(root.clone()).unwrap(); + let project = tcode_core::project::Project::from_root(root.join("project")); + let sessions = [("third", 10), ("first", 30), ("second", 20)] + .into_iter() + .map(|(id, updated_at)| { + let mut meta = tcode_core::project::SessionMeta::new( + agent::ProviderKind::Codex, + project.root.clone(), + None, + ); + meta.id = id.into(); + meta.project_id = Some(project.id.clone()); + meta.updated_at = updated_at; + meta + }) + .collect(); + let host = + tcode_runtime::pipe::spawn_host(disk, tcode_runtime::pipe::HostServices::default()) + .unwrap(); + smol::block_on(host.update_state_for_test(move |state, _| { + state.projects = vec![project]; + state.sessions = sessions; + state.settings.auto_archive_disabled = true; + })) + .unwrap(); + let (shell, _transport, cx) = mount(cx); + cx.update(|window, cx| set_back_target(window.window_handle(), &shell, cx)); + let store = store_of(&shell, cx); + store.update(cx, |store, cx| { + *store = WorkspaceStore::new(host.link(), cx); + store.select_session("first".into()); + }); + await_restore_update(&shell, cx, |store| !store.chat_loading()); + resize(cx, 1200.); + draw(cx); + + let [first_key, second_key, out_of_range_key] = if cfg!(target_os = "macos") { + ["cmd-1", "cmd-2", "cmd-9"] + } else { + ["ctrl-1", "ctrl-2", "ctrl-9"] + }; + let picker = cx.debug_bounds("model-picker").expect("model picker"); + cx.simulate_click(picker.center(), gpui::Modifiers::default()); + draw(cx); + cx.simulate_keystrokes(second_key); + draw(cx); + assert_eq!( + store + .read_with(cx, |store, _| store.active_session_id()) + .as_deref(), + Some("first"), + "model picker must keep number keys" + ); + cx.simulate_keystrokes("escape"); + draw(cx); + + for (keys, expected) in [ + (second_key, "second"), + ("ctrl-tab", "third"), + ("ctrl-tab", "first"), + ("ctrl-shift-tab", "third"), + (out_of_range_key, "third"), + (first_key, "first"), + ] { + cx.simulate_keystrokes(keys); + await_restore_update(&shell, cx, |store| !store.chat_loading()); + assert_eq!( + store + .read_with(cx, |store, _| store.active_session_id()) + .as_deref(), + Some(expected), + "{keys}" + ); + } + assert_eq!( + store.read_with(cx, |store, _| store.composer_state().interaction_mode), + agent::InteractionMode::Build, + "Ctrl+Shift+Tab must not toggle the composer's mode" + ); + cx.update(|window, cx| window.blur(cx)); + draw(cx); + cx.simulate_keystrokes(second_key); + assert_eq!( + store + .read_with(cx, |store, _| store.active_session_id()) + .as_deref(), + Some("second"), + "shortcuts work without a focused input" + ); + shell.update(cx, |shell, cx| shell.go(Destination::Settings, cx)); + draw(cx); + cx.simulate_keystrokes(first_key); + assert_eq!( + shell.read_with(cx, |shell, cx| shell.destination(cx)), + Destination::Thread + ); + resize(cx, 393.); + cx.simulate_keystrokes("ctrl-tab"); + assert_eq!( + store + .read_with(cx, |store, _| store.active_session_id()) + .as_deref(), + Some("second"), + "compact navigation uses the same shortcuts" + ); + host.shutdown_blocking().unwrap(); + std::fs::remove_dir_all(root).unwrap(); + } + /// Resizing a window is a layout decision and nothing else: it must not /// touch the attachment, the selected thread, the draft or the split. #[gpui::test] diff --git a/crates/ui/src/shortcut.rs b/crates/ui/src/shortcut.rs index d6542064..5592617c 100644 --- a/crates/ui/src/shortcut.rs +++ b/crates/ui/src/shortcut.rs @@ -1,5 +1,29 @@ use crate::widgets::kbd::Kbd; -use gpui::{Keystroke, Modifiers}; +use gpui::{Action, App, KeyBinding, Keystroke, Modifiers, NoAction}; +use serde::Deserialize; + +#[derive(Action, Clone, PartialEq, Eq, Deserialize)] +#[action(namespace = tcode, no_json)] +pub(crate) enum NavigateThread { + Index(usize), + Next, + Previous, +} + +pub(crate) fn init(cx: &mut App) { + cx.on_action(crate::shell::navigate_thread); + for number in 1..=9 { + let key = format!("secondary-{number}"); + cx.bind_keys([ + KeyBinding::new(&key, NavigateThread::Index(number - 1), None), + KeyBinding::new(&key, NoAction, Some("ModelPicker || ModelPicker > Input")), + ]); + } + cx.bind_keys([ + KeyBinding::new("ctrl-tab", NavigateThread::Next, None), + KeyBinding::new("ctrl-shift-tab", NavigateThread::Previous, None), + ]); +} /// Format a shortcut using GPUI's semantic secondary modifier. /// diff --git a/crates/ui/src/sidebar.rs b/crates/ui/src/sidebar.rs index da598cb7..10c043df 100644 --- a/crates/ui/src/sidebar.rs +++ b/crates/ui/src/sidebar.rs @@ -625,7 +625,178 @@ pub struct SessionsSidebar { _subscriptions: Vec, } +/// Included desktop rows plus the pre-disclosure counts used by list controls. +struct ThreadRows<'a> { + active: Vec<&'a SessionMeta>, + settled: Vec<&'a SessionMeta>, + active_count: usize, + settled_count: usize, +} + +fn session_flags(sessions: &[SessionMeta], store: &WorkspaceStore) -> HashMap { + sessions + .iter() + .map(|meta| { + ( + meta.id.clone(), + ThreadFlags { + unread: store.session_unread(&meta.id), + waiting_for_approval: store.pending_approval_for(&meta.id), + waiting_for_input: store.pending_user_input_for(&meta.id), + working: store.turn_running_for(&meta.id), + background: store.background_only_for(&meta.id), + }, + ) + }) + .collect() +} + impl SessionsSidebar { + fn flat_thread_rows<'a>( + &self, + active: &'a [SessionMeta], + settled: &'a [SessionMeta], + flags: &HashMap, + ) -> ThreadRows<'a> { + let active = flat_visible_threads( + active, + &self.collapsed_parents, + self.project_filter.as_deref(), + flags, + ); + let mut settled = flat_visible_threads( + settled, + &self.collapsed_parents, + self.project_filter.as_deref(), + flags, + ); + let active_count = active.len(); + let settled_count = settled.len(); + if !self.expanded_settled.contains("recent") { + settled.clear(); + } + ThreadRows { + active, + settled, + active_count, + settled_count, + } + } + + fn group_thread_rows<'a>( + &self, + project_id: &str, + active: &'a [SessionMeta], + settled: &'a [SessionMeta], + collapsed: bool, + ) -> ThreadRows<'a> { + let mut active = visible_threads(active, &self.collapsed_parents); + let active_count = active.len(); + let settled_count = settled.len(); + if collapsed { + active.clear(); + } else if !self.expanded_groups.contains(project_id) { + active.truncate(THREADS_COLLAPSED_LIMIT); + } + let settled = if !collapsed && self.expanded_settled.contains(project_id) { + visible_threads(settled, &self.collapsed_parents) + } else { + Vec::new() + }; + ThreadRows { + active, + settled, + active_count, + settled_count, + } + } + + /// Use the same filters, disclosures and ordering as the rendered list, + /// including rows outside the scroll viewport. + fn navigation_threads(&mut self, cx: &mut Context) -> Vec { + if self.compact(cx) { + return self + .compact_model(cx) + .rows + .iter() + .filter_map(|row| match row { + CompactListRow::Thread(row) => Some(row.meta.id.clone()), + _ => None, + }) + .collect(); + } + + let store = self.store.read(cx); + let mut ids = Vec::new(); + match store.sidebar_layout() { + SidebarLayout::Flat => { + let sessions = store.flat_sessions(); + let flags = session_flags(&sessions, store); + let (active, settled) = partition_settled(&sessions); + let rows = self.flat_thread_rows(&active, &settled, &flags); + ids.extend( + rows.active + .into_iter() + .chain(rows.settled) + .map(|meta| meta.id.clone()), + ); + } + SidebarLayout::Grouped => { + for group in store.grouped_sessions() { + let project_id = &group.project.id; + let (active, settled) = partition_settled(&group.sessions); + let rows = self.group_thread_rows( + project_id, + &active, + &settled, + store.is_project_collapsed(project_id), + ); + ids.extend( + rows.active + .into_iter() + .chain(rows.settled) + .map(|meta| meta.id.clone()), + ); + } + } + } + ids + } + + pub(crate) fn navigate_thread( + &mut self, + action: &crate::shortcut::NavigateThread, + cx: &mut Context, + ) -> bool { + use crate::shortcut::NavigateThread; + + let threads = self.navigation_threads(cx); + if threads.is_empty() { + return false; + } + let active = self.store.read(cx).active_session_id(); + let current = threads.iter().position(|id| Some(id) == active.as_ref()); + let index = match action { + NavigateThread::Index(index) => *index, + NavigateThread::Next => current.map_or(0, |index| (index + 1) % threads.len()), + NavigateThread::Previous => current.map_or(threads.len() - 1, |index| { + (index + threads.len() - 1) % threads.len() + }), + }; + if let Some(id) = threads.get(index) { + self.compact_model_dirty = true; + self.store.update(cx, |store, cx| { + store.select_session(id.clone()); + cx.notify(); + }); + self.window_state + .update(cx, |state, cx| state.open_thread(cx)); + cx.notify(); + return true; + } + false + } + fn compact(&self, cx: &gpui::App) -> bool { self.window_state.read(cx).compact } @@ -1673,13 +1844,8 @@ impl SessionsSidebar { let expanded = self.expanded_groups.contains(&project_id); let (active, settled) = partition_settled(&group.sessions); - let threads = visible_threads(&active, &self.collapsed_parents); - let total = threads.len(); - let visible = if expanded { - total - } else { - total.min(THREADS_COLLAPSED_LIMIT) - }; + let rows = self.group_thread_rows(&project_id, &active, &settled, collapsed); + let total = rows.active_count; let header_toggle_id = project_id.clone(); let plus_cwd = group.project.root.clone(); @@ -1811,7 +1977,7 @@ impl SessionsSidebar { ); if !collapsed { - for meta in threads.iter().take(visible).copied() { + for meta in rows.active { let is_active = active_id == Some(meta.id.as_str()); // "Working" covers parked sessions too — a thread that keeps // running in the background keeps its green dot. @@ -1876,19 +2042,20 @@ impl SessionsSidebar { .child(label), ); } - if !settled.is_empty() { - container = - container.child(self.render_settled_header(&project_id, settled.len(), cx)); - if self.expanded_settled.contains(&project_id) { - for meta in visible_threads(&settled, &self.collapsed_parents) { - container = container.child(self.render_thread( - meta, - sessions, - flags, - active_id == Some(meta.id.as_str()), - cx, - )); - } + if rows.settled_count > 0 { + container = container.child(self.render_settled_header( + &project_id, + rows.settled_count, + cx, + )); + for meta in rows.settled { + container = container.child(self.render_thread( + meta, + sessions, + flags, + active_id == Some(meta.id.as_str()), + cx, + )); } } } @@ -2653,21 +2820,7 @@ impl SessionsSidebar { .into_iter() .filter(|meta| meta.archived_at.is_none()) .collect::>(); - let flags = sessions - .iter() - .map(|meta| { - ( - meta.id.clone(), - ThreadFlags { - unread: store.session_unread(&meta.id), - waiting_for_approval: store.pending_approval_for(&meta.id), - waiting_for_input: store.pending_user_input_for(&meta.id), - working: store.turn_running_for(&meta.id), - background: store.background_only_for(&meta.id), - }, - ) - }) - .collect::>(); + let flags = session_flags(&sessions, store); let groups = store.grouped_sessions(); let collapsed = groups .iter() @@ -3338,21 +3491,7 @@ impl Render for SessionsSidebar { ) = { let store = self.store.read(cx); let sessions = store.sidebar_sessions(); - let flags = sessions - .iter() - .map(|meta| { - ( - meta.id.clone(), - ThreadFlags { - unread: store.session_unread(&meta.id), - waiting_for_approval: store.pending_approval_for(&meta.id), - waiting_for_input: store.pending_user_input_for(&meta.id), - working: store.turn_running_for(&meta.id), - background: store.background_only_for(&meta.id), - }, - ) - }) - .collect::>(); + let flags = session_flags(&sessions, store); let groups = store.grouped_sessions(); let collapsed_projects = groups .iter() @@ -3408,19 +3547,12 @@ impl Render for SessionsSidebar { } SidebarLayout::Flat => { let (active, settled) = partition_settled(&flat_sessions); - let visible = flat_visible_threads( - &active, - &self.collapsed_parents, - self.project_filter.as_deref(), - &flags, - ); - let settled_visible = flat_visible_threads( - &settled, - &self.collapsed_parents, - self.project_filter.as_deref(), - &flags, - ); - let settled_count = settled_visible.len(); + let ThreadRows { + active: visible, + settled: settled_visible, + settled_count, + .. + } = self.flat_thread_rows(&active, &settled, &flags); if visible.is_empty() && settled_count == 0 { // An active project filter can empty the list while threads // exist; that state gets its own hint, not the no-projects one. @@ -3475,16 +3607,14 @@ impl Render for SessionsSidebar { .collect::>(); if settled_count > 0 { visible.push(None); - if self.expanded_settled.contains("recent") { - let offsets = flat_thread_top_offsets(&settled_visible, &flat_sessions); - visible.extend( - settled_visible - .into_iter() - .cloned() - .zip(offsets.into_iter().map(|offset| offset + settled_top)) - .map(Some), - ); - } + let offsets = flat_thread_top_offsets(&settled_visible, &flat_sessions); + visible.extend( + settled_visible + .into_iter() + .cloned() + .zip(offsets.into_iter().map(|offset| offset + settled_top)) + .map(Some), + ); } if self.flat_list_state.item_count() != visible.len() { self.flat_list_state.reset(visible.len()); @@ -3687,7 +3817,7 @@ mod tests { } #[gpui::test] - fn settled_groups_collapse_and_navigation_reveals_them_at_both_widths(cx: &mut TestAppContext) { + fn thread_navigation_matches_displayed_rows_and_disclosures(cx: &mut TestAppContext) { use tcode_protocol::{ EventEnvelope, HostMessage, IndexSnapshot, ServerEvent, Topic, encode_line, }; @@ -3784,6 +3914,10 @@ mod tests { "sidebar-thread-settled" }; assert!(cx.debug_bounds(row).is_none(), "settled starts collapsed"); + assert_eq!( + sidebar.update(cx, |sidebar, cx| sidebar.navigation_threads(cx)), + ["active"] + ); let header = cx.debug_bounds(key).unwrap(); cx.simulate_click(header.center(), gpui::Modifiers::default()); draw(cx); @@ -3791,6 +3925,10 @@ mod tests { cx.debug_bounds(row).is_some(), "expansion exposes settled thread" ); + assert_eq!( + sidebar.update(cx, |sidebar, cx| sidebar.navigation_threads(cx)), + ["active", "settled"] + ); let header = cx.debug_bounds(key).unwrap(); cx.simulate_click(header.center(), gpui::Modifiers::default()); draw(cx); @@ -3813,6 +3951,125 @@ mod tests { })); } } + + // Exercise desktop filtering and the grouped six-row disclosure against + // actual rendered positions, independently of the shortcut row helpers. + let ids = [ + "one", "two", "three", "four", "five", "six", "seven", "child", + ]; + let sessions = ids + .iter() + .enumerate() + .rev() + .map(|(index, id)| { + let parent = if *id == "child" { Some("one") } else { None }; + let mut meta = session(id, parent); + meta.project_id = Some("project".into()); + meta.updated_at = 100 - index as u64; + meta + }) + .collect(); + send( + Topic::Index, + ServerEvent::IndexSnapshot(IndexSnapshot { + sessions, + projects: store.read_with(cx, |store, _| store.projects()), + activity: HashMap::new(), + }), + ); + window_state.update(cx, |state, _| state.compact = false); + store.update(cx, |store, _| store.select_session("one".into())); + let cases = [ + (SidebarLayout::Grouped, false, false, true, None, &ids[..6]), + (SidebarLayout::Grouped, true, false, true, None, &ids[..7]), + (SidebarLayout::Grouped, true, true, true, None, &[]), + ( + SidebarLayout::Flat, + false, + false, + true, + Some("missing"), + &[], + ), + ( + SidebarLayout::Flat, + false, + false, + true, + Some("project"), + &ids[..7], + ), + ( + SidebarLayout::Flat, + false, + false, + false, + Some("project"), + &[ + "one", "child", "two", "three", "four", "five", "six", "seven", + ], + ), + ]; + for (layout, expanded, collapsed, children_collapsed, filter, expected) in cases { + send( + Topic::Settings, + ServerEvent::SettingsSnapshot(tcode_core::settings::Settings { + sidebar_layout: layout, + collapsed_projects: if collapsed { + vec!["project".into()] + } else { + Vec::new() + }, + auto_archive_disabled: true, + ..Default::default() + }), + ); + sidebar.update(cx, |sidebar, cx| { + sidebar.expanded_groups.clear(); + if expanded { + sidebar.expanded_groups.insert("project".into()); + } + sidebar.collapsed_parents.clear(); + if children_collapsed { + sidebar.collapsed_parents.insert("one".into()); + } + sidebar.project_filter = filter.map(str::to_string); + cx.notify(); + }); + draw(cx); + store.update(cx, |store, cx| store.drain_host_events_for_test(cx)); + draw(cx); + cx.executor() + .advance_clock(std::time::Duration::from_secs(1)); + draw(cx); + let mut rendered = ids + .iter() + .zip([ + "sidebar-thread-one", + "sidebar-thread-two", + "sidebar-thread-three", + "sidebar-thread-four", + "sidebar-thread-five", + "sidebar-thread-six", + "sidebar-thread-seven", + "sidebar-thread-child", + ]) + .filter_map(|(id, selector)| { + cx.debug_bounds(selector).map(|bounds| (*id, bounds.top())) + }) + .collect::>(); + rendered.sort_by(|a, b| a.1.partial_cmp(&b.1).unwrap()); + assert_eq!( + rendered.iter().map(|(id, _)| *id).collect::>(), + expected, + "displayed rows for {layout:?}" + ); + assert_eq!( + sidebar.update(cx, |sidebar, cx| sidebar.navigation_threads(cx)), + expected, + "shortcut order for {layout:?}" + ); + } } #[test] diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 7eeef9bb..2c73f8a7 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -193,6 +193,18 @@ the attachment's store and survives layout changes. ## Opening a conversation +Command+1 through Command+9 on macOS, or Ctrl+1 through Ctrl+9 elsewhere, open +the corresponding thread in the current thread-list order. Ctrl+Tab opens the +next thread and Ctrl+Shift+Tab opens the previous one, +wrapping at either end. Navigation follows the current layout, sort, project +filter and expanded groups, including rows outside the scroll viewport but +excluding folded-away threads. Rendering and shortcuts use the same included +thread rows; group headings and disclosure controls do not consume shortcut +positions. A number beyond the list length does nothing; +with no listed thread selected, next starts at the first and previous at the +last. Tab navigation uses Control on every platform. While the model picker is +open, number shortcuts remain with the picker. + The first tap selects the sidebar row and pushes the compact Thread destination immediately. Until both status and timeline arrive, the chat shows a muted message skeleton. Selecting that conversation again sends no requests. Changing selection