Skip to content

Commit ad18da4

Browse files
authored
feat(desktop): pick session rows with shift and cmd (#4455)
* refactor(desktop): move task deletion out of the Session rail Deleting a task is the one row action that cannot be undone, and the rail is where a mis-click is likeliest: rows are dense, the ⋯ menu is one hover away, and the row under the cursor moves as the catalog refreshes. The rail now stops at 归档 — the row menu, the selection bar's second button, and the Delete/Backspace binding are all gone. Deleting still exists, in Settings › 已归档任务, which can only reach a task that was archived first. That archive step is what makes the intent deliberate, and it is the same route the bulk purge already took. Two behaviours change for the user. Deleting a task that was never archived is now two steps instead of one: archive it in the rail, then delete it in Settings. Delete and Backspace no longer act on the focused row. With the rail's bulk delete gone, `sweepSessions` had one caller left and one live value for its `requireArchivedFor` parameter, so it collapses back into `purgeSessions`. Generated-by: Claude Opus 5 via Claude Code * feat(desktop): pick session rows with shift and cmd The Session rail's multi-select was a MODE. A row's ⋯ offered 选择任务, which entered it; entering swapped a checkbox column in beside every row, put a bar with a master checkbox above the list, and changed what an ordinary click meant. Leaving it was a 取消 button or Escape. The mode lived in one state, the marks in another, and the bar and the rows read two different halves of it — while the ⋯ menu went on offering to act on the single row under the cursor with twelve rows visibly marked. Replace it with the gesture every file list already uses. A plain click opens a task and picks exactly it; ⌘/Ctrl-click adds or removes one row without moving the main pane; Shift-click picks the run from the anchor to the row clicked. There is nothing to enter and nothing to leave, so Escape simply drops the picks. Picked rows and the open row share one ground — the same `var(--color-neutral)` Astryx's own `isSelected` compiles to, so the two are one paint rather than a near-match kept by hand. `isSelected` itself stays on the open row alone: it renders `aria-current="page"`, and a set of picked rows is not a set of current pages. What a screen reader hears instead rides in the row's existing description. ⋯ and right-click are one menu, not two lists to keep in agreement: right-click opens the row's own ⋯. Its verbs count the set — 置顶 3 项, 归档 3 项 — and a set already pinned throughout offers 取消置顶 instead, which is what a set-wide toggle has to do to be unambiguous. A menu opened on a row outside the set replaces the set with that row, the way a file list answers a right-click on an unselected file; a row already in the set leaves it alone. 重命名 is absent from a set menu: renaming names one task. The picked set reaches rows as PROPS. A plain click now changes the selection, so a context the rows subscribed to would redraw all ~1,000 fibers for a switch that moved two of them (#4109); the rendered order a Shift range needs is read off the DOM at the moment of the click, which costs nothing per render and is the true order rather than a reconstruction of it. Archiving a set does not confirm, matching the single row — it is reversible, and Settings › 已归档任务 is where it can be undone or made permanent. Generated-by: Claude Opus 5 via Claude Code * fix(desktop): only let a gesture pick rows the user can see and act on A Shift range read the rail's rendered order straight off the DOM, and rendered is not the same as reachable. `SideNavItem` collapses a project by grid track rather than by unmounting, so a range dragged past a collapsed group swept up rows that were nowhere on screen — twenty tasks archived from a gesture that highlighted three. A session shared from someone else's Host is handed no row actions for the same reason it has no ⋯, but nothing stopped it joining a set and being archived with it; and right-clicking one claimed the press, discarded the set, and opened nothing, because the ⋯ it went looking for was never rendered. One question was being answered in two selectors that only knew how to find a row, so `isPickableRow` answers it once and the list reads it everywhere: a row is pickable when it is ours to act on and not inside an inert subtree. A row with no actions is now a plain navigation item all the way down — a modifier on it navigates rather than dying, and a right-click on it goes to the native menu. Escape had the same shape of bug one layer up: the guard asked whether focus was in a text field, when the question is whether anything above the rail owns the press. Astryx's layer stack listens on `document`, below this handler in the bubble, and stands down on a press that is already defaultPrevented — so Escape in a rename dialog cleared the set AND left the dialog with no way to close. Also drops the sweep's post-completion unmark. "The selection follows the catalog" already has an owner in the prune effect, and an archive refreshes the catalog before it resolves; saying it twice was redundant for archive and wrong for pin, which leaves its rows listed and had the set empty itself out from under the next action. Generated-by: Claude Opus 5 via Claude Code * fix(desktop): sweep only the picked rows the user can still see A pick can stop being visible without leaving the catalog: collapsing its project keeps the row mounted and merely `inert`, and a session that becomes a shared projection loses its actions in place. Pruning against the catalog still lists both, so a menu opened from a visible row could count — and archive — rows that are not on screen. Narrow the set at the menu, which is the single entrance to the sweeps, so the menu means the set it names: opening one fixes the set at the rows the user can see and act on, and a pick that was folded away is out of it from then on, dismissed menu included. Not narrowed when the project actually collapses, and not derived during render, because collapse is uncontrolled state inside Astryx's `SideNavItem` that the rail can only read back off the DOM — the menu's entrance is the one moment that holds both the set and the DOM. * fix(desktop): drop the picks when a task opens from outside the rail Command palette, Module Hub and 新建任务 move the open task without going through a row, so the rail hears about it only as a new active id. Picked rows and the open row share one ground, so a set built around the task the user walked away from goes on reading as a set around the one they are now looking at — and the next ⋯ sweeps it. Cleared rather than replaced onto the new row: the rail paints the open row whether or not it is picked, so an empty selection already reads as "just this row", and a set of one that happens to equal it would be a second way to say the same thing. A click inside the rail arrives here too, but its `replace` has already put the new row in the set, so it is left alone.
1 parent c86da40 commit ad18da4

28 files changed

Lines changed: 1777 additions & 1329 deletions

‎apps/desktop/e2e/composer-plus-menu-stability.spec.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -359,13 +359,18 @@ test('latest Plan intent still reaches the Host after a catalog refresh fails',
359359
await expect(planRow).toHaveAttribute('aria-checked', 'false');
360360
});
361361

