From 729f9657a330d14740a93eeb9c1f7b0777343a47 Mon Sep 17 00:00:00 2001 From: Franz Hemmer Date: Fri, 6 Nov 2026 19:51:33 -0500 Subject: [PATCH 1/2] fix: keep the selection on the saved item after a reorder (#91) Manage > Work items and Manage > Projects kept the selection as a row number. A priority or rank save re-sorted the list under it, so after Back the highlight, the scroll and Enter went to whichever item now held the old row. The selection now follows the item's identity (work item id, project slug) through the Work items reload and the Projects upsert and reload. When the item is no longer listed, the old position is kept within the list; an empty list selects nothing. Filter changes still start at the first row. The identity rule is dashboard_core::selection::reselect. Co-Authored-By: Claude Opus 5.5 --- crates/dashboard-app/src/captures.rs | 191 +++++++++++++++ crates/dashboard-app/src/manage/projects.rs | 48 +++- crates/dashboard-app/src/manage/work_items.rs | 13 +- crates/dashboard-app/src/projects_tests.rs | 165 +++++++++++++ crates/dashboard-app/src/work_items_tests.rs | 217 +++++++++++++++++- crates/dashboard-core/src/lib.rs | 1 + crates/dashboard-core/src/selection.rs | 53 +++++ 7 files changed, 675 insertions(+), 13 deletions(-) create mode 100644 crates/dashboard-core/src/selection.rs diff --git a/crates/dashboard-app/src/captures.rs b/crates/dashboard-app/src/captures.rs index ee4f8a8..27fee91 100644 --- a/crates/dashboard-app/src/captures.rs +++ b/crates/dashboard-app/src/captures.rs @@ -1482,3 +1482,194 @@ fn captures_fleet_runtime_ages() { } } } + +fn has_focus(cx: &mut HeadlessAppContext, handle: AnyWindowHandle, id: &'static str) -> bool { + cx.update_window(handle, |_, window, _| window.find(id).focused() == Some(true)) + .unwrap() +} + +/// Presses `step` (Tab or Shift+Tab) until `id` has focus. +fn tab_to(cx: &mut HeadlessAppContext, handle: AnyWindowHandle, step: &str, id: &'static str) { + for _ in 0..40 { + if has_focus(cx, handle, id) { + return; + } + key(cx, handle, &[step]); + } + panic!("{id} never took focus"); +} + +/// Opens a Manage dialog on `write` at the default window size and theme. +fn open_manage_dialog( + cx: &mut HeadlessAppContext, + write: Arc, + config_path: Option, + feature: dashboard_core::write::Feature, +) -> (AnyWindowHandle, Entity) { + let (handle, view) = open_with( + cx, + demo::checkouts(), + 1100., + 760., + Options { + write, + config_path: config_path.or(options().config_path), + ..options() + }, + ); + fill(cx, &view, &demo::DemoSource); + let manage = cx.update(|cx| view.read(cx).manage().clone()); + settle(cx); + cx.update_window(handle, |_, window, cx| { + manage.update(cx, |m, cx| { + m.open_menu(window, cx); + m.activate(crate::manage::Entry::Feature(feature), window, cx); + }) + }) + .unwrap(); + settle(cx); + (handle, manage) +} + +/// Issue 91, Work items: a priority save that moves the item keeps it selected. Keyboard only, from a pmd that lists in +/// its own order. Each state is saved, then checked, so the old code still writes the state it gets wrong. +#[test] +#[ignore = "writes issue 91 Work items selection states offscreen"] +fn captures_work_selection_after_reorder() { + use crate::work_items_tests::{FakeWork, sample_items}; + use std::sync::atomic::Ordering; + let out = PathBuf::from(std::env::var_os("CAPTURE_DIR").expect("CAPTURE_DIR")); + std::fs::create_dir_all(&out).unwrap(); + let mut cx = headless(); + let backend = FakeWork::new(&["read", "write:work"], sample_items()); + backend.in_pmd_order.store(true, Ordering::SeqCst); + let (handle, manage) = open_manage_dialog( + &mut cx, + backend.clone(), + None, + dashboard_core::write::Feature::WorkItems, + ); + let numbers = |cx: &mut HeadlessAppContext| { + cx.update(|cx| { + let work = manage.read(cx).work_items().unwrap(); + let numbers: Vec = work.items().iter().map(|item| item.number).collect(); + (numbers, work.selected()) + }) + }; + let editing = |cx: &mut HeadlessAppContext| { + cx.update(|cx| { + manage + .read(cx) + .work_items() + .unwrap() + .editor_item() + .map(|item| item.number) + }) + }; + + // Down to the codexbar PR, #202, fifth in pmd's order. + key(&mut cx, handle, &["down", "down", "down", "down"]); + save(&mut cx, handle, &out, "selection-work-1-before"); + assert_eq!(numbers(&mut cx), (vec![203, 205, 200, 201, 202, 204], 4)); + + // Enter opens it; Tab to P0, Enter picks it, Ctrl+Enter saves it. + key(&mut cx, handle, &["enter"]); + assert_eq!(editing(&mut cx), Some(202)); + tab_to(&mut cx, handle, "tab", "work-priority-0"); + key(&mut cx, handle, &["enter", "ctrl-enter"]); + settle(&mut cx); + save(&mut cx, handle, &out, "selection-work-2-saved"); + let writes = backend.writes.lock().unwrap().clone(); + assert_eq!(writes.len(), 1); + assert_eq!(writes[0].path, "v1/work/01JBXWORK0002"); + assert_eq!(writes[0].body, serde_json::json!({"priority": 0})); + + // Tab to Back to list, Enter: the highlight is on #202, now first. + tab_to(&mut cx, handle, "tab", "work-back"); + key(&mut cx, handle, &["enter"]); + save(&mut cx, handle, &out, "selection-work-3-after-back"); + assert!(has_focus(&mut cx, handle, "work-list")); + assert_eq!(numbers(&mut cx), (vec![202, 203, 205, 200, 201, 204], 0)); + + // Enter reopens #202. + key(&mut cx, handle, &["enter"]); + save(&mut cx, handle, &out, "selection-work-4-enter"); + assert_eq!(editing(&mut cx), Some(202)); +} + +/// Issue 91, Projects: a priority save that re-sorts the project keeps it selected. Keyboard only; each state is saved, +/// then checked. +#[test] +#[ignore = "writes issue 91 Projects selection states offscreen"] +fn captures_projects_selection_after_reorder() { + use crate::projects_tests::{FakeProjects, Folder}; + let out = PathBuf::from(std::env::var_os("CAPTURE_DIR").expect("CAPTURE_DIR")); + std::fs::create_dir_all(&out).unwrap(); + let mut cx = headless(); + let folder = Folder::new(); + let backend = FakeProjects::new(&["read", "write:projects"]); + let (handle, manage) = open_manage_dialog( + &mut cx, + backend.clone(), + Some(folder.config_dir().join("config.toml")), + dashboard_core::write::Feature::Projects, + ); + let listed = |cx: &mut HeadlessAppContext| { + cx.update(|cx| { + let projects = manage.read(cx).projects().unwrap(); + (projects.visible_slugs(cx), projects.selected()) + }) + }; + let editing = |cx: &mut HeadlessAppContext| { + cx.update(|cx| { + manage + .read(cx) + .projects() + .unwrap() + .editor_form() + .map(|form| form.value("slug").to_owned()) + }) + }; + let slugs = |slugs: &[&str]| slugs.iter().map(|slug| (*slug).to_owned()).collect::>(); + + // Down to gh-x, P3. + key(&mut cx, handle, &["down", "down", "down"]); + save(&mut cx, handle, &out, "selection-projects-1-before"); + assert_eq!( + listed(&mut cx), + ( + slugs(&["project-dashboard", "set-it-free-loop", "hs-buddy", "gh-x", "chief"]), + 3 + ) + ); + + // Enter edits it with focus on P3; Shift+Tab to P0, Enter picks it, Ctrl+Enter saves it. + key(&mut cx, handle, &["enter"]); + assert_eq!(editing(&mut cx).as_deref(), Some("gh-x")); + tab_to(&mut cx, handle, "shift-tab", "project-priority-0"); + key(&mut cx, handle, &["enter", "ctrl-enter"]); + settle(&mut cx); + save(&mut cx, handle, &out, "selection-projects-2-saved"); + let writes = backend.writes.lock().unwrap().clone(); + assert_eq!(writes.len(), 1); + assert_eq!(writes[0].path, "v1/projects/gh-x"); + assert_eq!(writes[0].body["priority"], serde_json::json!(0)); + + // Tab to Back to list, Enter: the highlight is on gh-x, now first. + tab_to(&mut cx, handle, "tab", "projects-back"); + key(&mut cx, handle, &["enter"]); + save(&mut cx, handle, &out, "selection-projects-3-after-back"); + assert!(has_focus(&mut cx, handle, "projects-list")); + assert_eq!( + listed(&mut cx), + ( + slugs(&["gh-x", "project-dashboard", "set-it-free-loop", "hs-buddy", "chief"]), + 0 + ) + ); + + // Enter reopens gh-x. + key(&mut cx, handle, &["enter"]); + save(&mut cx, handle, &out, "selection-projects-4-enter"); + assert_eq!(editing(&mut cx).as_deref(), Some("gh-x")); +} diff --git a/crates/dashboard-app/src/manage/projects.rs b/crates/dashboard-app/src/manage/projects.rs index e6d0f04..e884af7 100644 --- a/crates/dashboard-app/src/manage/projects.rs +++ b/crates/dashboard-app/src/manage/projects.rs @@ -13,6 +13,7 @@ use std::sync::Arc; use dashboard_core::config::Config; use dashboard_core::edit::{Form, Next}; use dashboard_core::projects::{self, FIELDS, Project}; +use dashboard_core::selection; use dashboard_core::write::{CreateCheck, Feature, Method, WriteError, WriteRequest, write_state}; use gpui_kit::component::input::{Input, InputState}; use gpui_kit::component::{ActiveTheme as _, Icon, IconName, Sizable as _, StyledExt as _, h_flex, v_flex}; @@ -41,14 +42,16 @@ enum After { } /// Puts a project pmd returned in the list, in order. A project the list holds at a newer version stays: a late -/// answer never undoes a later write. -fn upsert(projects: &mut ProjectsDialog, project: Project) { - match projects.rows.iter_mut().find(|row| row.slug == project.slug) { - Some(row) if row.version > project.version => {} - Some(row) => *row = project, - None => projects.rows.push(project), - } - projects::sort(&mut projects.rows); +/// answer never undoes a later write. The selection stays on its project. +fn upsert(projects: &mut ProjectsDialog, project: Project, cx: &gpui_kit::App) { + projects.keep_selection(cx, |projects| { + match projects.rows.iter_mut().find(|row| row.slug == project.slug) { + Some(row) if row.version > project.version => {} + Some(row) => *row = project, + None => projects.rows.push(project), + } + projects::sort(&mut projects.rows); + }); } /// The priority choices: P0 to P5, then no tier (empty). @@ -120,6 +123,22 @@ impl ProjectsDialog { .collect() } + /// Changes the rows and keeps the selection on the same project (#91): where a re-sort moved it or, once it is no + /// longer listed (untracked while untracked rows are hidden), on the row at its old position. + fn keep_selection(&mut self, cx: &gpui_kit::App, change: impl FnOnce(&mut Self)) { + let previous = self + .visible(cx) + .get(self.selected) + .map(|&row| self.rows[row].slug.clone()); + change(self); + let visible = self.visible(cx); + self.selected = selection::reselect( + previous.as_deref(), + self.selected, + visible.iter().map(|&row| self.rows[row].slug.as_str()), + ); + } + fn is_linked(&self, project: &Project) -> Option { self.linked .as_ref() @@ -134,6 +153,12 @@ impl ProjectsDialog { .collect() } + /// The selected row's position among the visible rows. + #[cfg(test)] + pub fn selected(&self) -> usize { + self.selected + } + #[cfg(test)] pub fn editor_form(&self) -> Option<&Form> { self.editor.as_ref().map(|editor| &editor.form) @@ -291,7 +316,7 @@ impl Manage { } } projects::sort(&mut rows); - projects.rows = rows; + projects.keep_selection(cx, |projects| projects.rows = rows); projects.load_error = None; } // The last good list stays beside the error. @@ -442,6 +467,7 @@ impl Manage { if let Some((_, projects)) = self.open_projects() { projects.editor = None; projects.list.focus(window, cx); + projects.scroll.scroll_to_item(projects.selected); } cx.notify(); } @@ -557,7 +583,7 @@ impl Manage { // the dialog closed while it was on its way. cx.emit(ManageEvent::Wrote); if let Some(projects) = manage.projects_of(id) { - upsert(projects, saved.clone()); + upsert(projects, saved.clone(), cx); } } let projects = manage.projects_of(id)?; @@ -619,7 +645,7 @@ impl Manage { }; // pmd's current project goes in the list, in order; a created one also reloads the table. if let Ok(fresh) = &fresh { - upsert(projects, fresh.clone()); + upsert(projects, fresh.clone(), cx); if after == After::Found { cx.emit(ManageEvent::Wrote); } diff --git a/crates/dashboard-app/src/manage/work_items.rs b/crates/dashboard-app/src/manage/work_items.rs index 8eb9300..7fd279e 100644 --- a/crates/dashboard-app/src/manage/work_items.rs +++ b/crates/dashboard-app/src/manage/work_items.rs @@ -10,6 +10,7 @@ use std::sync::Arc; use dashboard_core::edit::{Form, Next}; +use dashboard_core::selection; use dashboard_core::work::{ self, AUTO, FIELDS, Kind, MAX_REASON, STATUSES, WorkFilter, WorkItem, WorkPage, status_label, }; @@ -279,9 +280,19 @@ impl Manage { match result { Ok(page) => { if cursor.is_none() { + // Under the same filters the selected item stays selected wherever the new page puts + // it (#91), after a save reorders it or a change elsewhere moves it. Under new filters + // the position stays, as before; a filter chip has already reset it to the first row. + let previous = (work.rows_filter == filter) + .then(|| work.items.get(work.selected).map(|item| item.id.clone())) + .flatten(); work.rows_filter = filter; work.items = page.items; - work.selected = work.selected.min(work.items.len().saturating_sub(1)); + work.selected = selection::reselect( + previous.as_deref(), + work.selected, + work.items.iter().map(|item| item.id.as_str()), + ); } else { work.items.extend(page.items); } diff --git a/crates/dashboard-app/src/projects_tests.rs b/crates/dashboard-app/src/projects_tests.rs index d65d76a..8fd69fc 100644 --- a/crates/dashboard-app/src/projects_tests.rs +++ b/crates/dashboard-app/src/projects_tests.rs @@ -777,3 +777,168 @@ async fn a_priority_changed_elsewhere_moves_the_row_after_the_conflict(cx: &mut "P0 chief sorts first now: {order:?}" ); } + +fn selected(cx: &mut TestAppContext, manage: &Entity) -> usize { + cx.update(|cx| manage.read(cx).projects().unwrap().selected()) +} + +/// The slug of the project the editor shows, if one is open. +fn editing(cx: &mut TestAppContext, manage: &Entity) -> Option { + cx.update(|cx| { + manage + .read(cx) + .projects() + .unwrap() + .editor_form() + .map(|form| form.value("slug").to_owned()) + }) +} + +/// The test's one write went to `slug`, at the version it showed. +fn only_write_is_to(backend: &FakeProjects, slug: &str) { + let writes = backend.writes.lock().unwrap().clone(); + assert_eq!(writes.len(), 1, "{writes:?}"); + assert_eq!(writes[0].path, format!("v1/projects/{slug}")); + assert_eq!(writes[0].precondition, Precondition::IfMatch(2)); +} + +/// Down to visible row `ix`, then Enter edits it. +fn open_row(cx: &mut TestAppContext, handle: AnyWindowHandle, ix: usize) { + for _ in 0..ix { + press(cx, handle, "down"); + } + press(cx, handle, "enter"); +} + +#[gpui_kit::test] +async fn a_priority_save_that_moves_the_project_keeps_it_selected_after_back(cx: &mut TestAppContext) { + let folder = Folder::new(); + let backend = FakeProjects::new(&["read", "write:projects"]); + let (handle, view, manage) = open_with(cx, backend.clone(), &folder); + open_dialog(cx, handle, &view).await; + open_row(cx, handle, 3); + assert_eq!(editing(cx, &manage).as_deref(), Some("gh-x")); + click(cx, handle, "project-priority-0"); + press(cx, handle, "ctrl-enter"); + settle(cx, handle, &view).await; + only_write_is_to(&backend, "gh-x"); + click(cx, handle, "projects-back"); + assert!(focused(cx, handle, "projects-list")); + assert_eq!( + visible(cx, &manage), + ["gh-x", "project-dashboard", "set-it-free-loop", "hs-buddy", "chief"], + "P0 sorts gh-x first" + ); + assert_eq!(selected(cx, &manage), 0, "the highlight moved with gh-x"); + press(cx, handle, "enter"); + assert_eq!( + editing(cx, &manage).as_deref(), + Some("gh-x"), + "Enter reopens the project just saved" + ); +} + +#[gpui_kit::test] +async fn a_project_save_that_keeps_the_order_keeps_the_same_row_selected(cx: &mut TestAppContext) { + let folder = Folder::new(); + let backend = FakeProjects::new(&["read", "write:projects"]); + let (handle, view, manage) = open_with(cx, backend.clone(), &folder); + open_dialog(cx, handle, &view).await; + open_row(cx, handle, 3); + set_input(cx, handle, &manage, "goal", "Keep it healthy"); + press(cx, handle, "ctrl-enter"); + settle(cx, handle, &view).await; + only_write_is_to(&backend, "gh-x"); + click(cx, handle, "projects-back"); + assert_eq!(visible(cx, &manage)[3], "gh-x"); + assert_eq!(selected(cx, &manage), 3); + press(cx, handle, "enter"); + assert_eq!(editing(cx, &manage).as_deref(), Some("gh-x")); +} + +#[gpui_kit::test] +async fn a_priority_changed_elsewhere_keeps_the_project_selected_after_the_conflict(cx: &mut TestAppContext) { + let folder = Folder::new(); + let backend = FakeProjects::new(&["read", "write:projects"]); + let (handle, view, manage) = open_with(cx, backend.clone(), &folder); + open_dialog(cx, handle, &view).await; + open_row(cx, handle, 4); + assert_eq!(editing(cx, &manage).as_deref(), Some("chief")); + set_input(cx, handle, &manage, "notes", "mine"); + backend.fail_next(WriteError::StaleVersion, |rows| { + let row = rows.iter_mut().find(|row| row["slug"] == "chief").unwrap(); + row["priority"] = json!(0); + row["version"] = json!(7); + }); + click(cx, handle, "project-save"); + settle(cx, handle, &view).await; + click(cx, handle, "conflict-discard"); + click(cx, handle, "projects-back"); + assert_eq!(visible(cx, &manage)[0], "chief", "the re-read moved chief first"); + assert_eq!(selected(cx, &manage), 0); + press(cx, handle, "enter"); + assert_eq!(editing(cx, &manage).as_deref(), Some("chief")); + assert_eq!(backend.writes.lock().unwrap().len(), 1, "a conflict is never retried"); +} + +#[gpui_kit::test] +async fn an_untracked_project_hidden_from_the_list_hands_the_selection_to_a_listed_row(cx: &mut TestAppContext) { + let folder = Folder::new(); + let backend = FakeProjects::new(&["read", "write:projects"]); + let (handle, view, manage) = open_with(cx, backend.clone(), &folder); + open_dialog(cx, handle, &view).await; + // chief is the last visible row. + open_row(cx, handle, 4); + assert_eq!(editing(cx, &manage).as_deref(), Some("chief")); + click(cx, handle, "project-untrack"); + click(cx, handle, "untrack-confirm-button"); + settle(cx, handle, &view).await; + only_write_is_to(&backend, "chief"); + assert_eq!(backend.row("chief")["tracked"], json!(false)); + click(cx, handle, "projects-back"); + assert_eq!( + visible(cx, &manage), + ["project-dashboard", "set-it-free-loop", "hs-buddy", "gh-x"], + "untracked rows are hidden" + ); + assert_eq!(selected(cx, &manage), 3, "the last listed row, not past it"); + press(cx, handle, "enter"); + assert_eq!(editing(cx, &manage).as_deref(), Some("gh-x")); +} + +#[gpui_kit::test] +async fn when_the_last_project_leaves_the_list_enter_opens_nothing(cx: &mut TestAppContext) { + let folder = Folder::new(); + let backend = FakeProjects::new(&["read", "write:projects"]); + let (handle, view, manage) = open_with(cx, backend.clone(), &folder); + open_dialog(cx, handle, &view).await; + set_input(cx, handle, &manage, "filter", "chief"); + assert_eq!(visible(cx, &manage), ["chief"]); + click(cx, handle, "project-row-0"); + click(cx, handle, "project-untrack"); + click(cx, handle, "untrack-confirm-button"); + settle(cx, handle, &view).await; + only_write_is_to(&backend, "chief"); + click(cx, handle, "projects-back"); + assert!(visible(cx, &manage).is_empty()); + assert!(exists(cx, handle, "projects-list-empty")); + assert_eq!(selected(cx, &manage), 0); + press(cx, handle, "enter"); + assert_eq!(editing(cx, &manage), None, "no row, no editor"); + assert_eq!(backend.writes.lock().unwrap().len(), 1, "and nothing else is written"); +} + +#[gpui_kit::test] +async fn a_project_filter_change_selects_the_first_row(cx: &mut TestAppContext) { + let folder = Folder::new(); + let backend = FakeProjects::new(&["read", "write:projects"]); + let (handle, view, manage) = open_with(cx, backend.clone(), &folder); + open_dialog(cx, handle, &view).await; + for _ in 0..3 { + press(cx, handle, "down"); + } + assert_eq!(selected(cx, &manage), 3); + set_input(cx, handle, &manage, "filter", "hemsoft-dev"); + assert_eq!(visible(cx, &manage)[3], "gh-x", "gh-x is still listed at row 3"); + assert_eq!(selected(cx, &manage), 0); +} diff --git a/crates/dashboard-app/src/work_items_tests.rs b/crates/dashboard-app/src/work_items_tests.rs index 3768cdb..1023cef 100644 --- a/crates/dashboard-app/src/work_items_tests.rs +++ b/crates/dashboard-app/src/work_items_tests.rs @@ -1,6 +1,7 @@ //! Headless UI tests for the Work items dialog (#35), against an in-memory pmd. No desktop window, OS input or network. use std::path::PathBuf; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; use dashboard_core::cache::StatusCache; @@ -58,6 +59,9 @@ pub(crate) struct FakeWork { pub fail_next: Mutex) + Send>>)>>, /// Listings fail while this is set. pub fail_lists: Mutex>, + /// Lists in pmd's order (effective priority, rank, then newest) instead of the order the items were given in, so + /// a priority or rank save moves the item. + pub in_pmd_order: AtomicBool, } impl FakeWork { @@ -69,6 +73,7 @@ impl FakeWork { writes: Mutex::default(), fail_next: Mutex::default(), fail_lists: Mutex::default(), + in_pmd_order: AtomicBool::new(false), }) } @@ -108,7 +113,7 @@ impl WriteBackend for FakeWork { } let get = |key: &str| query.iter().find(|(k, _)| k == key).map(|(_, v)| v.clone()); let items = self.items.lock().unwrap(); - let matching: Vec = items + let mut matching: Vec = items .iter() .filter(|item| item["ghState"] == "open") .filter(|item| get("status").is_none_or(|status| item["status"] == status.as_str())) @@ -120,6 +125,14 @@ impl WriteBackend for FakeWork { }) .cloned() .collect(); + if self.in_pmd_order.load(Ordering::SeqCst) { + matching.sort_by(|a, b| { + let key = |item: &Value| (item["priority"].as_u64(), item["rank"].as_i64()); + key(a) + .cmp(&key(b)) + .then_with(|| b["ghUpdatedAt"].as_str().cmp(&a["ghUpdatedAt"].as_str())) + }); + } let limit: usize = get("limit").unwrap().parse().unwrap(); let start: usize = get("cursor").map_or(0, |cursor| cursor.parse().unwrap()); let page: Vec = matching.iter().skip(start).take(limit).cloned().collect(); @@ -829,3 +842,205 @@ async fn an_item_that_closed_before_the_conflict_reread_shows_as_closed(cx: &mut assert!(!exists(cx, handle, "conflict-reapply")); assert!(exists(cx, handle, "work-closed")); } + +/// A pmd listing in its own order, over `items`. +fn in_pmd_order(items: Vec) -> Arc { + let backend = FakeWork::new(&["read", "write:work"], items); + backend.in_pmd_order.store(true, Ordering::SeqCst); + backend +} + +/// Three open gh-x items, #200 to #202, all P2 at rank 0. +fn three_items() -> Vec { + (0..3).map(|ix| work_item(ix, "gh-x", "issue", "An item")).collect() +} + +fn numbers(cx: &mut TestAppContext, manage: &Entity) -> Vec { + items(cx, manage).iter().map(|item| item.number).collect() +} + +fn selected(cx: &mut TestAppContext, manage: &Entity) -> usize { + cx.update(|cx| manage.read(cx).work_items().unwrap().selected()) +} + +fn editing(cx: &mut TestAppContext, manage: &Entity) -> Option { + cx.update(|cx| { + manage + .read(cx) + .work_items() + .unwrap() + .editor_item() + .map(|item| item.number) + }) +} + +/// The test's one write went to the item it edited, at the version it showed. +fn only_write_is_to(backend: &FakeWork, number: u64) { + let writes = backend.writes.lock().unwrap().clone(); + assert_eq!(writes.len(), 1, "{writes:?}"); + assert_eq!(writes[0].path, format!("v1/work/01JBXWORK{:04}", number - 200)); + assert_eq!(writes[0].precondition, Precondition::IfMatch(3)); +} + +#[gpui_kit::test] +async fn a_priority_save_that_moves_the_item_keeps_it_selected_after_back(cx: &mut TestAppContext) { + let backend = in_pmd_order(three_items()); + let (handle, view, manage) = open_with(cx, backend.clone()); + open_dialog(cx, handle, &view).await; + press(cx, handle, "down"); + press(cx, handle, "down"); + press(cx, handle, "enter"); + assert_eq!(editing(cx, &manage), Some(202)); + click(cx, handle, "work-priority-0"); + press(cx, handle, "ctrl-enter"); + settle(cx, handle, &view).await; + only_write_is_to(&backend, 202); + assert_eq!(numbers(cx, &manage), [202, 200, 201], "P0 sorts #202 first"); + click(cx, handle, "work-back"); + assert!(focused(cx, handle, "work-list")); + assert_eq!(selected(cx, &manage), 0, "the highlight moved with #202"); + press(cx, handle, "enter"); + assert_eq!(editing(cx, &manage), Some(202), "Enter reopens the item just saved"); +} + +#[gpui_kit::test] +async fn a_rank_save_that_moves_the_item_down_keeps_it_selected_after_back(cx: &mut TestAppContext) { + let mut items = three_items(); + items[2]["rank"] = json!(10); + let backend = in_pmd_order(items); + let (handle, view, manage) = open_with(cx, backend.clone()); + open_dialog(cx, handle, &view).await; + press(cx, handle, "enter"); + assert_eq!(editing(cx, &manage), Some(200)); + set_input(cx, handle, &manage, "rank", "5"); + press(cx, handle, "ctrl-enter"); + settle(cx, handle, &view).await; + only_write_is_to(&backend, 200); + assert_eq!( + numbers(cx, &manage), + [201, 200, 202], + "rank 5 sorts #200 between 0 and 10" + ); + click(cx, handle, "work-back"); + assert_eq!(selected(cx, &manage), 1); + press(cx, handle, "enter"); + assert_eq!(editing(cx, &manage), Some(200)); +} + +#[gpui_kit::test] +async fn a_save_that_keeps_the_order_keeps_the_same_row_selected(cx: &mut TestAppContext) { + let backend = in_pmd_order(three_items()); + let (handle, view, manage) = open_with(cx, backend.clone()); + open_dialog(cx, handle, &view).await; + click(cx, handle, "work-row-2"); + click(cx, handle, "work-status-blocked"); + set_input(cx, handle, &manage, "statusReason", "waiting on review"); + press(cx, handle, "ctrl-enter"); + settle(cx, handle, &view).await; + only_write_is_to(&backend, 202); + assert_eq!(numbers(cx, &manage), [200, 201, 202]); + click(cx, handle, "work-back"); + assert_eq!(selected(cx, &manage), 2); + press(cx, handle, "enter"); + assert_eq!(editing(cx, &manage), Some(202)); +} + +#[gpui_kit::test] +async fn a_moved_item_stays_selected_when_the_reload_brings_back_only_the_first_page(cx: &mut TestAppContext) { + let many = (0..62).map(|ix| work_item(ix, "gh-x", "issue", "An item")).collect(); + let backend = in_pmd_order(many); + let (handle, view, manage) = open_with(cx, backend.clone()); + open_dialog(cx, handle, &view).await; + click(cx, handle, "work-load-more"); + settle(cx, handle, &view).await; + assert_eq!(items(cx, &manage).len(), 62); + // Load more is gone with the last page, and its focus with it: Tab back to the list, then the keyboard reaches + // row 55, scrolling as it goes. + for _ in 0..40 { + if focused(cx, handle, "work-list") { + break; + } + press(cx, handle, "tab"); + } + assert!(focused(cx, handle, "work-list")); + for _ in 0..55 { + press(cx, handle, "down"); + } + press(cx, handle, "enter"); + assert_eq!(editing(cx, &manage), Some(255)); + click(cx, handle, "work-priority-0"); + click(cx, handle, "work-save"); + settle(cx, handle, &view).await; + let writes = backend.writes.lock().unwrap().clone(); + assert_eq!(writes.len(), 1); + assert_eq!(writes[0].path, "v1/work/01JBXWORK0055"); + assert_eq!( + items(cx, &manage).len(), + 50, + "the reload starts again from the first page" + ); + click(cx, handle, "work-back"); + assert_eq!(selected(cx, &manage), 0); + press(cx, handle, "enter"); + assert_eq!(editing(cx, &manage), Some(255)); +} + +#[gpui_kit::test] +async fn an_item_that_leaves_the_status_filter_hands_the_selection_to_a_listed_row(cx: &mut TestAppContext) { + let backend = in_pmd_order(three_items()); + let (handle, view, manage) = open_with(cx, backend.clone()); + open_dialog(cx, handle, &view).await; + click(cx, handle, "work-filter-status-ready"); + settle(cx, handle, &view).await; + click(cx, handle, "work-row-1"); + click(cx, handle, "work-status-blocked"); + set_input(cx, handle, &manage, "statusReason", "waiting on review"); + click(cx, handle, "work-save"); + settle(cx, handle, &view).await; + only_write_is_to(&backend, 201); + assert_eq!(numbers(cx, &manage), [200, 202], "#201 is blocked now, not ready"); + click(cx, handle, "work-back"); + assert_eq!(selected(cx, &manage), 1, "the same position, on a listed row"); + press(cx, handle, "enter"); + assert_eq!(editing(cx, &manage), Some(202)); +} + +#[gpui_kit::test] +async fn when_the_last_item_leaves_the_list_enter_opens_nothing(cx: &mut TestAppContext) { + let mut items = three_items(); + items[0]["status"] = json!("blocked"); + items[1]["status"] = json!("blocked"); + let backend = in_pmd_order(items); + let (handle, view, manage) = open_with(cx, backend.clone()); + open_dialog(cx, handle, &view).await; + click(cx, handle, "work-filter-status-ready"); + settle(cx, handle, &view).await; + assert_eq!(numbers(cx, &manage), [202]); + click(cx, handle, "work-row-0"); + click(cx, handle, "work-status-blocked"); + set_input(cx, handle, &manage, "statusReason", "waiting on review"); + press(cx, handle, "ctrl-enter"); + settle(cx, handle, &view).await; + only_write_is_to(&backend, 202); + click(cx, handle, "work-back"); + assert!(exists(cx, handle, "work-list-empty")); + assert_eq!(selected(cx, &manage), 0); + press(cx, handle, "enter"); + assert_eq!(editing(cx, &manage), None, "no row, no editor"); + assert_eq!(backend.writes.lock().unwrap().len(), 1, "and nothing else is written"); +} + +#[gpui_kit::test] +async fn a_filter_change_selects_the_first_row_even_when_the_selected_item_is_still_listed(cx: &mut TestAppContext) { + let backend = in_pmd_order(three_items()); + let (handle, view, manage) = open_with(cx, backend.clone()); + open_dialog(cx, handle, &view).await; + press(cx, handle, "down"); + press(cx, handle, "down"); + assert_eq!(selected(cx, &manage), 2); + click(cx, handle, "work-filter-kind-issue"); + settle(cx, handle, &view).await; + assert_eq!(numbers(cx, &manage), [200, 201, 202], "#202 is still listed"); + assert_eq!(selected(cx, &manage), 0); + assert!(backend.writes.lock().unwrap().is_empty()); +} diff --git a/crates/dashboard-core/src/lib.rs b/crates/dashboard-core/src/lib.rs index f9795c0..3cbc072 100644 --- a/crates/dashboard-core/src/lib.rs +++ b/crates/dashboard-core/src/lib.rs @@ -18,6 +18,7 @@ pub mod priority; pub mod projects; pub mod recovery; pub mod scheduler; +pub mod selection; pub mod source; pub mod status; pub mod t3; diff --git a/crates/dashboard-core/src/selection.rs b/crates/dashboard-core/src/selection.rs new file mode 100644 index 0000000..64a33e1 --- /dev/null +++ b/crates/dashboard-core/src/selection.rs @@ -0,0 +1,53 @@ +//! Which list row stays selected when the rows change under it (#91). + +/// The row to select once a list's rows change. `previous` is the identity the selected row had before the change and +/// `old` its position. The row that now has that identity stays selected, wherever it moved. Once nothing has it any +/// more, the position stays, kept within the new list. An empty list gives row 0, which holds nothing. +pub fn reselect<'a, K: PartialEq + ?Sized + 'a>( + previous: Option<&K>, + old: usize, + keys: impl IntoIterator, +) -> usize { + let mut len = 0; + for (ix, key) in keys.into_iter().enumerate() { + if previous == Some(key) { + return ix; + } + len = ix + 1; + } + old.min(len.saturating_sub(1)) +} + +#[cfg(test)] +mod tests { + use super::reselect; + + #[test] + fn the_selection_follows_its_row_to_a_new_position() { + assert_eq!(reselect(Some("c"), 2, ["c", "a", "b"]), 0); + assert_eq!(reselect(Some("a"), 0, ["b", "c", "a"]), 2); + } + + #[test] + fn an_unchanged_order_keeps_the_same_row() { + assert_eq!(reselect(Some("b"), 1, ["a", "b", "c"]), 1); + } + + #[test] + fn a_row_that_left_keeps_the_position_within_the_list() { + assert_eq!(reselect(Some("b"), 1, ["a", "c", "d"]), 1); + assert_eq!(reselect(Some("d"), 3, ["a", "b", "c"]), 2, "the last row, not past it"); + } + + #[test] + fn without_an_identity_the_position_stays_within_the_list() { + assert_eq!(reselect(None::<&str>, 4, ["a", "b"]), 1); + assert_eq!(reselect(None::<&str>, 0, ["a", "b"]), 0); + } + + #[test] + fn an_empty_list_selects_row_zero() { + assert_eq!(reselect(Some("a"), 3, Vec::<&str>::new()), 0); + assert_eq!(reselect(None::<&str>, 0, Vec::<&str>::new()), 0); + } +} From d1a076168012dc1968409822b1025d792129e7b2 Mon Sep 17 00:00:00 2001 From: Franz Hemmer Date: Fri, 6 Nov 2026 20:01:47 -0500 Subject: [PATCH 2/2] test: reach the list by Back after the last page loads (#91) Load more disappears with the last page and takes the focus with it, so Tab had nothing to move from. Opening a row and going Back focuses the list the way the dialog does. Co-Authored-By: Claude Opus 5.5 --- crates/dashboard-app/src/work_items_tests.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/crates/dashboard-app/src/work_items_tests.rs b/crates/dashboard-app/src/work_items_tests.rs index 1023cef..1c958e9 100644 --- a/crates/dashboard-app/src/work_items_tests.rs +++ b/crates/dashboard-app/src/work_items_tests.rs @@ -954,14 +954,10 @@ async fn a_moved_item_stays_selected_when_the_reload_brings_back_only_the_first_ click(cx, handle, "work-load-more"); settle(cx, handle, &view).await; assert_eq!(items(cx, &manage).len(), 62); - // Load more is gone with the last page, and its focus with it: Tab back to the list, then the keyboard reaches - // row 55, scrolling as it goes. - for _ in 0..40 { - if focused(cx, handle, "work-list") { - break; - } - press(cx, handle, "tab"); - } + // Load more is gone with the last page, and its focus with it. Back from an editor focuses the list again; the + // keyboard then reaches row 55, scrolling as it goes. + click(cx, handle, "work-row-0"); + click(cx, handle, "work-back"); assert!(focused(cx, handle, "work-list")); for _ in 0..55 { press(cx, handle, "down");