362-
test('deleting the session while a toggle is pending settles clean', async ({
362+
test('removing the session while a toggle is pending settles clean', async ({
363363
invocableSkillsWindow: page,
364364
}) => {
365365
// pending → cleanup → settle: the Session's renderer lifecycle ends while a
366366
// mode commit (and a queued follow-up intent) is still in flight. Cleanup
367367
// must drop the queued ask with the rest of the Session state, so the
368368
// commit's tail has nothing to replay against the removed Session.
369+
//
370+
// Archiving is what ends that lifecycle from the rail now that deleting has
371+
// moved to Settings. It is the same ending as far as this case is concerned:
372+
// `archiveSession` clears the active id, the active messages and the whole
373+
// family's renderer state, exactly as removal did.
369374
const composer = page.locator(COMPOSER_INPUT);
370375
await composer.fill('alpha-marker');
371376
await composer.press('Enter');
@@ -389,9 +394,7 @@ test('deleting the session while a toggle is pending settles clean', async ({
389394
const row = sidebar.locator('[data-maka-contract="session-row"]').first();
390395
await row.hover();
391396
await row.getByRole('button', { name: '任务操作' }).click();
392-
await page.getByRole('menuitem', { name: '删除', exact: true }).click();
393-
const confirm = page.getByRole('alertdialog');
394-
await confirm.getByRole('button', { name: '删除', exact: true }).click();
397+
await page.getByRole('menuitem', { name: '归档', exact: true }).click();
395398

396399
await releaseBridgeLatch(page, 'sessions.list');
397400
await expect(sidebar.locator('[data-maka-contract="session-row"]')).toHaveCount(0);

‎apps/desktop/e2e/parent-session-deletion.spec.ts‎

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ import {
2424
test,
2525
} from './fixtures';
2626

27+
/**
28+
* The route is two steps on purpose: the rail archives, Settings deletes. The
29+
* rail has no delete at all — a mis-click there is one hover away from an
30+
* irreversible loss — so a task must have been archived once before anything
31+
* can remove it. This walks the whole route rather than calling the command,
32+
* because the route is the thing that changed.
33+
*/
2734
test('deleting a parent task archives its linked subagent task', async ({
2835
parentRemovalWindow: page,
2936
}) => {
@@ -38,7 +45,23 @@ test('deleting a parent task archives its linked subagent task', async ({
3845

3946
await parentRow.hover();
4047
await parentRow.getByRole('button', { name: '任务操作' }).click();
41-
await page.getByRole('menuitem', { name: '删除', exact: true }).click();
48+
// The rail's menu ends at 归档. Deleting is not one of the things a row can
49+
// be asked to do here.
50+
await expect(page.getByRole('menuitem', { name: '删除', exact: true })).toHaveCount(0);
51+
await page.getByRole('menuitem', { name: '归档', exact: true }).click();
52+
await expect(parentRow).toHaveCount(0);
53+
54+
await page.getByRole('button', { name: '设置', exact: true }).click();
55+
await expect(page.getByRole('main', { name: '设置内容' })).toBeVisible();
56+
await page.getByRole('button', { name: '已归档任务', exact: true }).click();
57+
const archivedTasks = page.getByRole('main', { name: '设置内容' });
58+
59+
await archivedTasks
60+
.getByRole('button', { name: `「${PARENT_REMOVAL_PARENT_NAME}」的更多操作` })
61+
.click();
62+
// 彻底删除, not 删除: Settings names the irreversible verb in full, which is
63+
// the point of routing every deletion through a surface reached by archiving.
64+
await page.getByRole('menuitem', { name: '彻底删除', exact: true }).click();
4265
const confirm = page.getByRole('alertdialog', {
4366
name: `删除 "${PARENT_REMOVAL_PARENT_NAME}"`,
4467
});
@@ -49,14 +72,6 @@ test('deleting a parent task archives its linked subagent task', async ({
4972
await expect(confirm.getByText(/子任务.*归档/)).toBeVisible();
5073
await confirm.getByRole('button', { name: '删除', exact: true }).click();
5174

52-
await expect(parentRow).toHaveCount(0);
53-
await expect(taskList.getByText(PARENT_REMOVAL_CHILD_NAME, { exact: true })).toHaveCount(0);
54-
55-
await page.getByRole('button', { name: '设置', exact: true }).click();
56-
await expect(page.getByRole('main', { name: '设置内容' })).toBeVisible();
57-
await page.getByRole('button', { name: '已归档任务', exact: true }).click();
58-
59-
const archivedTasks = page.getByRole('main', { name: '设置内容' });
6075
await expect(archivedTasks.getByText(PARENT_REMOVAL_CHILD_NAME, { exact: true })).toBeVisible();
6176
await expect(archivedTasks.getByText(/原父任务已删除/)).toBeVisible();
6277
await expect(archivedTasks.getByText(PARENT_REMOVAL_PARENT_NAME, { exact: true })).toHaveCount(0);

‎apps/desktop/src/main/__tests__/session-navigation-selection.test.ts‎

Lines changed: 91 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -19,101 +19,135 @@
1919

2020
import assert from 'node:assert/strict';
2121
import { describe, test } from 'node:test';
22+
import type { SessionRowPick } from '@maka/ui';
2223
import {
2324
EMPTY_SESSION_SELECTION,
24-
enterSessionSelection,
25-
exitSessionSelection,
25+
pickSessionRow,
2626
pruneSessionSelection,
27-
sessionSelectionMasterState,
28-
setAllSessionsSelected,
2927
type SessionSelection,
3028
} from '../../renderer/features/session-navigation/testing.js';
3129

32-
const GROUP = ['a', 'b', 'c', 'd', 'e'];
30+
const ORDER = ['a', 'b', 'c', 'd', 'e'];
3331

3432
function ids(selection: SessionSelection): string[] {
3533
return [...selection.selectedIds].sort();
3634
}
3735

38-
/** What a row's checkbox does, as the hook applies it. */
39-
function mark(selection: SessionSelection, sessionId: string): SessionSelection {
40-
return {
41-
active: true,
42-
selectedIds: new Set([...selection.selectedIds, sessionId]),
43-
};
36+
/** One click on a row, as the rail's handler passes it down. */
37+
function click(
38+
selection: SessionSelection,
39+
sessionId: string,
40+
pick: SessionRowPick,
41+
openSessionId?: string,
42+
): SessionSelection {
43+
return pickSessionRow(selection, {
44+
sessionId,
45+
pick,
46+
orderedSessionIds: ORDER,
47+
openSessionId,
48+
});
4449
}
4550

46-
describe('selection mode', () => {
47-
test('entering marks nothing on its own', () => {
48-
const entered = enterSessionSelection(EMPTY_SESSION_SELECTION);
49-
assert.equal(entered.active, true);
50-
assert.deepEqual(ids(entered), []);
51+
describe('a plain click', () => {
52+
test('makes the set exactly this row', () => {
53+
const from = click(EMPTY_SESSION_SELECTION, 'a', 'replace');
54+
assert.deepEqual(ids(click(from, 'd', 'replace')), ['d']);
55+
});
56+
57+
test('leaves the anchor on the row that was clicked', () => {
58+
// Which is what lets the very next Shift-click reach from here, without a
59+
// separate gesture to say where a range starts.
60+
const anchored = click(EMPTY_SESSION_SELECTION, 'b', 'replace');
61+
assert.deepEqual(ids(click(anchored, 'd', 'range')), ['b', 'c', 'd']);
5162
});
63+
});
5264

53-
test('leaving drops the mode and the marks together', () => {
54-
assert.equal(exitSessionSelection().active, false);
55-
assert.deepEqual(ids(exitSessionSelection()), []);
65+
describe('⌘-click', () => {
66+
test('adds a row without disturbing the rest', () => {
67+
const one = click(EMPTY_SESSION_SELECTION, 'a', 'replace');
68+
assert.deepEqual(ids(click(one, 'd', 'toggle')), ['a', 'd']);
5669
});
5770

58-
test('unticking every row is select-none, not leave', () => {
59-
// A mode that ended itself on the last untick would take the checkboxes
60-
// away mid-gesture, and one mis-click would cost the user the way back.
61-
const all = setAllSessionsSelected(EMPTY_SESSION_SELECTION, GROUP, true);
62-
const none = setAllSessionsSelected(all, GROUP, false);
63-
assert.deepEqual(ids(none), []);
64-
assert.equal(none.active, true);
71+
test('removes a row it finds already picked', () => {
72+
const two = click(click(EMPTY_SESSION_SELECTION, 'a', 'replace'), 'd', 'toggle');
73+
assert.deepEqual(ids(click(two, 'a', 'toggle')), ['d']);
6574
});
6675

67-
test('an emptied selection keeps the mode it was in', () => {
68-
// It used to settle on the shared EMPTY value, which also carries
69-
// `active: false` — so a catalog change that pruned the last row would have
70-
// taken the checkboxes away while the user was still selecting.
71-
const pruned = pruneSessionSelection(mark(EMPTY_SESSION_SELECTION, 'a'), []);
72-
assert.deepEqual(ids(pruned), []);
73-
assert.equal(pruned.active, true);
76+
test('moves the anchor, including when it unpicked the row', () => {
77+
// The anchor is "where the last non-Shift click landed", not "the last row
78+
// added": a person who unticks a row and then Shift-clicks means the run
79+
// between those two clicks, whatever the first one did to the set.
80+
const removed = click(click(EMPTY_SESSION_SELECTION, 'b', 'replace'), 'b', 'toggle');
81+
assert.deepEqual(ids(removed), []);
82+
assert.deepEqual(ids(click(removed, 'd', 'range')), ['b', 'c', 'd']);
7483
});
7584
});
7685

77-
describe('the master box', () => {
78-
test('marks exactly the rows the rail is listing', () => {
79-
// Not every task in the catalog: the box sits above these rows, and a
80-
// selection that reached past them would name a number nobody agreed to.
81-
assert.deepEqual(ids(setAllSessionsSelected(EMPTY_SESSION_SELECTION, ['a', 'b'], true)), [
82-
'a',
83-
'b',
84-
]);
86+
describe('Shift-click', () => {
87+
test('picks the run between the anchor and the row, in either direction', () => {
88+
const anchored = click(EMPTY_SESSION_SELECTION, 'd', 'replace');
89+
assert.deepEqual(ids(click(anchored, 'b', 'range')), ['b', 'c', 'd']);
90+
});
91+
92+
test('keeps the anchor so the run can be re-dragged from the same origin', () => {
93+
// Shortening a range is the same gesture as lengthening it. An anchor that
94+
// moved to the last row touched would make the second Shift-click reach
95+
// from the end of the first one, and the run would only ever grow.
96+
const anchored = click(EMPTY_SESSION_SELECTION, 'b', 'replace');
97+
const long = click(anchored, 'e', 'range');
98+
assert.deepEqual(ids(click(long, 'c', 'range')), ['b', 'c']);
8599
});
86100

87-
test('reads unchecked, indeterminate, then checked', () => {
88-
assert.equal(sessionSelectionMasterState(EMPTY_SESSION_SELECTION, GROUP), false);
89-
assert.equal(sessionSelectionMasterState(mark(EMPTY_SESSION_SELECTION, 'b'), GROUP), 'indeterminate');
90-
assert.equal(
91-
sessionSelectionMasterState(setAllSessionsSelected(EMPTY_SESSION_SELECTION, GROUP, true), GROUP),
92-
true,
93-
);
101+
test('starts from the open task when no click has set an anchor', () => {
102+
// The rail is a navigation surface first: a person who has been reading a
103+
// task and Shift-clicks another means the two of them and everything
104+
// between, without a preparatory click to say so.
105+
assert.deepEqual(ids(click(EMPTY_SESSION_SELECTION, 'd', 'range', 'b')), ['b', 'c', 'd']);
94106
});
95107

96-
test('an empty list is unchecked, never checked', () => {
97-
// `every` over an empty array is vacuously true, which would tick the box
98-
// above no rows at all.
99-
assert.equal(sessionSelectionMasterState(EMPTY_SESSION_SELECTION, []), false);
108+
test('with nothing to reach from is just this row', () => {
109+
assert.deepEqual(ids(click(EMPTY_SESSION_SELECTION, 'd', 'range')), ['d']);
100110
});
101111

102-
test('a mark outside the listed rows does not make it checked', () => {
103-
assert.equal(sessionSelectionMasterState(mark(EMPTY_SESSION_SELECTION, 'zzz'), GROUP), 'indeterminate');
112+
test('reaching for a row the list is not showing picks only the row clicked', () => {
113+
const stale = pickSessionRow(EMPTY_SESSION_SELECTION, {
114+
sessionId: 'd',
115+
pick: 'range',
116+
orderedSessionIds: ORDER,
117+
openSessionId: 'gone',
118+
});
119+
assert.deepEqual(ids(stale), ['d']);
120+
});
121+
});
122+
123+
describe('clearing', () => {
124+
test('drops the picks and the anchor together', () => {
125+
const cleared = EMPTY_SESSION_SELECTION;
126+
assert.deepEqual(ids(cleared), []);
127+
// With no anchor left, the next range starts from whatever is open —
128+
// which is where the user's attention already is.
129+
assert.deepEqual(ids(click(cleared, 'c', 'range', 'a')), ['a', 'b', 'c']);
104130
});
105131
});
106132

107133
describe('pruneSessionSelection', () => {
108134
test('drops ids the catalog no longer lists', () => {
109-
const selection = setAllSessionsSelected(EMPTY_SESSION_SELECTION, ['a', 'b'], true);
110-
assert.deepEqual(ids(pruneSessionSelection(selection, ['a'])), ['a']);
135+
const two = click(click(EMPTY_SESSION_SELECTION, 'a', 'replace'), 'b', 'toggle');
136+
assert.deepEqual(ids(pruneSessionSelection(two, ['a'])), ['a']);
137+
});
138+
139+
test('drops an anchor that went with them', () => {
140+
// A range from a row that is gone would reach across the rows that took
141+
// its place, which is not the run anybody drew.
142+
const anchored = click(EMPTY_SESSION_SELECTION, 'b', 'replace');
143+
const pruned = pruneSessionSelection(anchored, ['a', 'c', 'd', 'e']);
144+
assert.deepEqual(ids(click(pruned, 'd', 'range', 'c')), ['c', 'd']);
111145
});
112146

113147
test('returns the same value when nothing was dropped', () => {
114148
// Identity matters here: this runs on every catalog refresh, and a new Set
115149
// each time would re-render every row of the rail.
116-
const selection = mark(EMPTY_SESSION_SELECTION, 'a');
150+
const selection = click(EMPTY_SESSION_SELECTION, 'a', 'replace');
117151
assert.equal(pruneSessionSelection(selection, ['a', 'b']), selection);
118152
});
119153
});

0 commit comments

Comments
 (0)