diff --git a/apps/desktop/e2e/composer-plus-menu-stability.spec.ts b/apps/desktop/e2e/composer-plus-menu-stability.spec.ts index 5c5275726b..e90ff398ad 100644 --- a/apps/desktop/e2e/composer-plus-menu-stability.spec.ts +++ b/apps/desktop/e2e/composer-plus-menu-stability.spec.ts @@ -359,13 +359,18 @@ test('latest Plan intent still reaches the Host after a catalog refresh fails', await expect(planRow).toHaveAttribute('aria-checked', 'false'); }); -test('deleting the session while a toggle is pending settles clean', async ({ +test('removing the session while a toggle is pending settles clean', async ({ invocableSkillsWindow: page, }) => { // pending → cleanup → settle: the Session's renderer lifecycle ends while a // mode commit (and a queued follow-up intent) is still in flight. Cleanup // must drop the queued ask with the rest of the Session state, so the // commit's tail has nothing to replay against the removed Session. + // + // Archiving is what ends that lifecycle from the rail now that deleting has + // moved to Settings. It is the same ending as far as this case is concerned: + // `archiveSession` clears the active id, the active messages and the whole + // family's renderer state, exactly as removal did. const composer = page.locator(COMPOSER_INPUT); await composer.fill('alpha-marker'); await composer.press('Enter'); @@ -389,9 +394,7 @@ test('deleting the session while a toggle is pending settles clean', async ({ const row = sidebar.locator('[data-maka-contract="session-row"]').first(); await row.hover(); await row.getByRole('button', { name: '任务操作' }).click(); - await page.getByRole('menuitem', { name: '删除', exact: true }).click(); - const confirm = page.getByRole('alertdialog'); - await confirm.getByRole('button', { name: '删除', exact: true }).click(); + await page.getByRole('menuitem', { name: '归档', exact: true }).click(); await releaseBridgeLatch(page, 'sessions.list'); await expect(sidebar.locator('[data-maka-contract="session-row"]')).toHaveCount(0); diff --git a/apps/desktop/e2e/parent-session-deletion.spec.ts b/apps/desktop/e2e/parent-session-deletion.spec.ts index 629022dfe4..c42e4c88ff 100644 --- a/apps/desktop/e2e/parent-session-deletion.spec.ts +++ b/apps/desktop/e2e/parent-session-deletion.spec.ts @@ -24,6 +24,13 @@ import { test, } from './fixtures'; +/** + * The route is two steps on purpose: the rail archives, Settings deletes. The + * rail has no delete at all — a mis-click there is one hover away from an + * irreversible loss — so a task must have been archived once before anything + * can remove it. This walks the whole route rather than calling the command, + * because the route is the thing that changed. + */ test('deleting a parent task archives its linked subagent task', async ({ parentRemovalWindow: page, }) => { @@ -38,7 +45,23 @@ test('deleting a parent task archives its linked subagent task', async ({ await parentRow.hover(); await parentRow.getByRole('button', { name: '任务操作' }).click(); - await page.getByRole('menuitem', { name: '删除', exact: true }).click(); + // The rail's menu ends at 归档. Deleting is not one of the things a row can + // be asked to do here. + await expect(page.getByRole('menuitem', { name: '删除', exact: true })).toHaveCount(0); + await page.getByRole('menuitem', { name: '归档', exact: true }).click(); + await expect(parentRow).toHaveCount(0); + + await page.getByRole('button', { name: '设置', exact: true }).click(); + await expect(page.getByRole('main', { name: '设置内容' })).toBeVisible(); + await page.getByRole('button', { name: '已归档任务', exact: true }).click(); + const archivedTasks = page.getByRole('main', { name: '设置内容' }); + + await archivedTasks + .getByRole('button', { name: `「${PARENT_REMOVAL_PARENT_NAME}」的更多操作` }) + .click(); + // 彻底删除, not 删除: Settings names the irreversible verb in full, which is + // the point of routing every deletion through a surface reached by archiving. + await page.getByRole('menuitem', { name: '彻底删除', exact: true }).click(); const confirm = page.getByRole('alertdialog', { name: `删除 "${PARENT_REMOVAL_PARENT_NAME}"`, }); @@ -49,14 +72,6 @@ test('deleting a parent task archives its linked subagent task', async ({ await expect(confirm.getByText(/子任务.*归档/)).toBeVisible(); await confirm.getByRole('button', { name: '删除', exact: true }).click(); - await expect(parentRow).toHaveCount(0); - await expect(taskList.getByText(PARENT_REMOVAL_CHILD_NAME, { exact: true })).toHaveCount(0); - - await page.getByRole('button', { name: '设置', exact: true }).click(); - await expect(page.getByRole('main', { name: '设置内容' })).toBeVisible(); - await page.getByRole('button', { name: '已归档任务', exact: true }).click(); - - const archivedTasks = page.getByRole('main', { name: '设置内容' }); await expect(archivedTasks.getByText(PARENT_REMOVAL_CHILD_NAME, { exact: true })).toBeVisible(); await expect(archivedTasks.getByText(/原父任务已删除/)).toBeVisible(); await expect(archivedTasks.getByText(PARENT_REMOVAL_PARENT_NAME, { exact: true })).toHaveCount(0); diff --git a/apps/desktop/src/main/__tests__/session-navigation-selection.test.ts b/apps/desktop/src/main/__tests__/session-navigation-selection.test.ts index c260776e8a..322f9c3c55 100644 --- a/apps/desktop/src/main/__tests__/session-navigation-selection.test.ts +++ b/apps/desktop/src/main/__tests__/session-navigation-selection.test.ts @@ -19,101 +19,135 @@ import assert from 'node:assert/strict'; import { describe, test } from 'node:test'; +import type { SessionRowPick } from '@maka/ui'; import { EMPTY_SESSION_SELECTION, - enterSessionSelection, - exitSessionSelection, + pickSessionRow, pruneSessionSelection, - sessionSelectionMasterState, - setAllSessionsSelected, type SessionSelection, } from '../../renderer/features/session-navigation/testing.js'; -const GROUP = ['a', 'b', 'c', 'd', 'e']; +const ORDER = ['a', 'b', 'c', 'd', 'e']; function ids(selection: SessionSelection): string[] { return [...selection.selectedIds].sort(); } -/** What a row's checkbox does, as the hook applies it. */ -function mark(selection: SessionSelection, sessionId: string): SessionSelection { - return { - active: true, - selectedIds: new Set([...selection.selectedIds, sessionId]), - }; +/** One click on a row, as the rail's handler passes it down. */ +function click( + selection: SessionSelection, + sessionId: string, + pick: SessionRowPick, + openSessionId?: string, +): SessionSelection { + return pickSessionRow(selection, { + sessionId, + pick, + orderedSessionIds: ORDER, + openSessionId, + }); } -describe('selection mode', () => { - test('entering marks nothing on its own', () => { - const entered = enterSessionSelection(EMPTY_SESSION_SELECTION); - assert.equal(entered.active, true); - assert.deepEqual(ids(entered), []); +describe('a plain click', () => { + test('makes the set exactly this row', () => { + const from = click(EMPTY_SESSION_SELECTION, 'a', 'replace'); + assert.deepEqual(ids(click(from, 'd', 'replace')), ['d']); + }); + + test('leaves the anchor on the row that was clicked', () => { + // Which is what lets the very next Shift-click reach from here, without a + // separate gesture to say where a range starts. + const anchored = click(EMPTY_SESSION_SELECTION, 'b', 'replace'); + assert.deepEqual(ids(click(anchored, 'd', 'range')), ['b', 'c', 'd']); }); +}); - test('leaving drops the mode and the marks together', () => { - assert.equal(exitSessionSelection().active, false); - assert.deepEqual(ids(exitSessionSelection()), []); +describe('⌘-click', () => { + test('adds a row without disturbing the rest', () => { + const one = click(EMPTY_SESSION_SELECTION, 'a', 'replace'); + assert.deepEqual(ids(click(one, 'd', 'toggle')), ['a', 'd']); }); - test('unticking every row is select-none, not leave', () => { - // A mode that ended itself on the last untick would take the checkboxes - // away mid-gesture, and one mis-click would cost the user the way back. - const all = setAllSessionsSelected(EMPTY_SESSION_SELECTION, GROUP, true); - const none = setAllSessionsSelected(all, GROUP, false); - assert.deepEqual(ids(none), []); - assert.equal(none.active, true); + test('removes a row it finds already picked', () => { + const two = click(click(EMPTY_SESSION_SELECTION, 'a', 'replace'), 'd', 'toggle'); + assert.deepEqual(ids(click(two, 'a', 'toggle')), ['d']); }); - test('an emptied selection keeps the mode it was in', () => { - // It used to settle on the shared EMPTY value, which also carries - // `active: false` — so a catalog change that pruned the last row would have - // taken the checkboxes away while the user was still selecting. - const pruned = pruneSessionSelection(mark(EMPTY_SESSION_SELECTION, 'a'), []); - assert.deepEqual(ids(pruned), []); - assert.equal(pruned.active, true); + test('moves the anchor, including when it unpicked the row', () => { + // The anchor is "where the last non-Shift click landed", not "the last row + // added": a person who unticks a row and then Shift-clicks means the run + // between those two clicks, whatever the first one did to the set. + const removed = click(click(EMPTY_SESSION_SELECTION, 'b', 'replace'), 'b', 'toggle'); + assert.deepEqual(ids(removed), []); + assert.deepEqual(ids(click(removed, 'd', 'range')), ['b', 'c', 'd']); }); }); -describe('the master box', () => { - test('marks exactly the rows the rail is listing', () => { - // Not every task in the catalog: the box sits above these rows, and a - // selection that reached past them would name a number nobody agreed to. - assert.deepEqual(ids(setAllSessionsSelected(EMPTY_SESSION_SELECTION, ['a', 'b'], true)), [ - 'a', - 'b', - ]); +describe('Shift-click', () => { + test('picks the run between the anchor and the row, in either direction', () => { + const anchored = click(EMPTY_SESSION_SELECTION, 'd', 'replace'); + assert.deepEqual(ids(click(anchored, 'b', 'range')), ['b', 'c', 'd']); + }); + + test('keeps the anchor so the run can be re-dragged from the same origin', () => { + // Shortening a range is the same gesture as lengthening it. An anchor that + // moved to the last row touched would make the second Shift-click reach + // from the end of the first one, and the run would only ever grow. + const anchored = click(EMPTY_SESSION_SELECTION, 'b', 'replace'); + const long = click(anchored, 'e', 'range'); + assert.deepEqual(ids(click(long, 'c', 'range')), ['b', 'c']); }); - test('reads unchecked, indeterminate, then checked', () => { - assert.equal(sessionSelectionMasterState(EMPTY_SESSION_SELECTION, GROUP), false); - assert.equal(sessionSelectionMasterState(mark(EMPTY_SESSION_SELECTION, 'b'), GROUP), 'indeterminate'); - assert.equal( - sessionSelectionMasterState(setAllSessionsSelected(EMPTY_SESSION_SELECTION, GROUP, true), GROUP), - true, - ); + test('starts from the open task when no click has set an anchor', () => { + // The rail is a navigation surface first: a person who has been reading a + // task and Shift-clicks another means the two of them and everything + // between, without a preparatory click to say so. + assert.deepEqual(ids(click(EMPTY_SESSION_SELECTION, 'd', 'range', 'b')), ['b', 'c', 'd']); }); - test('an empty list is unchecked, never checked', () => { - // `every` over an empty array is vacuously true, which would tick the box - // above no rows at all. - assert.equal(sessionSelectionMasterState(EMPTY_SESSION_SELECTION, []), false); + test('with nothing to reach from is just this row', () => { + assert.deepEqual(ids(click(EMPTY_SESSION_SELECTION, 'd', 'range')), ['d']); }); - test('a mark outside the listed rows does not make it checked', () => { - assert.equal(sessionSelectionMasterState(mark(EMPTY_SESSION_SELECTION, 'zzz'), GROUP), 'indeterminate'); + test('reaching for a row the list is not showing picks only the row clicked', () => { + const stale = pickSessionRow(EMPTY_SESSION_SELECTION, { + sessionId: 'd', + pick: 'range', + orderedSessionIds: ORDER, + openSessionId: 'gone', + }); + assert.deepEqual(ids(stale), ['d']); + }); +}); + +describe('clearing', () => { + test('drops the picks and the anchor together', () => { + const cleared = EMPTY_SESSION_SELECTION; + assert.deepEqual(ids(cleared), []); + // With no anchor left, the next range starts from whatever is open — + // which is where the user's attention already is. + assert.deepEqual(ids(click(cleared, 'c', 'range', 'a')), ['a', 'b', 'c']); }); }); describe('pruneSessionSelection', () => { test('drops ids the catalog no longer lists', () => { - const selection = setAllSessionsSelected(EMPTY_SESSION_SELECTION, ['a', 'b'], true); - assert.deepEqual(ids(pruneSessionSelection(selection, ['a'])), ['a']); + const two = click(click(EMPTY_SESSION_SELECTION, 'a', 'replace'), 'b', 'toggle'); + assert.deepEqual(ids(pruneSessionSelection(two, ['a'])), ['a']); + }); + + test('drops an anchor that went with them', () => { + // A range from a row that is gone would reach across the rows that took + // its place, which is not the run anybody drew. + const anchored = click(EMPTY_SESSION_SELECTION, 'b', 'replace'); + const pruned = pruneSessionSelection(anchored, ['a', 'c', 'd', 'e']); + assert.deepEqual(ids(click(pruned, 'd', 'range', 'c')), ['c', 'd']); }); test('returns the same value when nothing was dropped', () => { // Identity matters here: this runs on every catalog refresh, and a new Set // each time would re-render every row of the rail. - const selection = mark(EMPTY_SESSION_SELECTION, 'a'); + const selection = click(EMPTY_SESSION_SELECTION, 'a', 'replace'); assert.equal(pruneSessionSelection(selection, ['a', 'b']), selection); }); }); diff --git a/apps/desktop/src/main/__tests__/session-navigation-session-purge.test.ts b/apps/desktop/src/main/__tests__/session-navigation-session-purge.test.ts index c9bdeb51d4..83a5f7099b 100644 --- a/apps/desktop/src/main/__tests__/session-navigation-session-purge.test.ts +++ b/apps/desktop/src/main/__tests__/session-navigation-session-purge.test.ts @@ -445,264 +445,3 @@ describe('deleteSession', () => { assert.deepEqual(h.toasts, ['rescued was restored, so it was kept']); }); }); - -describe('deleteSessions', () => { - it('reads the archived premise per task instead of asserting it', async () => { - const h = harness(); - // The rail lists unarchived tasks. Asserting the archived premise for them - // — which is what the Settings purge does — would make the Host refuse - // every deletion the rail can ask for. - const sessions = [summary('live', { isArchived: false }), summary('filed')]; - const activeIdRef = { current: undefined as string | undefined }; - const service = installService(h, { catalog: sessions }); - const actions = createActions({ harness: h, sessions, activeIdRef, service }); - - const outcome = await actions.deleteSessions(['live', 'filed']); - - assert.deepEqual(h.removed, ['live', 'filed']); - assert.deepEqual(h.removeOptions, [ - ['live', false], - ['filed', true], - ]); - assert.equal(outcome.removed, 2); - assert.deepEqual(outcome.remaining, []); - assert.equal(outcome.verified, true); - }); - - it('accounts for a rejection against the catalog, exactly as the purge does', async () => { - const h = harness(); - const sessions = [summary('a', { isArchived: false }), summary('b', { isArchived: false })]; - const activeIdRef = { current: undefined as string | undefined }; - // 'b' rejects but is gone from the catalog anyway: the delete IPC commits - // the removal before it releases renderer resources, so a rejection is not - // evidence the task survived. - const service = installService(h, { rejectIds: ['b'], surviving: [] }); - const actions = createActions({ harness: h, sessions, activeIdRef, service }); - - const outcome = await actions.deleteSessions(['a', 'b']); - - assert.equal(outcome.removed, 2); - assert.deepEqual(outcome.remaining, []); - assert.equal(outcome.verified, true); - assert.ok(outcome.firstFailure); - assert.equal((outcome.firstFailure.error as Error).message, 'busy:b'); - }); - - it('claims nothing when the catalog cannot be read back', async () => { - const h = harness(); - const sessions = [summary('a', { isArchived: false })]; - const activeIdRef = { current: undefined as string | undefined }; - const service = installService(h, { rejectIds: ['a'] }); - const actions = createActions({ harness: h, sessions, activeIdRef, service }); - - const outcome = await actions.deleteSessions(['a']); - - assert.equal(outcome.verified, false); - assert.equal(outcome.removed, 0); - assert.deepEqual(outcome.remaining, []); - }); -}); - -describe('archiveSessions', () => { - it('archives the set, clears each family, and refreshes once', async () => { - const h = harness(); - const sessions = [ - summary('a', { isArchived: false }), - summary('a-v2', { - isArchived: false, - revisionRootSessionId: 'a', - revisionParentSessionId: 'a', - }), - summary('b', { isArchived: false }), - ]; - const activeIdRef = { current: 'a-v2' as string | undefined }; - let refreshes = 0; - const actions = createSessionNavigationRowActions({ - uiLocale: 'en', - activeIdRef, - clearActiveMessages: () => undefined, - clearSessionRendererState: (id) => { - h.cleared.push(id); - }, - pendingSessionRowActionsRef: { current: new Set() }, - refreshSessions: async () => { - refreshes += 1; - return []; - }, - service: installService(h), - sessionsRef: { current: sessions }, - setActiveId: (id) => { - h.selections.push(id); - activeIdRef.current = id; - }, - toastApi: { - success: (title: string) => { - h.toasts.push(title); - }, - error: () => undefined, - confirm: async () => true, - }, - }); - - const outcome = await actions.archiveSessions(['a-v2', 'b']); - - assert.deepEqual(h.archived, ['a-v2', 'b']); - assert.deepEqual(outcome, { archived: 2, failed: [], firstFailure: undefined }); - // The whole revision family leaves the rail, so the renderer drops all of - // it — and the active task went with it. - assert.deepEqual(h.cleared, ['a', 'a-v2', 'b']); - assert.deepEqual(h.selections, [undefined]); - // Once for the sweep, not once per task: a refresh per id re-renders the - // rail under the cursor for every row in the selection. - assert.equal(refreshes, 1); - }); - - it('keeps going past a failure and names the first one', async () => { - const h = harness(); - const sessions = ['a', 'b', 'c'].map((id) => summary(id, { isArchived: false })); - const activeIdRef = { current: undefined as string | undefined }; - const service = installService(h, { rejectArchiveIds: ['b'] }); - const actions = createActions({ harness: h, sessions, activeIdRef, service }); - - const outcome = await actions.archiveSessions(['a', 'b', 'c']); - - // 'c' is still attempted: one Host refusing is not the sweep's answer for - // every task in it. - assert.deepEqual(h.archived, ['a', 'b', 'c']); - assert.equal(outcome.archived, 2); - assert.deepEqual(outcome.failed, ['b']); - assert.ok(outcome.firstFailure); - assert.equal((outcome.firstFailure.error as Error).message, 'archive-busy:b'); - assert.equal(outcome.firstFailure?.sessionId, 'b'); - // No per-task toast: the caller phrases one message for the sweep. - assert.deepEqual(h.toasts, []); - }); - - it('skips a task whose row action is already in flight', async () => { - const h = harness(); - const sessions = [summary('a', { isArchived: false }), summary('b', { isArchived: false })]; - const activeIdRef = { current: undefined as string | undefined }; - const service = installService(h); - const actions = createActions({ - harness: h, - sessions, - activeIdRef, - pending: new Set(['a:rename']), - service, - }); - - const outcome = await actions.archiveSessions(['a', 'b']); - - assert.deepEqual(h.archived, ['b']); - // Reported rather than silently dropped: the count the caller shows has to - // add up to the number of tasks the user selected. - assert.deepEqual(outcome.failed, ['a']); - assert.equal(outcome.archived, 1); - }); -}); - -describe('deleteSelected', () => { - it('warns that linked subtasks will be archived, and reports what was', async () => { - // The Host archives a deleted parent's ordinary subagent tasks rather than - // deleting them. Without the warning they reappear under Archived with no - // explanation; without the report the user never learns how many did. - const h = harness(); - const sessions = ['a', 'b'].map((id) => summary(id, { isArchived: false })); - const activeIdRef = { current: undefined as string | undefined }; - const service = installService(h, { - previewSubtasks: { a: 2 }, - archivedByRemoval: { a: 2, b: 1 }, - }); - const confirms: Array<{ description: string }> = []; - const actions = createActions({ - harness: h, - sessions, - activeIdRef, - service, - onConfirm: (options) => { - confirms.push({ description: options.description }); - return true; - }, - }); - - await actions.deleteSelected(['a', 'b']); - - // One preview per selected task: the renderer's projection cannot answer - // this, so it is asked for every id the confirm is about to name. - assert.deepEqual(h.previews, ['a', 'b']); - assert.equal(confirms.length, 1); - assert.match(confirms[0]!.description, /moved to Archived/); - assert.deepEqual(h.toasts, ['Deleted 2 tasks']); - assert.deepEqual(h.toastDescriptions, ['3 subtasks moved to Archived']); - }); - - it('says the subtask warning is uncertain when one preview cannot be read', async () => { - // Under-reporting a destructive set is worse than admitting the number is - // unknown: a silent zero reads as "nothing else will move". - const h = harness(); - const sessions = ['a', 'b'].map((id) => summary(id, { isArchived: false })); - const activeIdRef = { current: undefined as string | undefined }; - const service = installService(h, { - previewSubtasks: { a: 2 }, - rejectPreviewIds: ['b'], - }); - const confirms: string[] = []; - const actions = createActions({ - harness: h, - sessions, - activeIdRef, - service, - onConfirm: (options) => { - confirms.push(options.description); - return true; - }, - }); - - await actions.deleteSelected(['a', 'b']); - - assert.equal(confirms.length, 1); - assert.match(confirms[0]!, /Any ordinary subtasks/); - }); - - it('says nothing about subtasks when the selection has none', async () => { - const h = harness(); - const sessions = [summary('a', { isArchived: false })]; - const activeIdRef = { current: undefined as string | undefined }; - const service = installService(h); - const confirms: string[] = []; - const actions = createActions({ - harness: h, - sessions, - activeIdRef, - service, - onConfirm: (options) => { - confirms.push(options.description); - return true; - }, - }); - - await actions.deleteSelected(['a']); - - assert.doesNotMatch(confirms[0]!, /Archived/); - assert.deepEqual(h.toastDescriptions, [undefined]); - }); - - it('does nothing when the confirm is declined', async () => { - const h = harness(); - const sessions = [summary('a', { isArchived: false })]; - const activeIdRef = { current: undefined as string | undefined }; - const service = installService(h); - const actions = createActions({ - harness: h, - sessions, - activeIdRef, - service, - onConfirm: () => false, - }); - - await actions.deleteSelected(['a']); - - assert.deepEqual(h.removed, []); - assert.deepEqual(h.toasts, []); - }); -}); diff --git a/apps/desktop/src/main/__tests__/session-selection-open-elsewhere.test.ts b/apps/desktop/src/main/__tests__/session-selection-open-elsewhere.test.ts new file mode 100644 index 0000000000..46cde50f20 --- /dev/null +++ b/apps/desktop/src/main/__tests__/session-selection-open-elsewhere.test.ts @@ -0,0 +1,143 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * ⌘K, the Module Hub and 新建任务 open a task without going through a row, so + * the rail hears about it only as a new active id. The picked ground and the + * open ground are the same ground, so a set left over from the task the user + * walked away from would read as a set around the one they are now reading — + * and the next ⋯ would sweep it. + * + * These drive the real hook rather than the reducer: what has to hold is that + * the open task and the picked set agree, and only the hook holds both. + */ + +import assert from 'node:assert/strict'; +import { afterEach, test } from 'node:test'; +import { act, createElement, useState, type ReactNode } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { parseHTML } from 'linkedom'; +import type { SessionSummary } from '@maka/core/session'; +import type { SessionRailSelection } from '@maka/ui'; +import { useSessionSelection } from '../../renderer/features/session-navigation/testing.js'; +import type { SessionNavigationRowActions } from '../../renderer/features/session-navigation/testing.js'; + +const originalGlobals = { + document: globalThis.document, + window: globalThis.window, + IS_REACT_ACT_ENVIRONMENT: (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }) + .IS_REACT_ACT_ENVIRONMENT, +}; + +let mountedRoot: Root | undefined; + +afterEach(async () => { + if (mountedRoot) await act(() => mountedRoot?.unmount()); + mountedRoot = undefined; + Object.assign(globalThis, originalGlobals); +}); + +const ORDER = ['a', 'b', 'c']; + +function summary(id: string): SessionSummary { + return { + id, + name: id, + isFlagged: false, + isArchived: false, + labels: [], + hasUnread: false, + status: 'active', + backend: 'fake', + llmConnectionSlug: 'test', + connectionLocked: true, + model: 'test', + permissionMode: 'ask', + }; +} + +/** A ⌘-click on a row, which is how a set is built without opening anything. */ +function toggle(selection: SessionRailSelection, sessionId: string): void { + selection.commands.pick({ sessionId, pick: 'toggle', orderedSessionIds: ORDER }); +} + +async function mountSelection(openId: string): Promise<{ + latest(): SessionRailSelection; + /** A task opened from outside the rail: a new active id, and nothing else. */ + open(sessionId: string): Promise; +}> { + const { document, window } = parseHTML('
'); + Object.assign(globalThis, { document, window, IS_REACT_ACT_ENVIRONMENT: true }); + + const commands = {} as unknown as SessionNavigationRowActions; + let latest: SessionRailSelection | undefined; + let setActiveId: ((sessionId: string) => void) | undefined; + function Probe(): ReactNode { + const [activeId, setActive] = useState(openId); + setActiveId = setActive; + latest = useSessionSelection({ sessions: ORDER.map(summary), commands, activeId }); + return null; + } + + const container = document.querySelector('#root'); + assert.ok(container); + const root = createRoot(container); + mountedRoot = root; + await act(() => root.render(createElement(Probe))); + return { + latest: () => { + assert.ok(latest); + return latest; + }, + open: async (sessionId) => { + await act(() => { + setActiveId?.(sessionId); + }); + }, + }; +} + +test('opening a task the set does not hold drops the picks', async () => { + const probe = await mountSelection('a'); + await act(() => toggle(probe.latest(), 'a')); + await act(() => toggle(probe.latest(), 'b')); + assert.deepEqual([...probe.latest().selectedIds].sort(), ['a', 'b']); + + // ⌘K opens C. Nothing in the rail was clicked. + await probe.open('c'); + + // Empty, not {c}: the rail paints the open row regardless, so an empty set + // already reads as "just this row". + assert.deepEqual([...probe.latest().selectedIds], []); +}); + +test('opening a task the set already holds leaves it alone', async () => { + // ⌘K again, and again nothing in the rail was clicked — the contrast with the + // test above is the membership, not the way the task was opened. Every row on + // the picked ground is still genuinely picked, so there is nothing to drop. + // A plain click inside the rail lands here too, having already replaced the + // set with the row it opened by the time the active id moves. + const probe = await mountSelection('a'); + await act(() => toggle(probe.latest(), 'a')); + await act(() => toggle(probe.latest(), 'b')); + + await probe.open('b'); + + assert.deepEqual([...probe.latest().selectedIds].sort(), ['a', 'b']); +}); diff --git a/apps/desktop/src/main/__tests__/session-selection-sweep-race.test.ts b/apps/desktop/src/main/__tests__/session-selection-sweep-race.test.ts index d3d8dd8dca..f53e1bdfbe 100644 --- a/apps/desktop/src/main/__tests__/session-selection-sweep-race.test.ts +++ b/apps/desktop/src/main/__tests__/session-selection-sweep-race.test.ts @@ -18,16 +18,21 @@ */ /** - * `Done` stays enabled while a sweep runs — leaving asks nothing of the Host — - * so a person can leave the selection, re-enter it from another row's menu, and - * mark a different task before the first request settles. What the sweep - * unmarks on completion has to be the set it asked about, not whatever happens - * to be marked when it lands. + * Nothing blocks the rail while a sweep is with the Host — there is no mode to + * be held in — so a person can go on picking rows before the first request + * settles. + * + * The selection follows the CATALOG, and nothing else: rows leave the set by + * leaving the rail. A sweep that also unmarked what it swept would be that rule + * stated twice — right for archive, which removes the rows, and wrong for pin, + * which leaves them exactly where they are. So these cases drive the catalog, + * not just the hook: a fixture whose session list never changes cannot tell the + * two apart. */ import assert from 'node:assert/strict'; import { afterEach, test } from 'node:test'; -import { act, createElement, type ReactNode } from 'react'; +import { act, createElement, useState, type ReactNode } from 'react'; import { createRoot, type Root } from 'react-dom/client'; import { parseHTML } from 'linkedom'; import type { SessionSummary } from '@maka/core/session'; @@ -67,10 +72,51 @@ function summary(id: string): SessionSummary { }; } -test('a settled sweep unmarks what it asked about, not what is marked now', async () => { +const ORDER = ['a', 'b']; + +/** A ⌘-click on a row, which is how a set is built without opening anything. */ +function toggle(selection: SessionRailSelection | undefined, sessionId: string): void { + selection?.commands.pick({ sessionId, pick: 'toggle', orderedSessionIds: ORDER }); +} + +async function mountSelection(commands: SessionNavigationRowActions): Promise<{ + latest(): SessionRailSelection; + /** What the rail lists, as a refresh would leave it. */ + relist(sessionIds: readonly string[]): Promise; +}> { const { document, window } = parseHTML('
'); Object.assign(globalThis, { document, window, IS_REACT_ACT_ENVIRONMENT: true }); + let latest: SessionRailSelection | undefined; + let listed: readonly string[] = ORDER; + let rerender: (() => void) | undefined; + function Probe(): ReactNode { + const [, bump] = useState(0); + rerender = () => bump((count) => count + 1); + latest = useSessionSelection({ sessions: listed.map(summary), commands }); + return null; + } + + const container = document.querySelector('#root'); + assert.ok(container); + const root = createRoot(container); + mountedRoot = root; + await act(() => root.render(createElement(Probe))); + return { + latest: () => { + assert.ok(latest); + return latest; + }, + relist: async (sessionIds) => { + listed = sessionIds; + await act(async () => { + rerender?.(); + }); + }, + }; +} + +test('an archive drops its rows by way of the catalog, keeping later picks', async () => { let releaseArchive: (() => void) | undefined; const archived: string[][] = []; const commands = { @@ -80,91 +126,98 @@ test('a settled sweep unmarks what it asked about, not what is marked now', asyn releaseArchive = resolve; }); }, - deleteSelected: async () => undefined, } as unknown as SessionNavigationRowActions; - let latest: SessionRailSelection | undefined; - function Probe(): ReactNode { - const { selection } = useSessionSelection({ - sessions: [summary('a'), summary('b')], - commands, - }); - latest = selection; - return null; - } - - const container = document.querySelector('#root'); - assert.ok(container); - const root = createRoot(container); - mountedRoot = root; - await act(() => root.render(createElement(Probe))); - assert.ok(latest); + const probe = await mountSelection(commands); - // Mark A and start the sweep. It does not settle yet. - await act(() => latest?.onEnter('a')); + // Pick A and start the sweep. It does not settle yet. + await act(() => toggle(probe.latest(), 'a')); await act(() => { - void latest?.onArchiveSelected(); + void probe.latest().commands.archiveSelected(); }); assert.deepEqual(archived, [['a']]); - // Leave the mode and come back on a different row, exactly as the rail - // allows while a sweep is in flight. - await act(() => latest?.onExit()); - await act(() => latest?.onEnter('b')); - assert.deepEqual([...(latest?.selectedIds ?? [])], ['b']); + // Go on picking, exactly as the rail allows while a sweep is in flight. + await act(() => toggle(probe.latest(), 'b')); + assert.deepEqual([...probe.latest().selectedIds].sort(), ['a', 'b']); - // Now the first request lands. + // The request lands, and the refresh it awaited takes A off the rail. await act(async () => { releaseArchive?.(); await Promise.resolve(); }); + await probe.relist(['b']); - // B survives. Clearing the whole set here would answer A's completion by - // discarding a selection the user made afterwards and never submitted. - assert.deepEqual([...(latest?.selectedIds ?? [])], ['b']); - assert.equal(latest?.active, true); + // B survives. Answering A's completion by clearing the set would discard a + // pick the user made afterwards and never submitted. + assert.deepEqual([...probe.latest().selectedIds], ['b']); }); -test('a settled sweep does unmark its own set when nothing else happened', async () => { - const { document, window } = parseHTML('
'); - Object.assign(globalThis, { document, window, IS_REACT_ACT_ENVIRONMENT: true }); +test('a pin sweep leaves the set exactly as it was', async () => { + // Pinning does not remove a row, it moves it into the pinned group. A set + // that emptied itself here would make "pin these, then archive them" two + // selections instead of one, which is the whole point of holding a set. + const asked: Array<{ sessionIds: string[]; flagged: boolean }> = []; + const commands = { + flagSelected: async (sessionIds: readonly string[], flagged: boolean) => { + asked.push({ sessionIds: [...sessionIds], flagged }); + }, + } as unknown as SessionNavigationRowActions; + const probe = await mountSelection(commands); + await act(() => toggle(probe.latest(), 'a')); + await act(() => toggle(probe.latest(), 'b')); + await act(async () => { + await probe.latest().commands.flagSelected(true); + }); + + assert.deepEqual(asked, [{ sessionIds: ['a', 'b'], flagged: true }]); + assert.deepEqual([...probe.latest().selectedIds].sort(), ['a', 'b']); +}); + +test('a second sweep is refused while the first is still out', async () => { + // Two archive requests over overlapping sets would report two counts for one + // set of tasks, and the second would name rows the first has already taken. let releaseArchive: (() => void) | undefined; + const asked: string[][] = []; const commands = { - archiveSelected: () => - new Promise((resolve) => { + archiveSelected: (sessionIds: readonly string[]) => { + asked.push([...sessionIds]); + return new Promise((resolve) => { releaseArchive = resolve; - }), - deleteSelected: async () => undefined, + }); + }, } as unknown as SessionNavigationRowActions; - let latest: SessionRailSelection | undefined; - function Probe(): ReactNode { - const { selection } = useSessionSelection({ - sessions: [summary('a'), summary('b')], - commands, - }); - latest = selection; - return null; - } - - const container = document.querySelector('#root'); - assert.ok(container); - const root = createRoot(container); - mountedRoot = root; - await act(() => root.render(createElement(Probe))); - - await act(() => latest?.onEnter('a')); + const probe = await mountSelection(commands); + await act(() => toggle(probe.latest(), 'a')); + await act(() => { + void probe.latest().commands.archiveSelected(); + }); await act(() => { - void latest?.onArchiveSelected(); + void probe.latest().commands.archiveSelected(); }); + assert.deepEqual(asked, [['a']]); + await act(async () => { releaseArchive?.(); await Promise.resolve(); }); + await probe.relist(['b']); + assert.deepEqual([...probe.latest().selectedIds], []); +}); + +test('a sweep over nothing asks nothing', async () => { + const asked: string[][] = []; + const commands = { + flagSelected: async (sessionIds: readonly string[]) => { + asked.push([...sessionIds]); + }, + } as unknown as SessionNavigationRowActions; - assert.deepEqual([...(latest?.selectedIds ?? [])], []); - // The mode stays on: the person was tidying up, and taking the checkboxes - // away after each sweep would make them re-enter for the next one. - assert.equal(latest?.active, true); + const probe = await mountSelection(commands); + await act(async () => { + await probe.latest().commands.flagSelected(true); + }); + assert.deepEqual(asked, []); }); diff --git a/apps/desktop/src/renderer/features/session-navigation/controller/session-row-actions.ts b/apps/desktop/src/renderer/features/session-navigation/controller/session-row-actions.ts index 06fb83f931..8303b9f2c1 100644 --- a/apps/desktop/src/renderer/features/session-navigation/controller/session-row-actions.ts +++ b/apps/desktop/src/renderer/features/session-navigation/controller/session-row-actions.ts @@ -86,8 +86,11 @@ export interface SessionPurgeOutcome { /** * What a bulk archive can honestly say afterwards. There is no third * disposition: a task is archived or its call failed. + * + * Not on `SessionNavigationRowActions`: `archiveSelected` is the rail's whole + * bulk archive, and this is what it reads on the way to its own report. */ -export interface SessionArchiveOutcome { +interface SessionArchiveOutcome { archived: number; /** Tasks the sweep could not archive, including ones it had to skip. */ failed: string[]; @@ -105,11 +108,10 @@ export interface SessionNavigationRowActions { renameSession(sessionId: string, name: string): Promise; deleteSession(sessionId: string): Promise; purgeSessions(sessionIds: readonly string[]): Promise; - deleteSessions(sessionIds: readonly string[]): Promise; - archiveSessions(sessionIds: readonly string[]): Promise; - /** Confirms, sweeps, and reports — the rail's own wording. */ + /** Sweeps and reports — the rail's own wording. */ archiveSelected(sessionIds: readonly string[]): Promise; - deleteSelected(sessionIds: readonly string[]): Promise; + /** Pins or unpins a picked set in one sweep. */ + flagSelected(sessionIds: readonly string[], flagged: boolean): Promise; } export function createSessionNavigationRowActions(deps: { @@ -274,12 +276,10 @@ export function createSessionNavigationRowActions(deps: { } /** - * Deletes a set of tasks in one sweep. - * - * `requireArchivedFor` decides, per id, whether the deletion asserts that the - * task is still archived. Settings' purge asserts it for every target; the - * rail reads it off the task, exactly as single-row delete does, because the - * rail lists unarchived tasks and asserting it there would refuse them all. + * Settings › 已归档任务, sweeping a set of archived tasks in one pass. The one + * bulk delete the product has: the rail cannot delete at all, so every target + * here is archived by definition, and the premise is asserted anyway so a task + * restored between the confirm and the write is kept rather than removed. * * Every id takes one path and lands in exactly one outcome. A task whose * premise still holds is removed; one restored meanwhile answers `restored` @@ -302,10 +302,7 @@ export function createSessionNavigationRowActions(deps: { * No confirm and no toast: the caller owns the wording for a sweep, which is * the one thing single-row delete cannot phrase. */ - async function sweepSessions( - sessionIds: readonly string[], - requireArchivedFor: (sessionId: string) => boolean, - ): Promise { + async function purgeSessions(sessionIds: readonly string[]): Promise { const unsettled: string[] = []; const restored: string[] = []; let firstFailure: SessionPurgeOutcome['firstFailure']; @@ -324,7 +321,7 @@ export function createSessionNavigationRowActions(deps: { pendingSessionRowActionsRef.current.add(key); try { const { disposition, archivedSubtaskCount } = await removeSessionFamily(sessionId, { - requireArchived: requireArchivedFor(sessionId), + requireArchived: true, }); if (disposition === 'restored') restored.push(sessionId); else { @@ -378,32 +375,6 @@ export function createSessionNavigationRowActions(deps: { }; } - /** - * Settings › archived tasks. Every target is archived by definition, and the - * premise is asserted anyway so a task restored between the confirm and the - * write is kept rather than removed. - */ - async function purgeSessions(sessionIds: readonly string[]): Promise { - return sweepSessions(sessionIds, () => true); - } - - /** - * The rail's multi-select delete. The rail lists unarchived tasks, so the - * archived premise is read per task exactly as single-row delete reads it: - * asserting it for a task that was never archived would refuse every - * deletion the rail can actually ask for. - * - * No confirm here either — one sweep is one question, and only the caller - * knows how many tasks it is about to name. - */ - async function deleteSessions(sessionIds: readonly string[]): Promise { - return sweepSessions( - sessionIds, - (sessionId) => - sessionsRef.current.find((entry) => entry.id === sessionId)?.isArchived === true, - ); - } - /** * The rail's multi-select archive. * @@ -455,24 +426,23 @@ export function createSessionNavigationRowActions(deps: { /** * The rail's own bulk archive, wording included. * - * The sweeps below it stay silent on purpose — Settings' purge phrases its - * own confirm — but the rail's phrasing belongs to the rail, and this module - * is where the feature already holds its copy. Putting it in the selection - * hook instead would have made that hook the feature's second importer of - * renderer legacy copy, which the architecture check refuses. + * `archiveSessions` above it stays silent on purpose: it counts, and the + * caller words the count. The rail's phrasing belongs to the rail, and this + * module is where the feature already holds its copy. Putting it in the + * selection hook instead would have made that hook the feature's second + * importer of renderer legacy copy, which the architecture check refuses. + * + * NO CONFIRM, at one row or twenty. Archiving is reversible, the single-row + * ⋯ has never asked, and a dialog in front of one of two identical verbs + * teaches that the count is what makes an action dangerous rather than the + * action. The dialog's one piece of information — where the tasks went — is + * kept, as the success toast's description. */ async function archiveSelected(sessionIds: readonly string[]): Promise { if (sessionIds.length === 0) return; - const ok = await toastApi.confirm({ - title: copy.bulkArchiveTitle(sessionIds.length), - description: copy.bulkArchiveDescription, - confirmLabel: copy.bulkArchiveLabel, - cancelLabel: copy.cancelLabel, - }); - if (!ok) return; const outcome = await archiveSessions(sessionIds); if (outcome.failed.length === 0) { - toastApi.success(copy.bulkArchivedTitle(outcome.archived)); + toastApi.success(copy.bulkArchivedTitle(outcome.archived), copy.bulkArchiveDescription); return; } toastApi.error( @@ -486,72 +456,52 @@ export function createSessionNavigationRowActions(deps: { } /** - * The rail's own bulk delete. See `archiveSelected` for why the wording is here. + * The rail's bulk pin, in one direction for the whole set. * - * The confirm warns about linked subtasks for the same reason single-row - * delete does: the Host archives a deleted parent's ordinary subagent tasks - * rather than deleting them, so without the warning they reappear under - * Archived with no explanation. The Host owns that plan — the renderer's - * catalog projection lacks the operator marker — so the count is asked for, - * one preview per selected task, and a single failure makes the whole warning - * uncertain rather than silently under-reporting the set. + * The direction is the caller's: the menu shows 取消置顶 only when every + * picked row is already pinned, so a mixed set pins — which is the one rule + * that keeps a set-wide toggle from meaning something different for each row + * in it. * - * N previews before a destructive confirm is N round trips, which is the - * price of naming a number the user can act on. The toast afterwards reports - * the Host's executed total, not this estimate. + * Silent on success. Pinning moves the rows between 置顶 and 最近 in front of + * the user, which says it better than a toast, and the single-row pin has + * never raised one either. */ - async function deleteSelected(sessionIds: readonly string[]): Promise { + async function flagSelected(sessionIds: readonly string[], flagged: boolean): Promise { if (sessionIds.length === 0) return; - let previewedSubtasks: number | undefined = 0; + const failed: string[] = []; + let firstFailure: { error: unknown; sessionId: string } | undefined; for (const sessionId of sessionIds) { + const key = `${sessionId}:flag`; + if ( + Array.from(pendingSessionRowActionsRef.current).some((pending) => + pending.startsWith(`${sessionId}:`), + ) + ) { + failed.push(sessionId); + continue; + } + pendingSessionRowActionsRef.current.add(key); try { - const count = await service.previewRemoval(sessionId); - if (previewedSubtasks !== undefined) previewedSubtasks += count; - } catch { - previewedSubtasks = undefined; + await service.setFlagged(sessionId, flagged, { revisionFamily: true }); + } catch (error) { + failed.push(sessionId); + firstFailure ??= { error, sessionId }; + } finally { + pendingSessionRowActionsRef.current.delete(key); } } - const subtaskNote = - previewedSubtasks === undefined - ? copy.bulkDeleteSubtaskNoteUncertain() - : previewedSubtasks > 0 - ? copy.bulkDeleteSubtaskNote() - : undefined; - const ok = await toastApi.confirm({ - title: copy.bulkDeleteTitle(sessionIds.length), - description: subtaskNote - ? `${copy.bulkDeleteDescription} ${subtaskNote}` - : copy.bulkDeleteDescription, - confirmLabel: copy.deleteLabel, - cancelLabel: copy.cancelLabel, - destructive: true, - }); - if (!ok) return; - const outcome = await deleteSessions(sessionIds); - // Kept tasks and failures are independent, and reporting one while dropping - // the other is how a count quietly stops adding up. - const kept = - outcome.restored.length > 0 ? copy.bulkKeptRestored(outcome.restored.length) : undefined; - // The Host's executed number, not the preview's estimate. - const archived = - outcome.archivedSubtasks > 0 ? copy.deletedSubtaskNote(outcome.archivedSubtasks) : undefined; - if (outcome.verified && outcome.remaining.length === 0) { - toastApi.success( - copy.bulkDeletedTitle(outcome.removed), - [kept, archived].filter(Boolean).join(' ') || undefined, - ); - return; - } - const reason = !outcome.verified - ? copy.bulkUnverified - : outcome.firstFailure - ? localizedShellErrorMessage(outcome.firstFailure.error, copy.actionFallback, uiLocale) - : copy.bulkFailedBody(outcome.remaining.length); + // Once, after the whole sweep. Refreshing per task would re-render the rail + // under the user's cursor for every id in the set. + await refreshSessions(); + if (failed.length === 0) return; toastApi.error( - copy.bulkDeleteFailedTitle, - [reason, kept, archived].filter(Boolean).join(' '), + flagged ? copy.flagFailedTitle : copy.unflagFailedTitle, + firstFailure + ? localizedShellErrorMessage(firstFailure.error, copy.actionFallback, uiLocale) + : copy.bulkFailedBody(failed.length), undefined, - outcome.firstFailure ? { sessionId: outcome.firstFailure.sessionId } : undefined, + firstFailure ? { sessionId: firstFailure.sessionId } : undefined, ); } @@ -562,9 +512,7 @@ export function createSessionNavigationRowActions(deps: { renameSession, deleteSession, purgeSessions, - deleteSessions, - archiveSessions, archiveSelected, - deleteSelected, + flagSelected, }; } diff --git a/apps/desktop/src/renderer/features/session-navigation/controller/use-session-navigation-controller.ts b/apps/desktop/src/renderer/features/session-navigation/controller/use-session-navigation-controller.ts index 5b81001996..d676fc0222 100644 --- a/apps/desktop/src/renderer/features/session-navigation/controller/use-session-navigation-controller.ts +++ b/apps/desktop/src/renderer/features/session-navigation/controller/use-session-navigation-controller.ts @@ -24,7 +24,6 @@ import { runtimeHostProfileUsesHostWorkspace } from '@maka/runtime-host/profile- import { useUiLocale, type SessionHistoryGroup, - type SessionRailRowSelection, type SessionRailSelection, } from '@maka/ui'; import { useExternalStoreSelector } from '../../../use-external-store-selector.js'; @@ -106,8 +105,6 @@ export interface SessionNavigationController { selectors: SessionNavigationSelectors; commands: SessionNavigationRowActions; selection: SessionRailSelection; - /** The narrow half every row subscribes to; see `useSessionSelection`. */ - rowSelection: SessionRailRowSelection; } /** @@ -199,10 +196,14 @@ export function useSessionNavigationController( [groups, sessionMeta, worktreeSessionIds], ); - const { selection, rowSelection } = useSessionSelection({ sessions: rail.sessions, commands }); + const selection = useSessionSelection({ + sessions: rail.sessions, + commands, + activeId: rail.activeRowId, + }); return useMemo( - () => ({ layout, selectors, commands, selection, rowSelection }), - [commands, layout, rowSelection, selection, selectors], + () => ({ layout, selectors, commands, selection }), + [commands, layout, selection, selectors], ); } diff --git a/apps/desktop/src/renderer/features/session-navigation/controller/use-session-selection.ts b/apps/desktop/src/renderer/features/session-navigation/controller/use-session-selection.ts index 7521f49293..3d004f5162 100644 --- a/apps/desktop/src/renderer/features/session-navigation/controller/use-session-selection.ts +++ b/apps/desktop/src/renderer/features/session-navigation/controller/use-session-selection.ts @@ -19,31 +19,36 @@ import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'; import type { SessionSummary } from '@maka/core/session'; -import type { SessionRailRowSelection, SessionRailSelection } from '@maka/ui'; +import type { SessionRailSelection, SessionRailSelectionCommands } from '@maka/ui'; import { EMPTY_SESSION_SELECTION, - enterSessionSelection, - exitSessionSelection, + pickSessionRow, pruneSessionSelection, - setAllSessionsSelected, } from '../model/session-selection.js'; import type { SessionNavigationRowActions } from './session-row-actions.js'; /** - * The rail's multi-select: which rows are marked, and the two sweeps they feed. + * The rail's multi-select: which rows are picked, and the sweeps they feed. + * + * One state, and it is the whole feature. There is no mode flag beside it, no + * "what does all mean" list, and no second half for the rows — the rows are + * handed what they need as props, because the picked set now changes on an + * ordinary session switch and a context they subscribed to would redraw all of + * them for a switch that moved two (#4109). * * The selection is reconciled against the catalog on every change to it, not * only after a sweep. Another window deleting a task, a Host going away, or a * grouping change that drops rows all leave ids behind, and a count that - * includes them is a count that does not match what the confirm names. + * includes them is a count that does not match what the menu named. */ export function useSessionSelection(input: { sessions: readonly SessionSummary[]; commands: SessionNavigationRowActions; -}): { selection: SessionRailSelection; rowSelection: SessionRailRowSelection } { - const { sessions, commands } = input; + /** The open task, as the rail paints it. */ + activeId?: string; +}): SessionRailSelection { + const { sessions, commands, activeId } = input; const [selection, setSelection] = useState(EMPTY_SESSION_SELECTION); - const [busy, setBusy] = useState(false); const listedIds = useMemo(() => new Set(sessions.map((session) => session.id)), [sessions]); useEffect(() => { @@ -52,145 +57,113 @@ export function useSessionSelection(input: { setSelection((current) => pruneSessionSelection(current, listedIds)); }, [listedIds]); - // The sweep reads the ids at the moment it runs, and the state it reads is - // the one the confirm was built from — held in a ref so the callbacks below - // do not change identity per selection change and re-render the bar's buttons. + /** + * The set drops when the open task is not one of its members. + * + * A picked row and the open row are painted on the same ground, so a set that + * does not hold the open row can be read as a set around it, and the next ⋯ + * on a picked row sweeps something other than what the user was looking at. + * ⌘K, the Module Hub and 新建任务 all move the open task without touching the + * set, which is how that misreading gets built. + * + * Membership is the rule, not a stand-in for where the navigation came from. + * When the open row IS in the set, every row on that ground is genuinely + * picked, so nothing is misread and there is nothing to drop — the same + * answer whether the user clicked that row or reached it through ⌘K, and the + * verb still names its own count before it sweeps. Correlating the click with + * the active id would be a second account of the same question, and a lossier + * one: it would drop a set the user can plainly see is still theirs. + * + * Cleared, not `replace`d onto the new row: an empty selection ALREADY reads + * as "just the open row", because the rail paints that row whether or not it + * is picked (see `EMPTY_SESSION_SELECTION`). A set of one that happens to + * equal the open row would be a second way to say the same thing. + */ + useEffect(() => { + if (activeId === undefined) return; + setSelection((current) => + current.selectedIds.has(activeId) ? current : EMPTY_SESSION_SELECTION, + ); + }, [activeId]); + + // A sweep reads the ids at the moment it runs, and the state it reads is the + // one the menu's verb was counted from — held in a ref so the commands below + // never change identity, because every row carries them as a prop. const selectionRef = useRef(selection); - const busyRef = useRef(busy); + const busyRef = useRef(false); // Published on commit, not during render: a render React throws away must not // leave a ref pointing at a selection that was never shown. useLayoutEffect(() => { selectionRef.current = selection; - busyRef.current = busy; - listedRef.current = listedSessionIds; }); - const listedSessionIds = useMemo(() => sessions.map((session) => session.id), [sessions]); - const listedRef = useRef(listedSessionIds); - - const onToggleRow = useCallback((sessionId, selected) => { - setSelection((current) => { - if (current.selectedIds.has(sessionId) === selected) return current; - const selectedIds = new Set(current.selectedIds); - if (selected) selectedIds.add(sessionId); - else selectedIds.delete(sessionId); - return { active: true, selectedIds }; - }); + const pick = useCallback((request) => { + setSelection((current) => pickSessionRow(current, request)); }, []); - const onEnter = useCallback((sessionId) => { - setSelection((current) => { - const entered = enterSessionSelection(current); - if (sessionId === undefined) return entered; - return { active: true, selectedIds: new Set([...entered.selectedIds, sessionId]) }; - }); - }, []); + const clear = useCallback(() => setSelection(EMPTY_SESSION_SELECTION), []); - const onExit = useCallback(() => setSelection(exitSessionSelection()), []); - - const onToggleAll = useCallback((selected) => { - setSelection((current) => setAllSessionsSelected(current, listedRef.current, selected)); + /** + * The same reconciliation the catalog gets, against a narrower list: the rows + * the rail can see and act on right now. The menu asks for it as it opens, + * because a collapsed project keeps its rows mounted and a shared projection + * keeps its row listed — neither is gone from the catalog, and neither may be + * swept. `useCallback` because every row carries these commands as a prop + * (#4109), and `pruneSessionSelection` returns its input untouched when + * nothing was dropped, so the common case is not a state change at all. + */ + const retain = useCallback((sessionIds) => { + setSelection((current) => pruneSessionSelection(current, sessionIds)); }, []); /** * One sweep at a time, over the ids the user could see when they pressed. * * Frozen at the click for the reason the archived-task purge freezes its own - * set: a confirm names a number to a person, and a set re-read after the - * dialog can be a different one. The confirm, the sweep and the report all - * live in `session-row-actions`, which is where this feature keeps its copy. + * set: a verb names a number to a person, and a set re-read afterwards can be + * a different one. The sweeps and their reports live in `session-row-actions`, + * which is where this feature already keeps its copy. + * + * It does not unmark what it swept. The rule is "the selection follows the + * catalog", and the prune above already owns it: an archive refreshes the + * catalog before it resolves, so those rows leave the set by leaving the + * rail. Unmarking here would be that rule stated a second time — right for + * archive, wrong for pin, which leaves the rows exactly where they are and + * would drop a set the user was still working with. When a refresh fails and + * the rows stay listed, keeping them picked is the consistent answer: the + * rail still shows them. */ const runSweep = useCallback( async (run: (sessionIds: readonly string[]) => Promise) => { if (busyRef.current) return; const sessionIds = [...selectionRef.current.selectedIds]; if (sessionIds.length === 0) return; - setBusy(true); + busyRef.current = true; try { await run(sessionIds); } finally { - setBusy(false); - // Unmark exactly what this sweep asked about — never whatever happens - // to be marked when it lands. - // - // `Done` stays enabled during a sweep, so a person can leave the mode, - // re-enter it from another row's menu and mark B while A's request is - // still with the Host. Clearing the whole set here would then answer - // A's completion by discarding B, which the user never asked about. - // - // The MODE stays on. The person was in the middle of tidying up, and - // taking the checkboxes away after each sweep would make them re-enter - // for the next one. - setSelection((current) => { - const remaining = new Set(current.selectedIds); - let changed = false; - for (const sessionId of sessionIds) { - if (remaining.delete(sessionId)) changed = true; - } - return changed ? { active: current.active, selectedIds: remaining } : current; - }); + busyRef.current = false; } }, [], ); - const onArchiveSelected = useCallback( + const archiveSelected = useCallback( () => runSweep((ids) => commands.archiveSelected(ids)), [commands, runSweep], ); - const onDeleteSelected = useCallback( - () => runSweep((ids) => commands.deleteSelected(ids)), + const flagSelected = useCallback( + (flagged: boolean) => runSweep((ids) => commands.flagSelected(ids, flagged)), [commands, runSweep], ); - /** - * The rows' half, memoized on the selection ALONE. - * - * Every row subscribes to this, and a context consumer re-renders whenever - * the value it reads changes — `memo` cannot stop it. Keeping - * `listedSessionIds` out of it is the whole point: that array is derived from - * the catalog and moves on a session switch, which would re-render all of the - * rail's rows for a switch that changed two of them (#4109). - */ - const rowSelection = useMemo( - () => ({ - active: selection.active, - selectedIds: selection.selectedIds, - onToggleRow, - onEnter, - }), - [onEnter, onToggleRow, selection.active, selection.selectedIds], - ); - - const wholeSelection = useMemo( - () => ({ - active: selection.active, - selectedIds: selection.selectedIds, - listedSessionIds, - onToggleRow, - onEnter, - onExit, - onToggleAll, - onArchiveSelected, - onDeleteSelected, - busy, - }), - [ - busy, - listedSessionIds, - onArchiveSelected, - onDeleteSelected, - onEnter, - onExit, - onToggleAll, - onToggleRow, - selection.active, - selection.selectedIds, - ], + const selectionCommands = useMemo( + () => ({ pick, clear, retain, archiveSelected, flagSelected }), + [archiveSelected, clear, flagSelected, pick, retain], ); - return useMemo( - () => ({ selection: wholeSelection, rowSelection }), - [rowSelection, wholeSelection], + return useMemo( + () => ({ selectedIds: selection.selectedIds, commands: selectionCommands }), + [selection.selectedIds, selectionCommands], ); } diff --git a/apps/desktop/src/renderer/features/session-navigation/model/session-selection.ts b/apps/desktop/src/renderer/features/session-navigation/model/session-selection.ts index 95953feefe..64d256fe3a 100644 --- a/apps/desktop/src/renderer/features/session-navigation/model/session-selection.ts +++ b/apps/desktop/src/renderer/features/session-navigation/model/session-selection.ts @@ -17,32 +17,56 @@ * under the License. */ -/** What the rail has marked, and whether the mode is on at all. */ +import type { SessionRailSelectionCommands } from '@maka/ui'; + +/** + * What the rail has picked, and where a Shift range starts. + * + * There is no `active` flag and no mode to be in. A modifier held during a + * click is the whole vocabulary, which is what a Finder, a VS Code explorer and + * a Codex task list all use, so there is nothing to enter, nothing to leave, + * and no state in which a plain click means something else. + */ export interface SessionSelection { + readonly selectedIds: ReadonlySet; /** - * Whether the rail is in selection mode. - * - * Separate from `selectedIds` being empty, because unticking the master box - * is "select none" and not "leave". A mode that exited itself the moment the - * last row was cleared would take the checkboxes away mid-gesture, and the - * user would have to find the way back in to correct one mis-click. + * The row a Shift range extends from: the last row picked WITHOUT Shift. + * Undefined until one is, and again after a clear — a range then starts from + * whatever is open, which is where the user's attention already is. */ - readonly active: boolean; - readonly selectedIds: ReadonlySet; + readonly anchorId: string | undefined; } +/** + * What one click asks of the set — the rail's own contract, not a second copy + * of it. The rail is where the gesture is read, so the vocabulary is declared + * there and this reducer answers exactly the request the rail sends. + */ +export type SessionPickRequest = Parameters[0]; + +/** + * Nothing picked, no anchor — where the rail starts, and what a clear returns + * to. + * + * Clearing is not "collapse to the open row": the open row is painted by the + * rail whether or not it is picked, so an empty selection already reads as that + * one row, and a set of one that happens to equal the open row is a second way + * to say the same thing. + */ export const EMPTY_SESSION_SELECTION: SessionSelection = Object.freeze({ - active: false, selectedIds: Object.freeze(new Set()) as ReadonlySet, + anchorId: undefined, }); /** * Drops ids the catalog no longer lists. * * A selection outlives the list it was made from: another client deletes a - * task, a filter narrows, a bulk action removes what it removed. Acting on an - * id that is gone is at best a no-op and at worst a count that does not add up, - * so the selection is reconciled against the catalog rather than trusted. + * task, a filter narrows, an archive sweep removes what it removed. Acting on + * an id that is gone is at best a no-op and at worst a count that does not add + * up, so the selection is reconciled against the catalog rather than trusted. + * The anchor is reconciled with it — a range from a row that is no longer there + * would reach across the rows that took its place. */ export function pruneSessionSelection( selection: SessionSelection, @@ -53,46 +77,47 @@ export function pruneSessionSelection( for (const sessionId of selection.selectedIds) { if (listed.has(sessionId)) selectedIds.add(sessionId); } - if (selectedIds.size === selection.selectedIds.size) return selection; - // Pruning empties the set; it does not end the mode. The rows went away - // because the catalog changed, not because the user was finished. - return { active: selection.active, selectedIds }; -} - -/** Enters selection mode with nothing marked. */ -export function enterSessionSelection(selection: SessionSelection): SessionSelection { - return selection.active ? selection : { ...selection, active: true }; -} - -/** Leaves selection mode and drops what was marked. */ -export function exitSessionSelection(): SessionSelection { - return EMPTY_SESSION_SELECTION; + const anchorId = + selection.anchorId !== undefined && listed.has(selection.anchorId) + ? selection.anchorId + : undefined; + if (selectedIds.size === selection.selectedIds.size && anchorId === selection.anchorId) { + return selection; + } + return { selectedIds, anchorId }; } /** - * The master box: every listed row, or none of them. + * Applies one click to the selection. * - * "All" means every row the rail is listing right now, which is what the user - * can see the box sitting above — not every task in the catalog. A box that - * silently included rows behind a collapsed project, or filtered out of view, - * would name a number the user never agreed to. + * `orderedSessionIds` is the rail's RENDERED order, and it is an argument + * rather than state for the reason the rows never receive it: it is a property + * of the list, it changes identity whenever the catalog does, and #4365's first + * revision handed each row its group's copy — a changed prop on every memoised + * row, which turned a two-row session switch into a twelve-row one (#4109). + * Read at the moment of a click it costs one query and nothing per render. + * + * A range runs from the anchor, and the anchor does NOT move: a range can be + * re-dragged shorter or longer from the same origin, and the task the main pane + * is showing stays the one the user opened. */ -export function setAllSessionsSelected( +export function pickSessionRow( selection: SessionSelection, - listedSessionIds: readonly string[], - selected: boolean, + input: SessionPickRequest, ): SessionSelection { - if (!selected) return { active: selection.active, selectedIds: new Set() }; - return { active: true, selectedIds: new Set(listedSessionIds) }; -} - -/** What the master box shows: all, none, or some. */ -export function sessionSelectionMasterState( - selection: SessionSelection, - listedSessionIds: readonly string[], -): boolean | 'indeterminate' { - if (selection.selectedIds.size === 0) return false; - if (listedSessionIds.length === 0) return false; - const allListed = listedSessionIds.every((id) => selection.selectedIds.has(id)); - return allListed ? true : 'indeterminate'; + const { sessionId, pick, orderedSessionIds, openSessionId } = input; + if (pick === 'replace') return { selectedIds: new Set([sessionId]), anchorId: sessionId }; + if (pick === 'toggle') { + const selectedIds = new Set(selection.selectedIds); + if (!selectedIds.delete(sessionId)) selectedIds.add(sessionId); + return { selectedIds, anchorId: sessionId }; + } + const anchorId = selection.anchorId ?? openSessionId; + const from = anchorId === undefined ? -1 : orderedSessionIds.indexOf(anchorId); + const to = orderedSessionIds.indexOf(sessionId); + // No anchor to reach from, or a row the list is not showing: the range is the + // one row that was actually clicked. + if (from === -1 || to === -1) return { selectedIds: new Set([sessionId]), anchorId: sessionId }; + const run = orderedSessionIds.slice(Math.min(from, to), Math.max(from, to) + 1); + return { selectedIds: new Set(run), anchorId }; } diff --git a/apps/desktop/src/renderer/features/session-navigation/testing.ts b/apps/desktop/src/renderer/features/session-navigation/testing.ts index 8fe566ef5b..f20d456de7 100644 --- a/apps/desktop/src/renderer/features/session-navigation/testing.ts +++ b/apps/desktop/src/renderer/features/session-navigation/testing.ts @@ -45,11 +45,8 @@ export { deriveSessionRail } from './model/session-rail.js'; export { deriveSessionRevisionNavigation } from './model/session-revisions.js'; export { EMPTY_SESSION_SELECTION, - enterSessionSelection, - exitSessionSelection, + pickSessionRow, pruneSessionSelection, - sessionSelectionMasterState, - setAllSessionsSelected, type SessionSelection, } from './model/session-selection.js'; export { diff --git a/apps/desktop/src/renderer/features/session-navigation/ui/session-navigation-provider.tsx b/apps/desktop/src/renderer/features/session-navigation/ui/session-navigation-provider.tsx index 9abab676ca..efbcc31235 100644 --- a/apps/desktop/src/renderer/features/session-navigation/ui/session-navigation-provider.tsx +++ b/apps/desktop/src/renderer/features/session-navigation/ui/session-navigation-provider.tsx @@ -118,9 +118,9 @@ export function SessionNavigationProvider(props: SessionNavigationProviderProps) onRename: (sessionId, name) => { void controller.commands.renameSession(sessionId, name); }, - onDelete: (sessionId) => { - void controller.commands.deleteSession(sessionId); - }, + // No `onDelete`: the rail cannot delete. `deleteSession` is still a + // command, reached from Settings › 已归档任务, where the task has already + // been archived once. }), [controller.commands], ); @@ -227,7 +227,6 @@ export function SessionNavigationProvider(props: SessionNavigationProviderProps) data={data} chrome={chrome} selection={controller.selection} - rowSelection={controller.rowSelection} > {props.children} diff --git a/apps/desktop/src/renderer/locales/shell-copy.ts b/apps/desktop/src/renderer/locales/shell-copy.ts index 4de4e1a9c5..ed545587d2 100644 --- a/apps/desktop/src/renderer/locales/shell-copy.ts +++ b/apps/desktop/src/renderer/locales/shell-copy.ts @@ -277,24 +277,11 @@ type ShellCopy = { deleteSubtaskNoteUncertain(): string; /** Toast description after deleting a task that had linked subagent subtasks. */ deletedSubtaskNote(count: number): string; - bulkDeleteTitle(count: number): string; - bulkDeleteDescription: string; - bulkArchiveTitle(count: number): string; + /** Where the archived tasks went, said by the toast rather than a dialog. */ bulkArchiveDescription: string; - bulkArchiveLabel: string; - bulkDeletedTitle(count: number): string; bulkArchivedTitle(count: number): string; - /** Tasks restored while the sweep was reaching them, so they were kept. */ - bulkKeptRestored(count: number): string; - bulkDeleteFailedTitle: string; bulkArchiveFailedTitle: string; bulkFailedBody(count: number): string; - /** The catalog could not be read back, so nothing can be claimed. */ - bulkUnverified: string; - /** Appended to the bulk delete confirm when the selection has linked subtasks. */ - bulkDeleteSubtaskNote(): string; - /** Appended when the subtask preview could not be read for the whole selection. */ - bulkDeleteSubtaskNoteUncertain(): string; }; skillActions: { refreshSkillsFailedTitle: string; @@ -919,21 +906,10 @@ const SHELL_COPY_BY_LOCALE = { deleteSubtaskNote: () => '其普通子任务不会被删除，将保留并移入归档。', deleteSubtaskNoteUncertain: () => '其普通子任务（如有）不会被删除，将保留并移入归档。', deletedSubtaskNote: (count: number) => `${count} 个子任务已移入归档`, - bulkDeleteTitle: (count: number) => `删除选中的 ${count} 个任务？`, - bulkDeleteDescription: '删除后无法恢复，任务的全部修订版本都会一并删除。', - bulkArchiveTitle: (count: number) => `归档选中的 ${count} 个任务？`, bulkArchiveDescription: '归档后可在「设置 › 活动 › 已归档任务」中找回。', - bulkArchiveLabel: '归档', - bulkDeletedTitle: (count: number) => `已删除 ${count} 个任务`, bulkArchivedTitle: (count: number) => `已归档 ${count} 个任务`, - bulkKeptRestored: (count: number) => `另有 ${count} 个已被恢复，未删除。`, - bulkDeleteFailedTitle: '部分任务未能删除', bulkArchiveFailedTitle: '部分任务未能归档', bulkFailedBody: (count: number) => `还有 ${count} 个没有处理成功。`, - bulkUnverified: '无法确认处理结果，请刷新后查看。', - bulkDeleteSubtaskNote: () => '它们的普通子任务不会被删除，将保留并移入归档。', - bulkDeleteSubtaskNoteUncertain: () => - '它们的普通子任务（如有）不会被删除，将保留并移入归档。', }, skillActions: { refreshSkillsFailedTitle: '刷新技能失败', @@ -1437,22 +1413,10 @@ const SHELL_COPY_BY_LOCALE = { 'Its ordinary subtasks, if any, will be kept and moved to Archived.', deletedSubtaskNote: (count: number) => count === 1 ? '1 subtask moved to Archived' : `${count} subtasks moved to Archived`, - bulkDeleteTitle: (count: number) => `Delete ${count} selected tasks?`, - bulkDeleteDescription: - 'This cannot be undone, and every revision of each task goes with it.', - bulkArchiveTitle: (count: number) => `Archive ${count} selected tasks?`, bulkArchiveDescription: 'Archived tasks stay available under Settings › Activity.', - bulkArchiveLabel: 'Archive', - bulkDeletedTitle: (count: number) => `Deleted ${count} tasks`, bulkArchivedTitle: (count: number) => `Archived ${count} tasks`, - bulkKeptRestored: (count: number) => `${count} were restored meanwhile and kept.`, - bulkDeleteFailedTitle: 'Some tasks were not deleted', bulkArchiveFailedTitle: 'Some tasks were not archived', bulkFailedBody: (count: number) => `${count} of them did not go through.`, - bulkUnverified: 'The outcome could not be confirmed. Refresh to see what remains.', - bulkDeleteSubtaskNote: () => 'Their ordinary subtasks will be kept and moved to Archived.', - bulkDeleteSubtaskNoteUncertain: () => - 'Any ordinary subtasks they have will be kept and moved to Archived.', }, skillActions: { refreshSkillsFailedTitle: 'Could not refresh Skills', diff --git a/apps/desktop/src/renderer/styles/sidebar.css b/apps/desktop/src/renderer/styles/sidebar.css index 67b7ddc359..b0ceeafc71 100644 --- a/apps/desktop/src/renderer/styles/sidebar.css +++ b/apps/desktop/src/renderer/styles/sidebar.css @@ -537,15 +537,32 @@ /* * Multi-select. * - * The marked state is a background, not a border: the rail already spends its - * border vocabulary on the active row's own chrome, and a second outlined - * treatment beside it reads as two competing "current" marks rather than one - * current row among several marked ones. `--color-overlay-pressed` is the - * heavier of the rail's two overlays, so a marked row sits visibly above a - * hovered one without inventing a colour. + * A picked row and the OPEN row share one ground, which is what Codex and + * Claude Desktop both do and what makes "the open task is the selection when it + * holds one row" true on screen rather than only in the model. The token is the + * one Astryx's own `SideNavItem` uses for `isSelected`, so the two states are + * the same paint and not a near-match maintained by hand. + * + * `isSelected` itself is spent 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 is in the row's description. */ -.maka-session-row[data-selected='true'] > div > .astryx-side-nav-item { - background: var(--color-overlay-pressed); +.maka-session-row[data-picked='true'] .astryx-side-nav-item { + background-color: var(--color-neutral); +} + +/* + * Forced colors flatten `--color-neutral` away, which is why Astryx's own + * `isSelected` names a system color here instead. Picked rows have to say it + * too, or the state that carries the whole interaction is the one state high + * contrast cannot show. Same pair as `isSelected`, because it is the same + * ground — the two states stay one paint in both worlds. + */ +@media (forced-colors: active) { + .maka-session-row[data-picked='true'] .astryx-side-nav-item { + background-color: Highlight; + color: HighlightText; + } } /* @@ -557,103 +574,3 @@ .maka-session-row { user-select: none; } - -/* - * The bar heads the LIST, not the chrome, so it carries no rule of its own. - * SideNav already draws one under the whole sticky top region; a second one - * here put two hairlines 9px apart and grouped the bar upwards, away from the - * rows it governs. - */ -.maka-session-selection-bar { - position: sticky; - top: 0; - z-index: 2; - display: flex; - flex-direction: column; - gap: var(--space-1); - padding-block: var(--spacing-1) var(--spacing-2); - /* - * Opaque, because rows scroll underneath it. `--surface-canvas` is not a - * guess: the rail's ground is painted by `astryx-layout-panel` and resolves - * to the same oklch(0.975 0 0) this token does. - */ - background: var(--surface-canvas); -} - -/* - * Master box, count, way out. The box leads so it shares the rows' leading - * column: "all of these" is a claim the eye should be able to check against the - * boxes directly beneath it, not one it has to take on trust. - */ -.maka-session-selection-bar-head { - display: flex; - align-items: center; - gap: var(--space-2); -} - -/* Takes the leftover width so the way-out button stays on the trailing edge. */ -.maka-session-selection-count { - flex: 1 1 auto; -} - -/* - * The row's own leading column, which exists only while the mode does. Same - * gap as the head above, so a box and the box governing it sit on one line. - */ -.maka-session-row-check { - position: absolute; - inset-inline-start: 0; - top: 0; - bottom: 0; - display: flex; - align-items: center; - z-index: 1; -} - -/* - * Rows shift out of the way of their own box rather than the box overlapping - * the status dot. Padding on the row, not a margin on the item, so the hover - * and selected backgrounds still span the full width. - */ -.maka-session-row[data-selecting='true'] { - padding-inline-start: calc(20px + var(--space-2)); -} - -/* - * `min-width: 0` is what a flex child needs to be allowed to ellipsize rather - * than be crushed, and `nowrap` is what stops the crush from becoming one - * character per line — which is what a three-button single row did to it at - * 244px. Neither is decorative: the count is the number the confirm will name. - */ -/* - * `min-width: 0` is what a flex child needs to be allowed to ellipsize rather - * than be crushed, and `nowrap` is what stops the crush from becoming one - * character per line — which is what a three-button single row did to it at - * 244px. Neither is decorative: this is the number the confirm will name. - */ -.maka-session-selection-count { - min-width: 0; - overflow: hidden; - white-space: nowrap; - text-overflow: ellipsis; - font: var(--typography-caption); - color: var(--maka-text-supporting); - font-variant-numeric: tabular-nums; -} - -/* - * Two equal halves, not a flex row that lets the longer label win. Grid keeps - * 归档 and 删除 the same size at every rail width, so neither reads as the - * primary action — one of them is destructive and must not be emphasised by - * accident. - */ -.maka-session-selection-actions { - display: grid; - grid-template-columns: 1fr 1fr; - gap: var(--space-1); -} - -.maka-session-selection-actions > button { - width: 100%; - min-width: 0; -} diff --git a/apps/desktop/stories/app-shell.stories.tsx b/apps/desktop/stories/app-shell.stories.tsx index 858aa5867a..41256327ab 100644 --- a/apps/desktop/stories/app-shell.stories.tsx +++ b/apps/desktop/stories/app-shell.stories.tsx @@ -157,7 +157,6 @@ const sidebarRowActions: NonNullable = { onArchive: noop, onUnarchive: noop, onRename: noop, - onDelete: noop, }; const projectRowActions: NonNullable = { onNew: noop, diff --git a/apps/desktop/stories/session-rail-multi-select.stories.tsx b/apps/desktop/stories/session-rail-multi-select.stories.tsx new file mode 100644 index 0000000000..ae3465ce2a --- /dev/null +++ b/apps/desktop/stories/session-rail-multi-select.stories.tsx @@ -0,0 +1,176 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * The rail with more than one task picked. + * + * Lives here rather than beside the other rail stories in `packages/ui` + * because the selection is the app's: `useSessionSelection` and the reducer + * under it are desktop-side, and a story that reimplemented them would be + * showing its own behaviour rather than the product's. What it renders is the + * real `SessionListPanel`, through the same harness the other rail stories use. + */ + +import { useEffect, useRef, type ReactNode } from 'react'; +import type { Meta, StoryObj } from '@storybook/react-vite'; +import type { SessionSummary } from '@maka/core/session'; +import { SessionRail } from '../../../packages/ui/stories/session-rail-harness.js'; +import { + useSessionSelection, + type SessionNavigationRowActions, +} from '../src/renderer/features/session-navigation/testing'; + +const NOW = Date.now(); +const noop = () => undefined; + +// Fidelity convention (#1433): every story below names the real app path +// that reaches it. See apps/desktop/stories/FIDELITY.md. + +const meta = { + title: 'Product/Sidebar Multi Select', + parameters: { + layout: 'fullscreen', + }, +} satisfies Meta; + +export default meta; + +type Story = StoryObj; + +function makeSession(input: { + id: string; + name: string; + minutesAgo: number; + isFlagged?: boolean; +}): SessionSummary { + return { + id: input.id, + name: input.name, + isFlagged: input.isFlagged ?? false, + isArchived: false, + labels: [], + hasUnread: false, + status: 'active', + lastMessageAt: NOW - input.minutesAgo * 60 * 1000, + backend: 'ai-sdk', + llmConnectionSlug: 'zai-live', + connectionLocked: false, + model: 'glm-4.7', + permissionMode: 'ask', + }; +} + +const SESSIONS = [ + makeSession({ id: 'rail-a', name: '发布风险清单', minutesAgo: 8 }), + makeSession({ id: 'rail-b', name: '整理 compact controls', minutesAgo: 21 }), + makeSession({ id: 'rail-c', name: '刚结束的 smoke 回归', minutesAgo: 44 }), + makeSession({ id: 'rail-d', name: '长期跟踪的客户反馈', minutesAgo: 90 }), + makeSession({ id: 'rail-e', name: '权限模式的文案复核', minutesAgo: 150 }), +]; + +const OPEN_SESSION_ID = 'rail-a'; + +/** What a single row's menu asks for. */ +const ROW_ACTIONS = { + onToggleFlag: noop, + onArchive: noop, + onUnarchive: noop, + onRename: noop, +}; + +/** + * What a sweep asks for, answered locally. + * + * A story has no Host, and what a sweep does to the catalog is the row + * actions' business rather than the selection's — the states worth looking at + * here are the ones before a sweep runs. + */ +const SWEEPS = { + archiveSelected: async () => undefined, + flagSelected: async () => undefined, +} as unknown as SessionNavigationRowActions; + +function StoryFrame(props: { children: ReactNode }) { + // 260 is `SessionListPanel`'s own default width, and the height is the one + // the other rail stories use so the two are comparable side by side. + return ( +
+ {props.children} +
+ ); +} + +/** + * The rail, wired to the real selection, with a run already picked. + * + * The initial pick is made through `commands.pick` rather than seeded as + * state: it is the same pair of calls a click and a Shift-click make, so what + * the story mounts is a set the reducer produced, not one a fixture asserted. + */ +function MultiSelectRail() { + const selection = useSessionSelection({ sessions: SESSIONS, commands: SWEEPS }); + const commands = selection.commands; + const seeded = useRef(false); + + useEffect(() => { + if (seeded.current) return; + seeded.current = true; + const orderedSessionIds = SESSIONS.map((session) => session.id); + commands.pick({ sessionId: 'rail-b', pick: 'replace', orderedSessionIds }); + commands.pick({ sessionId: 'rail-d', pick: 'range', orderedSessionIds }); + }, [commands]); + + return ( + + ); +} + +// Real path: sidebar → click a task, then Shift-click one further down (or +// ⌘-click several). Three tasks are picked here and a fourth is open, which is +// the state the design turns on: picked and open share one ground, and only +// the open row is `aria-current`. The rail stays live — Shift-click, ⌘-click, +// Escape, right-click and ⋯ all work here as they do in the app. +export const PickedRun: Story = { + render: () => ( + + + + ), +}; diff --git a/docs/astryx-surface-file-inventory.md b/docs/astryx-surface-file-inventory.md index 29320a2e5d..d3bc54fc62 100644 --- a/docs/astryx-surface-file-inventory.md +++ b/docs/astryx-surface-file-inventory.md @@ -6,7 +6,7 @@ Generated against `@astryxdesign/core@0.5.2` (194 component exports). Wiki bar: Design Conventions · API Use-the-System · Theming · Container Padding. -**Totals:** 241 files — blocker 0, reimplementation 0, polish 1, aligned 240. +**Totals:** 240 files — blocker 0, reimplementation 0, polish 1, aligned 239. ## Exclusions (explicit) @@ -248,11 +248,10 @@ Wiki bar: Design Conventions · API Use-the-System · Theming · Container Paddi | `packages/ui/src/scheduled-task-panel.tsx` | module-hub | Button, Divider, DropdownMenu, DropdownMenuCheckboxItem, DropdownMenuItem, EmptyState, List, ListItem, SegmentedControl, SegmentedControlItem, Selector, StatusDot, Text, TextInput, Toolbar | aligned — uses Astryx (Button, Divider, DropdownMenu, DropdownMenuCheckboxItem, DropdownMenuItem, EmptyState, List, ListItem) | aligned | | `packages/ui/src/search-modal.tsx` | dialog-overlay | CommandPalette, CommandPaletteFooter, CommandPaletteInput | aligned — uses Astryx (CommandPalette, CommandPaletteFooter, CommandPaletteInput) | aligned | | `packages/ui/src/session-context-layer.tsx` | shell-chrome-or-panel | BreadcrumbItem, Breadcrumbs, ButtonGroup, Icon, IconButton, LayoutHeader, MoreMenu, OverflowList, StatusDot, Text, Token, Tooltip | aligned — uses Astryx (BreadcrumbItem, Breadcrumbs, ButtonGroup, Icon, IconButton, LayoutHeader, MoreMenu, OverflowList) | aligned | -| `packages/ui/src/session-history-list.tsx` | shell-chrome-or-panel | Badge, CheckboxInput, MoreMenu, SideNavItem, SideNavSection, StatusDot, VStack | aligned — uses Astryx (Badge, CheckboxInput, MoreMenu, SideNavItem, SideNavSection, StatusDot, VStack) | aligned | +| `packages/ui/src/session-history-list.tsx` | shell-chrome-or-panel | Badge, MoreMenu, SideNavItem, SideNavSection, StatusDot, VStack | aligned — uses Astryx (Badge, MoreMenu, SideNavItem, SideNavSection, StatusDot, VStack) | aligned | | `packages/ui/src/session-list-panel.tsx` | shell-chrome-or-panel | SegmentedControl, SegmentedControlItem, SideNav | aligned — uses Astryx (SegmentedControl, SegmentedControlItem, SideNav) | aligned | | `packages/ui/src/session-rail-context.tsx` | shell-chrome-or-panel | none | aligned — no raw controls; no Astryx JSX usage | aligned | | `packages/ui/src/session-rename-dialog.tsx` | dialog-overlay | Button, Dialog, DialogHeader, HStack, Layout, LayoutContent, LayoutFooter, TextInput | aligned — uses Astryx (Button, Dialog, DialogHeader, HStack, Layout, LayoutContent, LayoutFooter, TextInput) | aligned | -| `packages/ui/src/session-selection-bar.tsx` | shell-chrome-or-panel | Button, CheckboxInput | aligned — uses Astryx (Button, CheckboxInput) | aligned | | `packages/ui/src/session-sidebar-nav.tsx` | shell-chrome-or-panel | Icon, IconButton, SideNavItem, SideNavSection, Tooltip | aligned — uses Astryx (Icon, IconButton, SideNavItem, SideNavSection, Tooltip) | aligned | | `packages/ui/src/session-todo-panel.tsx` | shell-chrome-or-panel | Banner, EmptyState, IconButton, Spinner | aligned — uses Astryx (Banner, EmptyState, IconButton, Spinner) | aligned | | `packages/ui/src/skill-inspector.tsx` | shell-chrome-or-panel | Button, Divider, HStack, Heading, MetadataList, MetadataListItem, StackItem, StatusDot, Switch, Text, VStack | aligned — uses Astryx (Button, Divider, HStack, Heading, MetadataList, MetadataListItem, StackItem, StatusDot) | aligned | diff --git a/docs/astryx-surface-file-inventory.paths b/docs/astryx-surface-file-inventory.paths index ce7fea2e57..dbf0e86e67 100644 --- a/docs/astryx-surface-file-inventory.paths +++ b/docs/astryx-surface-file-inventory.paths @@ -223,7 +223,6 @@ packages/ui/src/session-history-list.tsx packages/ui/src/session-list-panel.tsx packages/ui/src/session-rail-context.tsx packages/ui/src/session-rename-dialog.tsx -packages/ui/src/session-selection-bar.tsx packages/ui/src/session-sidebar-nav.tsx packages/ui/src/session-todo-panel.tsx packages/ui/src/skill-inspector.tsx diff --git a/packages/ui/src/__tests__/session-history-multi-select.test.tsx b/packages/ui/src/__tests__/session-history-multi-select.test.tsx index 98bc493010..4f5ed3c679 100644 --- a/packages/ui/src/__tests__/session-history-multi-select.test.tsx +++ b/packages/ui/src/__tests__/session-history-multi-select.test.tsx @@ -19,14 +19,15 @@ /** * The rail is a navigation surface that also has to be selectable, so what - * matters is which of the two a click is. These cases drive real clicks rather - * than asserting markup: the branch under test is chosen inside the row's - * handler, and markup cannot say which branch ran. + * matters is which of the two a click is — and that is decided by the modifier + * held while clicking, nowhere else. These cases drive real clicks rather than + * asserting markup: the branch under test is chosen inside a handler, and + * markup cannot say which branch ran. */ import assert from 'node:assert/strict'; import test from 'node:test'; -import { act } from 'react'; +import { act, useMemo, useState, type Dispatch, type SetStateAction } from 'react'; import { createRoot } from 'react-dom/client'; import { parseHTML } from 'linkedom'; import type { SessionSummary } from '@maka/core/session'; @@ -35,38 +36,21 @@ import { SessionHistoryList } from '../session-history-list.js'; import { SessionRailProvider, type SessionRailData, - type SessionRailRowSelection, type SessionRailSelection, + type SessionRailSelectionCommands, } from '../session-rail-context.js'; /** - * What Astryx's SideNavItem reaches for that linkedom does not ship: a computed - * style and `matchMedia`, which its hover hook subscribes to. Neither is what - * these cases are about, so both answer the least interesting truth. + * What Astryx reaches for that linkedom does not ship: a computed style and + * `matchMedia`, which SideNavItem's hover hook subscribes to, and the frame + * callback DropdownMenu positions itself in. None is what these cases are + * about, so each answers the least interesting truth. */ -/** - * linkedom ships no `MouseEvent`, and React reads the modifier flags straight - * off the native event, so a plain Event carrying them is exactly as much event - * as the handler under test looks at. - */ -function clickEvent( - window: ReturnType['window'], - modifiers: Partial, -): Event { - const event = new window.Event('click', { bubbles: true, cancelable: true }); - Object.assign(event, { - detail: 1, - button: 0, - metaKey: false, - ctrlKey: false, - shiftKey: false, - altKey: false, - ...modifiers, - }); - return event as unknown as Event; -} - function installDomStubs(window: ReturnType['window']): void { + Object.assign(globalThis, { + requestAnimationFrame: () => 1, + cancelAnimationFrame: () => undefined, + }); window.getComputedStyle = () => ({ direction: 'ltr', @@ -85,7 +69,30 @@ function installDomStubs(window: ReturnType['window']): void { }); } -function summary(id: string): SessionSummary { +/** + * linkedom ships no `MouseEvent`, and React reads the modifier flags straight + * off the native event, so a plain Event carrying them is exactly as much event + * as the handlers under test look at. + */ +function pointerEvent( + window: ReturnType['window'], + type: 'click' | 'contextmenu', + modifiers: Record = {}, +): Event { + const event = new window.Event(type, { bubbles: true, cancelable: true }); + Object.assign(event, { + detail: 1, + button: type === 'contextmenu' ? 2 : 0, + metaKey: false, + ctrlKey: false, + shiftKey: false, + altKey: false, + ...modifiers, + }); + return event as unknown as Event; +} + +function summary(id: string, overrides: Partial = {}): SessionSummary { return { id, name: id, @@ -99,27 +106,50 @@ function summary(id: string): SessionSummary { connectionLocked: true, model: 'test-model', permissionMode: 'ask', + ...overrides, }; } -const SESSIONS = ['a', 'b', 'c'].map(summary); +const ROW_ACTIONS = { + onToggleFlag: () => undefined, + onArchive: () => undefined, + onUnarchive: () => undefined, + onRename: () => undefined, +}; -type Harness = { +type PickCall = Parameters[0]; + +interface Harness { opened: string[]; - toggles: Array<[string, boolean]>; - toggleAll: boolean[]; - entered: Array; - exits: number; - deleteRequests: number; - pressKey(key: string, focusedSessionId?: string): Promise; - dispose(): Promise; - clickRow(sessionId: string, modifiers?: Partial): Promise; - clickCheckbox(sessionId: string, checked: boolean): Promise; + picks: PickCall[]; + /** The id lists the menu narrowed the set to, in the order it asked. */ + retains: string[][]; + flags: boolean[]; + clears: number; + archives: number; document: Document; -}; + clickRow(sessionId: string, modifiers?: Record): Promise; + rightClickRow(sessionId: string): Promise; + openRowMenu(sessionId: string): Promise; + clickMenuItem(index: number): Promise; + menuLabels(): string[]; + /** What the row says to assistive tech and nowhere else. */ + rowDescription(sessionId: string): string; + hasRowMenu(sessionId: string): boolean; + collapseProject(projectId: string): Promise; + pressEscape(focusedSessionId?: string): Promise; + pressEscapeInsideRowMenu(sessionId: string): Promise; + dispose(): Promise; +} async function mount( - options: { selectedIds?: readonly string[]; active?: boolean } = {}, + options: { + selectedIds?: readonly string[]; + sessions?: SessionSummary[]; + activeId?: string; + groupVariant?: SessionRailData['groupVariant']; + groups?: SessionRailData['groups']; + } = {}, ): Promise { const original = { document: globalThis.document, @@ -132,35 +162,58 @@ async function mount( installDomStubs(window); Object.assign(globalThis, { document, window, IS_REACT_ACT_ENVIRONMENT: true }); + const sessions = options.sessions ?? ['a', 'b', 'c', 'd'].map((id) => summary(id)); const opened: string[] = []; - const toggles: Array<[string, boolean]> = []; - const toggleAll: boolean[] = []; - const entered: Array = []; - let exits = 0; - let deleteRequests = 0; - const rowSelection: SessionRailRowSelection = { - active: options.active ?? false, - selectedIds: new Set(options.selectedIds ?? []), - onToggleRow: (sessionId, selected) => toggles.push([sessionId, selected]), - onEnter: (sessionId) => entered.push(sessionId), - }; - const selection: SessionRailSelection = { - ...rowSelection, - listedSessionIds: SESSIONS.map((session) => session.id), - onExit: () => { - exits += 1; + const picks: PickCall[] = []; + const retains: string[][] = []; + const flags: boolean[] = []; + let clears = 0; + let archives = 0; + // `pick` only records, because what the app's reducer makes of a click is + // that reducer's own test. `retain` also applies, because the menu's wording + // and its sweep are read from the set it narrows — a stub that recorded and + // left the set alone would let an assertion about the menu pass over rows + // the narrowing was supposed to have removed. + let narrow: Dispatch>> | undefined; + const commands: SessionRailSelectionCommands = { + pick: (request) => picks.push(request), + clear: () => { + clears += 1; }, - onToggleAll: (selected) => toggleAll.push(selected), - onArchiveSelected: () => undefined, - onDeleteSelected: () => { - deleteRequests += 1; + retain: (sessionIds) => { + retains.push([...sessionIds]); + const keep = new Set(sessionIds); + narrow?.((current) => new Set([...current].filter((sessionId) => keep.has(sessionId)))); + }, + archiveSelected: () => { + archives += 1; + }, + flagSelected: (flagged) => { + flags.push(flagged); }, }; + + function Rail() { + const [selectedIds, setSelectedIds] = useState>( + () => new Set(options.selectedIds ?? []), + ); + narrow = setSelectedIds; + const selection = useMemo(() => ({ selectedIds, commands }), [ + selectedIds, + ]); + return ( + + + + ); + } const data: SessionRailData = { - sessions: SESSIONS, - groupVariant: 'conversation', - groups: [{ id: 'recent', label: 'Recent', sessions: [...SESSIONS] }], + sessions, + activeId: options.activeId, + groupVariant: options.groupVariant ?? 'conversation', + groups: options.groups ?? [{ id: 'recent', label: 'Recent', sessions: [...sessions] }], onSelectSession: (sessionId) => opened.push(sessionId), + rowActions: ROW_ACTIONS, }; const container = document.querySelector('#root'); @@ -169,93 +222,230 @@ async function mount( await act(() => root.render( - - - + , ), ); - const harness: Harness = { + function rowButton(sessionId: string): Element { + const node = document.querySelector( + `[data-session-id="${sessionId}"] button.astryx-side-nav-item`, + ); + assert.ok(node, `no clickable row for ${sessionId}`); + return node; + } + + // Every row owns a MoreMenu and Astryx renders each one's items eagerly, so + // the whole rail's items are in the document at once. Only the row whose ⋯ + // was last pressed is on screen, so that is the row these read. + let openedMenuRow: string | undefined; + + function menuItems(): Element[] { + assert.ok(openedMenuRow, 'no row menu has been opened'); + return [ + ...document.querySelectorAll(`[data-session-id="${openedMenuRow}"] [role="menuitem"]`), + ]; + } + + return { opened, - toggles, - toggleAll, - entered, - get exits() { - return exits; + picks, + retains, + flags, + get clears() { + return clears; }, - get deleteRequests() { - return deleteRequests; + get archives() { + return archives; }, - pressKey: async (key: string, focusedSessionId = 'a') => { - // linkedom has no focus model and reports `activeElement` as null, where a - // browser reports when nothing is focused — and here a row really - // is focused, because the user just clicked one. The handler's first - // guard reads it, so the test has to answer it. - const focused = document.querySelector(`[data-session-id="${focusedSessionId}"] button`); - assert.ok(focused); - Object.defineProperty(document, 'activeElement', { - configurable: true, - get: () => focused, + document: document as unknown as Document, + clickRow: async (sessionId, modifiers = {}) => { + await act(() => { + rowButton(sessionId).dispatchEvent(pointerEvent(window, 'click', modifiers)); }); - const list = document.querySelector('.maka-session-list'); - assert.ok(list); + }, + rightClickRow: async (sessionId) => { + const event = pointerEvent(window, 'contextmenu'); await act(() => { - const event = new window.Event('keydown', { bubbles: true, cancelable: true }); - Object.assign(event, { key }); - list.dispatchEvent(event); + rowButton(sessionId).dispatchEvent(event); }); + // Whether the rail claimed the press. Unclaimed, it goes on to the native + // menu, which is the whole answer for a row the rail cannot act on. + return event.defaultPrevented; }, - document: document as unknown as Document, - clickRow: async (sessionId, modifiers = {}) => { - const row = document.querySelector(`[data-session-id="${sessionId}"] button`); - assert.ok(row, `no clickable row for ${sessionId}`); + openRowMenu: async (sessionId) => { + const trigger = document.querySelector( + `[data-session-id="${sessionId}"] .maka-session-row-action button`, + ); + assert.ok(trigger, `no row menu trigger for ${sessionId}`); await act(() => { - row.dispatchEvent(clickEvent(window, modifiers)); + trigger.dispatchEvent(pointerEvent(window, 'click')); }); + // Scoping the reads below to this row is not enough on its own: the items + // are in the document before any ⋯ is pressed, so an assertion about the + // menu's wording would pass against a trigger that opens nothing. The + // menu has to be observably open before this row becomes the one read. + assert.equal( + trigger.closest('.maka-session-row-action')?.getAttribute('data-menu-open'), + 'true', + `⋯ on ${sessionId} did not open a menu`, + ); + openedMenuRow = sessionId; }, - clickCheckbox: async (sessionId, checked) => { - const box = document.querySelector( - `[data-session-id="${sessionId}"] .maka-session-row-check input`, - ) as HTMLInputElement | null; - assert.ok(box, `no checkbox for ${sessionId}`); - // React's checkbox `onChange` is driven by the native CLICK, not by a - // `change` event, and its value tracker swallows a programmatic - // `.checked` write that is not followed by one. + clickMenuItem: async (index) => { + const item = menuItems()[index]; + assert.ok(item, `no menu item at ${index}`); await act(() => { - box.checked = checked; - box.dispatchEvent(new window.Event('click', { bubbles: true, cancelable: true })); + item.dispatchEvent(pointerEvent(window, 'click')); }); }, + menuLabels: () => menuItems().map((item) => (item.textContent ?? '').trim()), + rowDescription: (sessionId) => + ( + document.querySelector(`[data-session-id="${sessionId}"] .maka-visually-hidden`) + ?.textContent ?? '' + ).trim(), + hasRowMenu: (sessionId) => + document.querySelector(`[data-session-id="${sessionId}"] .maka-session-row-action`) !== null, + collapseProject: async (projectId) => { + const group = document.querySelector(`[data-project-id="${projectId}"]`); + assert.ok(group, `no project row for ${projectId}`); + const toggle = [...group.querySelectorAll('button')].find( + (candidate) => candidate.getAttribute('aria-expanded') !== null, + ); + assert.ok(toggle, `no disclosure on project ${projectId}`); + await act(() => { + toggle.dispatchEvent(pointerEvent(window, 'click')); + }); + assert.equal(toggle.getAttribute('aria-expanded'), 'false', `${projectId} did not collapse`); + }, + pressEscape: async (focusedSessionId = 'a') => { + // linkedom has no focus model and reports `activeElement` as null, where a + // browser reports when nothing is focused — and here a row really + // is focused, because the user just clicked one. The handler's first + // guard reads it, so the test has to answer it. + const focused = rowButton(focusedSessionId); + return pressEscapeWith(focused); + }, + pressEscapeInsideRowMenu: async (sessionId) => { + const item = document.querySelector(`[data-session-id="${sessionId}"] [role="menuitem"]`); + assert.ok(item, `no menu item under ${sessionId}`); + return pressEscapeWith(item); + }, dispose: async () => { await act(() => root.unmount()); Object.assign(globalThis, original); }, }; - return harness; + + async function pressEscapeWith(focused: Element): Promise { + Object.defineProperty(document, 'activeElement', { + configurable: true, + get: () => focused, + }); + const list = document.querySelector('.maka-session-list'); + assert.ok(list); + const event = new window.Event('keydown', { bubbles: true, cancelable: true }); + Object.assign(event, { key: 'Escape' }); + await act(() => { + list.dispatchEvent(event); + }); + // Whether the rail claimed 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 claiming one the rail does not + // own leaves whatever is above it with no way to close. + return event.defaultPrevented; + } } -test('a plain click still opens the task', async () => { +test('a plain click opens the task and picks exactly it', async () => { const harness = await mount(); try { await harness.clickRow('b'); assert.deepEqual(harness.opened, ['b']); + assert.equal(harness.picks.length, 1); + assert.equal(harness.picks[0]?.pick, 'replace'); + assert.equal(harness.picks[0]?.sessionId, 'b'); } finally { await harness.dispose(); } }); -test('a marked row says so in the DOM', async () => { - const harness = await mount({ selectedIds: ['b'] }); +test('⌘-click adds a row without opening it', async () => { + // The whole point of the modifier: the main pane must not move away from the + // task the user is reading while they build a set beside it. + const harness = await mount({ selectedIds: ['a'] }); try { - const marked = harness.document.querySelectorAll('[data-selected="true"]'); - assert.equal(marked.length, 1); - assert.equal((marked[0] as HTMLElement).dataset.sessionId, 'b'); + await harness.clickRow('c', { metaKey: true }); + assert.deepEqual(harness.opened, []); + assert.equal(harness.picks[0]?.pick, 'toggle'); + assert.equal(harness.picks[0]?.sessionId, 'c'); } finally { await harness.dispose(); } }); +test('Ctrl-click is the same gesture, for the platforms that spell it that way', async () => { + const harness = await mount(); + try { + await harness.clickRow('c', { ctrlKey: true }); + assert.deepEqual(harness.opened, []); + assert.equal(harness.picks[0]?.pick, 'toggle'); + } finally { + await harness.dispose(); + } +}); + +test('Shift-click asks for a range, and hands over the rendered order', async () => { + // The order comes off the DOM rather than out of a prop, which is what keeps + // it right across groups and off the rows entirely (#4109). + const harness = await mount({ selectedIds: ['a'] }); + try { + await harness.clickRow('c', { shiftKey: true }); + assert.deepEqual(harness.opened, []); + assert.equal(harness.picks[0]?.pick, 'range'); + assert.deepEqual(harness.picks[0]?.orderedSessionIds, ['a', 'b', 'c', 'd']); + } finally { + await harness.dispose(); + } +}); + +test('a picked row says so on its ground', async () => { + const harness = await mount({ selectedIds: ['b', 'c'] }); + try { + const picked = [...harness.document.querySelectorAll('[data-picked="true"]')].map( + (row) => (row as HTMLElement).dataset.sessionId, + ); + assert.deepEqual(picked, ['b', 'c']); + } finally { + await harness.dispose(); + } +}); + +/** + * The ground says "picked" and `isSelected` says "open", and they are the same + * ground — so the open row has to spell out which of the two it is doing. Alone + * in the set it is only open, and "selected" would be a second word for the + * highlight a screen reader already reads. Inside a run it is one of several, + * and a reader that never hears so cannot tell what the ⋯ is about to sweep. + */ +test('the open row says it is picked once the set is more than itself', async () => { + const alone = await mount({ selectedIds: ['b'], activeId: 'b' }); + try { + assert.equal(alone.rowDescription('b').includes('Selected'), false); + } finally { + await alone.dispose(); + } + + const inARun = await mount({ selectedIds: ['b', 'c'], activeId: 'b' }); + try { + assert.equal(inARun.rowDescription('b').includes('Selected'), true); + assert.equal(inARun.rowDescription('c').includes('Selected'), true); + } finally { + await inARun.dispose(); + } +}); + test('a rail with no selection wired up behaves exactly as before', async () => { // The context is optional so a surface that never adopts multi-select — or a // story that renders rows alone — keeps plain clicks and gains no chrome. @@ -269,6 +459,7 @@ test('a rail with no selection wired up behaves exactly as before', async () => const { document, window } = parseHTML('
'); installDomStubs(window); Object.assign(globalThis, { document, window, IS_REACT_ACT_ENVIRONMENT: true }); + const sessions = ['a', 'b'].map((id) => summary(id)); const opened: string[] = []; const container = document.querySelector('#root'); assert.ok(container); @@ -279,9 +470,9 @@ test('a rail with no selection wired up behaves exactly as before', async () => opened.push(sessionId), }} > @@ -290,94 +481,284 @@ test('a rail with no selection wired up behaves exactly as before', async () => , ), ); - const row = document.querySelector('[data-session-id="b"] button'); + const row = document.querySelector('[data-session-id="b"] button.astryx-side-nav-item'); assert.ok(row); await act(() => { - row.dispatchEvent(clickEvent(window, { metaKey: true })); + row.dispatchEvent(pointerEvent(window, 'click', { metaKey: true })); }); // A modifier with nothing wired up is still a click on a task. assert.deepEqual(opened, ['b']); - assert.equal(document.querySelector('[data-selected="true"]'), null); + assert.equal(document.querySelector('[data-picked="true"]'), null); } finally { await act(() => root.unmount()); Object.assign(globalThis, original); } }); -test('Escape leaves the mode', async () => { - const harness = await mount({ active: true, selectedIds: ['b'] }); +test('Escape drops the picks', async () => { + const harness = await mount({ selectedIds: ['b', 'c'] }); try { - await harness.pressKey('Escape'); - assert.equal(harness.exits, 1); + await harness.pressEscape(); + assert.equal(harness.clears, 1); } finally { await harness.dispose(); } }); -test('Escape outside the mode is not this handler\'s business', async () => { +test("Escape with nothing picked is not this handler's business", async () => { const harness = await mount(); try { - await harness.pressKey('Escape'); - assert.equal(harness.exits, 0); + await harness.pressEscape(); + assert.equal(harness.clears, 0); } finally { await harness.dispose(); } }); -test('Delete asks for the marked set, not the focused row', async () => { - // Deleting the focused row while several are marked is the shape of an - // unrecoverable surprise: the user sees N marked and loses one they did not - // single out. - const harness = await mount({ active: true, selectedIds: ['a', 'c'] }); +test('right-clicking a pickable row opens its menu', async () => { + // The whole point of claiming the press: the row's own ⋯ menu opens, so ⋯ and + // right-click cannot drift into two lists of items that disagree. + const harness = await mount({ selectedIds: ['b'] }); try { - await harness.pressKey('Delete'); - assert.equal(harness.deleteRequests, 1); + const prevented = await harness.rightClickRow('b'); + assert.equal(prevented, true); + assert.equal( + harness.document + .querySelector('[data-session-id="b"] .maka-session-row-action') + ?.getAttribute('data-menu-open'), + 'true', + ); } finally { await harness.dispose(); } }); -test('no checkbox exists until the mode is on', async () => { - const harness = await mount(); +test('right-clicking a row outside the set replaces it', async () => { + // A menu is about a set, and one opened on a row the user never picked must + // not silently be about the rows they did. + const harness = await mount({ selectedIds: ['a', 'b'] }); try { - assert.equal(harness.document.querySelector('.maka-session-row-check'), null); + await harness.rightClickRow('d'); + assert.equal(harness.picks[0]?.pick, 'replace'); + assert.equal(harness.picks[0]?.sessionId, 'd'); + assert.deepEqual(harness.opened, []); } finally { await harness.dispose(); } }); -test('in the mode every row carries a box, ticked to match', async () => { - const harness = await mount({ active: true, selectedIds: ['b'] }); +test('right-clicking inside the set keeps it', async () => { + // A run built by dragging has to survive the gesture that asks what to do + // with it. + const harness = await mount({ selectedIds: ['a', 'b'] }); try { - const boxes = [...harness.document.querySelectorAll('.maka-session-row-check input')]; - assert.equal(boxes.length, 3); - assert.deepEqual( - boxes.map((box) => (box as HTMLInputElement).checked), - [false, true, false], - ); + await harness.rightClickRow('b'); + assert.deepEqual(harness.picks, []); + assert.deepEqual(harness.opened, []); } finally { await harness.dispose(); } }); -test('ticking a box reports the row and the direction', async () => { - const harness = await mount({ active: true }); +test('clicking ⋯ on a row outside the set replaces it, and never opens the task', async () => { + const harness = await mount({ selectedIds: ['a'] }); try { - await harness.clickCheckbox('c', true); - assert.deepEqual(harness.toggles, [['c', true]]); + await harness.openRowMenu('d'); + assert.deepEqual(harness.opened, []); + assert.equal(harness.picks[0]?.pick, 'replace'); + assert.equal(harness.picks[0]?.sessionId, 'd'); } finally { await harness.dispose(); } }); -test('a row click still opens the task while the mode is on', async () => { - // The box is the selection affordance; the row keeps its job. Making the - // whole row toggle would cost the rail the one thing it is for. - const harness = await mount({ active: true }); +test('the menu counts the set once more than one row is picked', async () => { + const harness = await mount({ selectedIds: ['a', 'b', 'c'] }); try { - await harness.clickRow('b'); - assert.deepEqual(harness.opened, ['b']); - assert.deepEqual(harness.toggles, []); + await harness.openRowMenu('b'); + assert.deepEqual(harness.menuLabels(), ['Pin 3 tasks', 'Archive 3 tasks']); + await harness.clickMenuItem(1); + assert.equal(harness.archives, 1); + } finally { + await harness.dispose(); + } +}); + +test('the menu is about the one row when only that row is picked', async () => { + const harness = await mount({ selectedIds: ['b'] }); + try { + await harness.openRowMenu('b'); + assert.deepEqual(harness.menuLabels(), ['Pin', 'Rename', 'Archive']); + } finally { + await harness.dispose(); + } +}); + +test('a set that is pinned throughout offers to unpin it', async () => { + // Every row pinned means the verb unpins; a mixed set pins. Without that rule + // one label would mean something different for each row under it. + const harness = await mount({ + sessions: [summary('a', { isFlagged: true }), summary('b', { isFlagged: true }), summary('c')], + selectedIds: ['a', 'b'], + }); + try { + await harness.openRowMenu('a'); + assert.deepEqual(harness.menuLabels(), ['Unpin 2 tasks', 'Archive 2 tasks']); + await harness.clickMenuItem(0); + assert.deepEqual(harness.flags, [false]); + } finally { + await harness.dispose(); + } +}); + +test('a mixed set pins, including the rows already pinned', async () => { + const harness = await mount({ + sessions: [summary('a', { isFlagged: true }), summary('b'), summary('c')], + selectedIds: ['a', 'b'], + }); + try { + await harness.openRowMenu('a'); + assert.deepEqual(harness.menuLabels(), ['Pin 2 tasks', 'Archive 2 tasks']); + await harness.clickMenuItem(0); + assert.deepEqual(harness.flags, [true]); + } finally { + await harness.dispose(); + } +}); + +/** + * A project group collapses by grid track — `SideNavItem` keeps the rows + * mounted and marks the subtree `inert` — so the rail renders more rows than + * the user can see. What a range may cross is the visible ones. + */ +test('a Shift range does not reach into a collapsed project', async () => { + const project = (id: string) => ({ + id, + name: id, + path: `/${id}`, + createdAt: 1, + updatedAt: 1, + available: true, + locations: [], + }); + const sessions = ['a1', 'a2', 'b1', 'b2', 'b3', 'c1'].map((id) => summary(id)); + const harness = await mount({ + sessions, + selectedIds: ['a1'], + groupVariant: 'project', + groups: [ + { id: 'pA', label: 'A', project: project('pA'), sessions: sessions.slice(0, 2) }, + { id: 'pB', label: 'B', project: project('pB'), sessions: sessions.slice(2, 5) }, + { id: 'pC', label: 'C', project: project('pC'), sessions: sessions.slice(5) }, + ], + }); + try { + await harness.collapseProject('pB'); + await harness.clickRow('c1', { shiftKey: true }); + assert.equal(harness.picks[0]?.pick, 'range'); + // Not a1..c1 across all six. B's rows are still in the document; they are + // just not on screen, and a set the user cannot see is a set they cannot + // check before the menu acts on it. + assert.deepEqual(harness.picks[0]?.orderedSessionIds, ['a1', 'a2', 'c1']); + } finally { + await harness.dispose(); + } +}); + +/** + * The other half of the same rule, at the other end of the gesture. A menu is + * the one entrance to the sweeps, so opening one fixes the set at the rows on + * screen: a pick that has been folded away leaves it there and does not come + * back. Narrowing here rather than at the collapse itself is not a softer rule + * — it is the only place the rail holds both the set and the DOM that knows + * which rows are showing. + */ +test('a menu sweeps only the picked rows still on screen', async () => { + const project = (id: string) => ({ + id, + name: id, + path: `/${id}`, + createdAt: 1, + updatedAt: 1, + available: true, + locations: [], + }); + const sessions = ['a1', 'a2', 'b1', 'b2'].map((id) => summary(id)); + const harness = await mount({ + sessions, + selectedIds: ['a1', 'b1'], + groupVariant: 'project', + groups: [ + { id: 'pA', label: 'A', project: project('pA'), sessions: sessions.slice(0, 2) }, + { id: 'pB', label: 'B', project: project('pB'), sessions: sessions.slice(2) }, + ], + }); + try { + await harness.collapseProject('pB'); + await harness.openRowMenu('a1'); + // b1 is still mounted, but it is not among the rows the menu was handed — + // so it leaves the set here, and stays out of it. + assert.deepEqual(harness.retains.at(-1), ['a1', 'a2']); + // And the menu says so: one row, with the single-row wording. + assert.deepEqual(harness.menuLabels(), ['Pin', 'Rename', 'Archive']); + } finally { + await harness.dispose(); + } +}); + +/** + * A session shared from someone else's Host is projected read-only: the rail + * hands its row no actions. A row with nothing that can be done to it has no + * business in a set whose only purpose is to be acted on. + */ +test('a shared row is not picked, and is still a plain navigation item', async () => { + const shared = { ...summary('shared'), shared: true } as SessionSummary; + const harness = await mount({ sessions: [summary('a'), shared, summary('c')] }); + try { + assert.equal(harness.hasRowMenu('shared'), false); + + await harness.clickRow('shared', { metaKey: true }); + assert.equal(harness.picks.length, 0); + // Not a dead click: with no set to join, the modifier means nothing and the + // row does what it has always done. + assert.deepEqual(harness.opened, ['shared']); + + await harness.clickRow('c', { shiftKey: true }); + assert.deepEqual(harness.picks.at(-1)?.orderedSessionIds, ['a', 'c']); + } finally { + await harness.dispose(); + } +}); + +test('right-clicking a shared row leaves the press, and the set, alone', async () => { + const shared = { ...summary('shared'), shared: true } as SessionSummary; + const harness = await mount({ + sessions: [summary('a'), shared, summary('c')], + selectedIds: ['a', 'c'], + }); + try { + const prevented = await harness.rightClickRow('shared'); + // Claiming it would take the native menu away and open nothing in its + // place, and adopting the row would discard the set on the way. + assert.equal(prevented, false); + assert.equal(harness.picks.length, 0); + } finally { + await harness.dispose(); + } +}); + +/** + * Escape belongs to whatever is on top. An open menu — and the rename dialog + * built on the same layer stack — is above the rail and owns the press, and the + * stack stands down on one that is already defaultPrevented: a rail that + * claimed it would clear the set AND leave the layer with no way to close. + */ +test('Escape inside a layer above the rail is not the rail\'s to take', async () => { + const harness = await mount({ selectedIds: ['a', 'b'] }); + try { + const prevented = await harness.pressEscapeInsideRowMenu('a'); + assert.equal(prevented, false); + assert.equal(harness.clears, 0); } finally { await harness.dispose(); } diff --git a/packages/ui/src/__tests__/session-history-row-actions.test.tsx b/packages/ui/src/__tests__/session-history-row-actions.test.tsx index e8bf08478b..15d7523263 100644 --- a/packages/ui/src/__tests__/session-history-row-actions.test.tsx +++ b/packages/ui/src/__tests__/session-history-row-actions.test.tsx @@ -68,7 +68,6 @@ const rowActions: SessionRowActions = { onArchive: () => undefined, onUnarchive: () => undefined, onRename: () => undefined, - onDelete: () => undefined, }; const project: ProjectRecord = { diff --git a/packages/ui/src/components.tsx b/packages/ui/src/components.tsx index 32acaa32a8..f528ca84c2 100644 --- a/packages/ui/src/components.tsx +++ b/packages/ui/src/components.tsx @@ -32,8 +32,9 @@ export { SessionRailProvider } from './session-rail-context.js'; export type { SessionRailChrome, SessionRailData, - SessionRailRowSelection, SessionRailSelection, + SessionRailSelectionCommands, + SessionRowPick, SessionViewMode, } from './session-rail-context.js'; export type { SidebarUpdateReminder } from './session-sidebar-nav.js'; diff --git a/packages/ui/src/conversation-copy.ts b/packages/ui/src/conversation-copy.ts index 1046134007..98ae0115c0 100644 --- a/packages/ui/src/conversation-copy.ts +++ b/packages/ui/src/conversation-copy.ts @@ -432,13 +432,12 @@ export interface ConversationCopy { promptRailAriaLabel: string; emptyPrompt: string; jumpToPrompt: (preview: string) => string; - selectRow: string; - selectionBarAriaLabel: string; - selectedCount: (selected: number, total: number) => string; - selectAllAriaLabel: string; - selectionArchive: string; - selectionDelete: string; - selectionClear: string; + /** Said about a picked row that is not the open one. */ + pickedAriaLabel: string; + /** The row menu's verbs when the picked set is more than this one row. */ + pinCount: (count: number) => string; + unpinCount: (count: number) => string; + archiveCount: (count: number) => string; }; } @@ -572,7 +571,7 @@ const CONVERSATION_COPY = { sessions: { status: { active: '可继续', running: '进行中', waiting_for_user: '等你确认', blocked: '需要处理', aborted: '已中止' }, blockedReason: { NO_REAL_CONNECTION: '等待配置可用模型连接', auth: '需要重新登录', permission_required: '等待权限确认', tool_failed: '工具调用失败', unknown: '运行中断，可重试' }, - listAriaLabel: '任务列表', showMore: '显示更多', showMoreAriaLabel: (count) => `显示 ${count} 条更多任务`, renameAriaLabel: '重命名任务', renameProjectTitle: '重命名项目', renameSubmit: '保存', respondingAriaLabel: '正在响应', respondingTitle: '任务正在流式响应中', staleTitle: '此任务使用的模型连接已不可用，发送时会切换到默认连接', staleAriaLabel: '任务已过期', stale: '已过期', unreadAriaLabel: '未读消息', actionsAriaLabel: (name) => `${name} 任务操作`, pin: '置顶', unpin: '取消置顶', rename: '重命名', archive: '归档', unarchive: '取消归档', delete: '删除', pinned: '置顶', recent: '最近', groupByTime: '按时间', groupByProject: '按项目', groupingAriaLabel: '任务分组方式', projectActionsAriaLabel: (name) => `${name} 项目操作`, projectNewTask: '新建任务', projectRename: '重命名', projectArchive: '归档', projectRestore: '恢复', projectRelink: '重新定位', projectUnavailable: '项目目录不可用', archivedProjects: '已归档项目', archivedProjectsAriaLabel: '展开已归档项目', worktreeAriaLabel: 'Git 工作树', promptRailAriaLabel: '按提问跳转', emptyPrompt: '（空提问）', jumpToPrompt: (preview) => `跳到提问：${preview}`, selectRow: '选择', selectionBarAriaLabel: '已选任务的批量操作', selectedCount: (selected, total) => `已选 ${selected} / ${total}`, selectAllAriaLabel: '全选或全不选', selectionArchive: '归档', selectionDelete: '删除', selectionClear: '取消', + listAriaLabel: '任务列表', showMore: '显示更多', showMoreAriaLabel: (count) => `显示 ${count} 条更多任务`, renameAriaLabel: '重命名任务', renameProjectTitle: '重命名项目', renameSubmit: '保存', respondingAriaLabel: '正在响应', respondingTitle: '任务正在流式响应中', staleTitle: '此任务使用的模型连接已不可用，发送时会切换到默认连接', staleAriaLabel: '任务已过期', stale: '已过期', unreadAriaLabel: '未读消息', actionsAriaLabel: (name) => `${name} 任务操作`, pin: '置顶', unpin: '取消置顶', rename: '重命名', archive: '归档', unarchive: '取消归档', delete: '删除', pinned: '置顶', recent: '最近', groupByTime: '按时间', groupByProject: '按项目', groupingAriaLabel: '任务分组方式', projectActionsAriaLabel: (name) => `${name} 项目操作`, projectNewTask: '新建任务', projectRename: '重命名', projectArchive: '归档', projectRestore: '恢复', projectRelink: '重新定位', projectUnavailable: '项目目录不可用', archivedProjects: '已归档项目', archivedProjectsAriaLabel: '展开已归档项目', worktreeAriaLabel: 'Git 工作树', promptRailAriaLabel: '按提问跳转', emptyPrompt: '（空提问）', jumpToPrompt: (preview) => `跳到提问：${preview}`, pickedAriaLabel: '已选中', pinCount: (count) => `置顶 ${count} 项`, unpinCount: (count) => `取消置顶 ${count} 项`, archiveCount: (count) => `归档 ${count} 项`, }, }, en: { @@ -730,7 +729,7 @@ const CONVERSATION_COPY = { sessions: { status: { active: 'Ready', running: 'Running', waiting_for_user: 'Waiting for you', blocked: 'Needs attention', aborted: 'Stopped' }, blockedReason: { NO_REAL_CONNECTION: 'Waiting for an available model connection', auth: 'Sign in again', permission_required: 'Waiting for permission', tool_failed: 'Tool call failed', unknown: 'Run interrupted; retry available' }, - listAriaLabel: 'Task list', showMore: 'Show more', showMoreAriaLabel: (count) => `Show ${count} more tasks`, renameAriaLabel: 'Rename task', renameProjectTitle: 'Rename project', renameSubmit: 'Save', respondingAriaLabel: 'Responding', respondingTitle: 'This task is streaming a response', staleTitle: 'This task\'s model connection is unavailable; sending will switch to the default connection', staleAriaLabel: 'Stale task', stale: 'Stale', unreadAriaLabel: 'Unread messages', actionsAriaLabel: (name) => `${name} task actions`, pin: 'Pin', unpin: 'Unpin', rename: 'Rename', archive: 'Archive', unarchive: 'Unarchive', delete: 'Delete', pinned: 'Pinned', recent: 'Recent', groupByTime: 'By time', groupByProject: 'By project', groupingAriaLabel: 'Task grouping', projectActionsAriaLabel: (name) => `${name} project actions`, projectNewTask: 'New task', projectRename: 'Rename', projectArchive: 'Archive', projectRestore: 'Restore', projectRelink: 'Relocate', projectUnavailable: 'Project directory unavailable', archivedProjects: 'Archived projects', archivedProjectsAriaLabel: 'Expand archived projects', worktreeAriaLabel: 'Git worktree', promptRailAriaLabel: 'Jump by prompt', emptyPrompt: '(empty prompt)', jumpToPrompt: (preview) => `Jump to prompt: ${preview}`, selectRow: 'Select', selectionBarAriaLabel: 'Bulk actions for selected tasks', selectedCount: (selected, total) => `${selected} / ${total} selected`, selectAllAriaLabel: 'Select all or none', selectionArchive: 'Archive', selectionDelete: 'Delete', selectionClear: 'Done', + listAriaLabel: 'Task list', showMore: 'Show more', showMoreAriaLabel: (count) => `Show ${count} more tasks`, renameAriaLabel: 'Rename task', renameProjectTitle: 'Rename project', renameSubmit: 'Save', respondingAriaLabel: 'Responding', respondingTitle: 'This task is streaming a response', staleTitle: 'This task\'s model connection is unavailable; sending will switch to the default connection', staleAriaLabel: 'Stale task', stale: 'Stale', unreadAriaLabel: 'Unread messages', actionsAriaLabel: (name) => `${name} task actions`, pin: 'Pin', unpin: 'Unpin', rename: 'Rename', archive: 'Archive', unarchive: 'Unarchive', delete: 'Delete', pinned: 'Pinned', recent: 'Recent', groupByTime: 'By time', groupByProject: 'By project', groupingAriaLabel: 'Task grouping', projectActionsAriaLabel: (name) => `${name} project actions`, projectNewTask: 'New task', projectRename: 'Rename', projectArchive: 'Archive', projectRestore: 'Restore', projectRelink: 'Relocate', projectUnavailable: 'Project directory unavailable', archivedProjects: 'Archived projects', archivedProjectsAriaLabel: 'Expand archived projects', worktreeAriaLabel: 'Git worktree', promptRailAriaLabel: 'Jump by prompt', emptyPrompt: '(empty prompt)', jumpToPrompt: (preview) => `Jump to prompt: ${preview}`, pickedAriaLabel: 'Selected', pinCount: (count) => `Pin ${count} tasks`, unpinCount: (count) => `Unpin ${count} tasks`, archiveCount: (count) => `Archive ${count} tasks`, }, }, } satisfies UiCatalog; diff --git a/packages/ui/src/session-history-list.tsx b/packages/ui/src/session-history-list.tsx index 51a136574e..7d9d986871 100644 --- a/packages/ui/src/session-history-list.tsx +++ b/packages/ui/src/session-history-list.tsx @@ -26,6 +26,7 @@ import { useRef, useState, type KeyboardEvent, + type MouseEvent, type ReactNode, type RefObject, } from 'react'; @@ -38,14 +39,12 @@ import { AlertTriangle, Archive, ArchiveRestore, - CircleCheckBig, FolderOpen, Pencil, Pin, PinOff, Plug, SquarePen, - Trash2, } from './icons.js'; import { RelativeTime } from './relative-time.js'; import { formatAbsoluteTimestamp } from '@maka/core/relative-time'; @@ -61,18 +60,18 @@ import { StatusDot, type StatusDotVariant } from '@astryxdesign/core/StatusDot'; import { describeBlockedReason, presentSessionStatus } from './session-status-presentation.js'; import { dotForStatus } from './status-vocabulary.js'; import { SessionRenameDialog, type SessionRenameTarget } from './session-rename-dialog.js'; -import { CheckboxInput } from '@astryxdesign/core/CheckboxInput'; import { type SessionRailData, useSessionRailData, - useSessionRailRowSelection, useSessionRailSelection, + type SessionRailSelectionCommands, + type SessionRowPick, } from './session-rail-context.js'; import { useUiLocale } from './locale-context.js'; import { getConversationCopy } from './conversation-copy.js'; import { getSessionHoverCardCopy } from './session-hover-card-copy.js'; -type SessionRowActionId = 'flag' | 'archive' | 'rename' | 'delete'; +type SessionRowActionId = 'flag' | 'archive' | 'rename'; type ProjectRowActionId = 'new' | 'relink' | 'rename' | 'archive' | 'restore'; type SessionHistoryGroupVariant = 'conversation' | 'project'; @@ -121,7 +120,6 @@ export interface SessionRowActions { onArchive(sessionId: string): void | Promise; onUnarchive(sessionId: string): void | Promise; onRename(sessionId: string, name: string): void | Promise; - onDelete(sessionId: string): void | Promise; } export interface ProjectRowActions { @@ -139,48 +137,171 @@ export interface SessionHistoryGroup { project?: ProjectRecord; } +/** + * The nearest ancestor of an event's target matching `selector`. + * + * Duck-typed rather than `instanceof Element`: this module is also rendered + * against linkedom, whose Element is not the ambient global, and the check + * would throw there rather than answer. + */ +function closestFrom(target: EventTarget | null, selector: string): HTMLElement | null { + const node = target as { closest?: (selector: string) => HTMLElement | null } | null; + return typeof node?.closest === 'function' ? node.closest(selector) : null; +} + +/** + * Whether a gesture may pick this row. + * + * The one place that answers it, because answering it twice is how a range + * comes to disagree with what the user sees. Two rows are rendered but not + * pickable: a row inside a collapsed project, which keeps its DOM node — + * `SideNavItem` collapses by grid track, not by unmounting — and a shared row + * projected from someone else's Host, which is handed no actions because + * there is nothing this window may do to it. Neither may be swept into a set, + * so neither may sit in the order a range is measured against. + */ +function isPickableRow(row: HTMLElement): boolean { + return row.dataset.actionable === 'true' && row.closest('[inert]') === null; +} + +/** The row a pointer event landed in, if it landed in a pickable one. */ +function pickableRowOf(target: EventTarget | null): HTMLElement | null { + const row = closestFrom(target, '.maka-session-row[data-session-id]'); + return row && isPickableRow(row) ? row : null; +} + +/** + * What a click on a row is asking for. + * + * Read in two places — the list picks the set, the row decides whether to + * navigate — so that "a modifier is a selection gesture, not navigation" is + * one sentence of code rather than a capture handler silencing a bubble one. + */ +function pickFor(event: Pick): SessionRowPick { + if (event.shiftKey) return 'range'; + return event.metaKey || event.ctrlKey ? 'toggle' : 'replace'; +} + export function SessionHistoryList() { const rail = useSessionRailData(); const selection = useSessionRailSelection(); const locale = useUiLocale(); + const listRef = useRef(null); + const commands = selection?.commands; - function handleListKeyDown(event: KeyboardEvent) { - if (event.key !== 'Escape' && event.key !== 'Delete' && event.key !== 'Backspace') return; - // The text-entry guard comes first, and now covers Escape too: a rename - // field is where Escape means "abandon this edit", and answering it by - // clearing the selection behind the dialog would be a second, invisible - // effect of one keypress. - const active = document.activeElement as HTMLElement | null; - if (!active || active.matches('input, textarea, [contenteditable="true"]')) return; - const selecting = selection?.active === true; - const marked = selecting && selection.selectedIds.size > 0; - if (event.key === 'Escape') { - // Escape leaves the mode, matching the 取消 button. Without it the only - // way out is finding that button, and a mode entered by accident from the - // row menu becomes one the user is stuck in. - if (!selecting) return; - event.preventDefault(); - selection.onExit(); + /** + * The rail's rendered order, read from the DOM at the moment of a click. + * + * A Shift range needs to know how far it may reach, and that is a property of + * the LIST. #4365's first revision handed each row its group's id array so a + * range could be computed inside the row; that array gets a fresh identity per + * render, which is a changed prop on every memoised row, and it turned a + * two-row session switch into a twelve-row one (#4109). Asked for here it + * costs one query per click and nothing per render — and it is the true order + * across groups, rather than a reconstruction of it. + * + * Rendered is not the same as reachable, so it is the pickable rows in that + * order: a range may only cross what the user can see and act on. + */ + function orderedSessionIds(): string[] { + const node = listRef.current; + if (!node) return []; + return [...node.querySelectorAll('.maka-session-row[data-session-id]')] + .filter(isPickableRow) + .map((row) => row.dataset.sessionId) + .filter((sessionId): sessionId is string => sessionId !== undefined); + } + + /** + * A menu is about a set, so opening one on a row outside the set replaces it + * — the way a file list answers a right-click on an unselected file. + * + * A row already in the set keeps the set: a run built by dragging must + * survive the gesture that asks what to do with it. But the set is narrowed + * to what is pickable at this instant, because the menu is the only entrance + * to a sweep and a sweep may act only on rows the user can see and act on. A + * pick does not have to leave the rail to stop being visible — its project + * collapses, or its session becomes a shared projection with no actions — and + * a set holding one of those would let a menu opened here archive a row that + * is not on screen. + * + * So the menu means the set it names: opening one FIXES the set at what is on + * screen, and a pick that was folded away is out of it for good, dismissed + * menu included. Folding does not narrow the set on its own — collapse lives + * as uncontrolled state inside `SideNavItem` and is only readable from the + * DOM, and this is the one moment that holds the set and the DOM together. + */ + function adoptForMenu(sessionId: string) { + if (!commands) return; + if (selection?.selectedIds.has(sessionId)) { + commands.retain(orderedSessionIds()); return; } - if (marked) { - // Delete is about the marked set once one exists. Deleting the focused - // row instead would act on one of several rows the user marked, which is - // the shape of an unrecoverable surprise. The sweep confirms first. - event.preventDefault(); - void selection?.onDeleteSelected(); + commands.pick({ sessionId, pick: 'replace', orderedSessionIds: [] }); + } + + function handleListClickCapture(event: MouseEvent) { + if (!commands) return; + const row = pickableRowOf(event.target); + const sessionId = row?.dataset.sessionId; + if (!row || !sessionId) return; + if (closestFrom(event.target, '.maka-session-row-action')) { + adoptForMenu(sessionId); return; } - const row = active.closest( - '[data-maka-contract="session-row"], [data-session-id]', - ); - const sessionId = - row?.dataset.sessionId ?? - row?.querySelector('[data-session-id]')?.dataset.sessionId; - if (sessionId && rail.rowActions) { - event.preventDefault(); - void rail.rowActions.onDelete(sessionId); - } + // The row's own button, not its hover card or any other descendant. + if (!closestFrom(event.target, 'button.astryx-side-nav-item')) return; + const pick = pickFor(event); + commands.pick({ + sessionId, + pick, + orderedSessionIds: pick === 'range' ? orderedSessionIds() : [], + openSessionId: rail.activeId, + }); + } + + /** + * Right-click opens the row's own ⋯ menu. + * + * Not a second menu mounted beside it. "⋯ and right-click agree" is then a + * fact rather than a promise kept by two lists of items that have to be + * maintained together — #4365 let them disagree, offering to act on one row + * from a ⋯ while twelve were marked. + * + * A row with no ⋯ has no menu to open, so this press is not ours: claiming it + * would take the native menu away and leave nothing in its place. + */ + function handleListContextMenu(event: MouseEvent) { + if (!commands) return; + const row = pickableRowOf(event.target); + const sessionId = row?.dataset.sessionId; + if (!row || !sessionId) return; + const trigger = row.querySelector('.maka-session-row-action button'); + event.preventDefault(); + adoptForMenu(sessionId); + // Opening the menu re-enters `handleListClickCapture` with a synthetic + // click, so the adoption is dispatched twice — harmless only because both + // of its branches are idempotent: `replace` sets the same one row, and + // `retain` intersects with the same list. Anything else here needs a guard. + trigger?.click(); + } + + function handleListKeyDown(event: KeyboardEvent) { + if (event.key !== 'Escape') return; + // Escape belongs to whatever is on top. A rename dialog or an open menu is + // above the rail and 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 claiming one here would clear the + // set AND leave the dialog with no way to close. + const active = document.activeElement as HTMLElement | null; + // The rail's only text field is the rename dialog's, so `dialog` already + // covers it and there is nothing else here for `input`/`textarea` to name. + if (!active || active.closest('dialog, [role="menu"]')) return; + // Escape drops the picks. The open task stays open and stays painted, so + // what the user sees is the selection collapsing back onto it. + if (!commands || selection.selectedIds.size === 0) return; + event.preventDefault(); + commands.clear(); } // Memoized on what it derives from. Rebuilt per render it would give @@ -210,7 +331,13 @@ export function SessionHistoryList() { // extra information for anyone hearing it. It is scroll content and a key // handler, nothing an assistive tech user needs to be told about separately. return ( -
+
); @@ -244,6 +371,24 @@ function SessionListGroups(props: { */ const renameOpenerRef = useRef(null); const [archivedExpanded, setArchivedExpanded] = useState(false); + // Read HERE and handed down as props, not read by the rows themselves. A + // context consumer re-renders on any change to the value it subscribes to, + // and the picked set now changes on an ordinary session switch — every row + // would redraw for a switch that changed two of them (#4109). This component + // is one fiber; the rows below it are ~1,000. + const selection = useSessionRailSelection(); + const selectedIds = selection?.selectedIds; + const pickedCount = selectedIds?.size ?? 0; + // Whether a set-wide pin should read 置顶 or 取消置顶. Every row already + // pinned means the verb unpins; a mixed set pins, which is the one rule that + // keeps a set-wide toggle from being ambiguous. + const allPickedPinned = useMemo(() => { + if (!selectedIds || selectedIds.size === 0) return false; + const pinned = new Set( + rail.sessions.filter((session) => session.isFlagged).map((session) => session.id), + ); + return [...selectedIds].every((sessionId) => pinned.has(sessionId)); + }, [rail.sessions, selectedIds]); const startRename = useCallback((target: SessionRenameTarget, opener: HTMLElement | null) => { renameOpenerRef.current = opener; @@ -278,24 +423,35 @@ function SessionListGroups(props: { // any one of them changing identity upstream rebuilt every row. It arrives as // `rail` now, so this array says what it always meant (#4109). const renderSessionRow = useCallback( - (session: SessionSummary): ReactNode => ( - - ), - [rail, startRename], + (session: SessionSummary): ReactNode => { + const picked = selectedIds?.has(session.id) ?? false; + // Scoped to picked rows on purpose. A count handed to every row would be + // a changed prop on every row each time the set grows; this way a plain + // click — which picks the row it opens — changes exactly two. + const bulk = picked && pickedCount > 1; + return ( + + ); + }, + [allPickedPinned, pickedCount, rail, selectedIds, selection?.commands, startRename], ); // Keyed per target so the field seeds from the name that row carries now, @@ -488,6 +644,12 @@ const ProjectHoverCardLayer = memo(function ProjectHoverCardLayer(props: { const SessionNavRow = memo(function SessionNavRow(props: { session: SessionSummary; active: boolean; + /** Part of the picked set. The open row is painted the same way. */ + picked: boolean; + /** How many rows a menu opened here would act on; 0 when it is this row alone. */ + bulkCount: number; + bulkAllPinned: boolean; + selectionCommands?: SessionRailSelectionCommands; streaming: boolean; stale: boolean; worktree: boolean; @@ -500,13 +662,6 @@ const SessionNavRow = memo(function SessionNavRow(props: { const containerRef = useRef(null); const hoverDescriptionId = useId(); const locale = useUiLocale(); - // The ROW half of the selection, not the whole of it. Read here rather than - // threaded through `renderSessionRow`, which is memoized on the rail value — - // but read from the narrow context, because a consumer re-renders on any - // change to the value it subscribes to and the wide one carries - // `listedSessionIds`, which moves whenever the catalog does (#4109). - const selection = useSessionRailRowSelection(); - const marked = selection?.selectedIds.has(props.session.id) ?? false; const copy = getConversationCopy(locale).sessions; const signals = sessionRowSignals( props.session, @@ -529,6 +684,16 @@ const SessionNavRow = memo(function SessionNavRow(props: { // the dot. const rowDescription = [ ...signals.slice(1).map((entry) => entry.tooltip ?? entry.label), + // Being picked is a fact about the row that the ground alone carries. It is + // NOT `aria-current`: that names the one current page, and a set of picked + // rows is not a set of current pages. + // + // The open row is exempt only while it is the whole set — the state a plain + // click leaves, where "selected" would say nothing `isSelected` has not. + // Inside a real run it is one of several, and `bulkCount` is how the row + // already knows: nonzero on picked rows alone, and above 1 only when the + // menu would offer a sweep. + props.picked && (!props.active || props.bulkCount > 1) ? copy.pickedAriaLabel : undefined, props.worktree ? copy.worktreeAriaLabel : undefined, props.meta, props.session.lastMessageAt @@ -546,24 +711,12 @@ const SessionNavRow = memo(function SessionNavRow(props: { data-session-id={props.session.id} data-stale={props.stale ? 'true' : undefined} data-worktree={props.worktree ? 'true' : undefined} - data-selected={marked ? 'true' : undefined} - data-selecting={selection?.active ? 'true' : undefined} + data-picked={props.picked ? 'true' : undefined} + // What the list reads to know this row can be picked. Having actions is + // the same fact as being ours to act on, so the two cannot drift: a + // shared row gets none, and is a plain navigation item all the way down. + data-actionable={props.actions ? 'true' : undefined} > - {/* Outside the SideNavItem, not in its icon slot: that slot is the status - dot's fixed gutter, and a checkbox there would take the one column the - rail uses to say what a task is doing. The box gets its own leading - column, which only exists while the mode does. */} - {selection?.active ? ( - - selection.onToggleRow(props.session.id, checked)} - /> - - ) : null} { + // Shift- and ⌘-clicks are answered by the list, which has already + // moved the set by the time this runs. Opening the task as well + // would move the main pane away from the run being built. Where + // nothing listens for picks — or this row cannot be picked at all — + // the modifier means nothing, and the row is a plain navigation item + // again rather than a dead click. + if (props.actions && props.selectionCommands && pickFor(event) !== 'replace') return; if (event.detail > 1 && props.actions) { props.onStartRename( { @@ -649,6 +809,9 @@ const SessionNavRow = memo(function SessionNavRow(props: { )} @@ -998,17 +1161,25 @@ function ProjectItemActions(props: { ); } +/** + * The row's ⋯ menu, and the menu right-click opens — one implementation, so + * they cannot disagree. + * + * Its verbs count the set: with seven rows picked and the menu opened on one of + * them it offers 归档 7 项, because acting on the one row under the cursor while + * six others are visibly picked is the shape of a surprise. `bulkCount` is 0 + * whenever the menu is about this row alone, which is both "nothing is picked" + * and "only this row is". + */ function SessionItemActions(props: { session: SessionSummary; actions: SessionRowActions; + bulkCount: number; + bulkAllPinned: boolean; + selectionCommands?: SessionRailSelectionCommands; onStartRename(target: SessionRenameTarget, opener: HTMLElement | null): void; }) { const trailingRef = useRef(null); - // The way in. ⌘-click is invisible to anyone who has not been told about it, - // and this menu is where a person already looks for what a row can do — so - // selection is discoverable from the same place as pin, rename, and archive. - // The narrow context again: this renders once per row. - const selection = useSessionRailRowSelection(); const locale = useUiLocale(); const copy = getConversationCopy(locale).sessions; const actionContext = [ @@ -1068,66 +1239,73 @@ function SessionItemActions(props: { pendingMenuIntentRef.current = null; if (intent) window.requestAnimationFrame(intent); }} - items={[ - ...(selection && !selection.active + items={ + props.bulkCount > 1 && props.selectionCommands ? [ { - label: copy.selectRow, - icon: CircleCheckBig, - // Enters the mode AND marks this row: the person picked a - // row, not an abstract mode, and landing them in an empty - // selection would discard the choice they just made. - onClick: () => selection.onEnter(props.session.id), + label: props.bulkAllPinned + ? copy.unpinCount(props.bulkCount) + : copy.pinCount(props.bulkCount), + icon: props.bulkAllPinned ? PinOff : Pin, + onClick: () => + void props.selectionCommands?.flagSelected(!props.bulkAllPinned), + }, + // No 重命名: renaming names ONE task, and there is no honest + // way to ask a dialog for seven names at once. + { + label: copy.archiveCount(props.bulkCount), + icon: Archive, + onClick: () => void props.selectionCommands?.archiveSelected(), }, ] - : []), - { - label: props.session.isFlagged ? copy.unpin : copy.pin, - icon: props.session.isFlagged ? PinOff : Pin, - onClick: () => - runRowAction('flag', () => - actions.onToggleFlag(props.session.id, !props.session.isFlagged), - ), - }, - { - label: copy.rename, - icon: Pencil, - onClick: () => { - // Read now, while the trigger is still the thing the user is on: - // by the time the intent runs the menu has closed and focus is - // mid-handover. - const opener = trailingRef.current?.querySelector('button') ?? null; - pendingMenuIntentRef.current = () => - props.onStartRename( - { - kind: 'session', - id: props.session.id, - name: props.session.name, + : [ + { + label: props.session.isFlagged ? copy.unpin : copy.pin, + icon: props.session.isFlagged ? PinOff : Pin, + onClick: () => + runRowAction('flag', () => + actions.onToggleFlag(props.session.id, !props.session.isFlagged), + ), + }, + { + label: copy.rename, + icon: Pencil, + onClick: () => { + // Read now, while the trigger is still the thing the user + // is on: by the time the intent runs the menu has closed + // and focus is mid-handover. + const opener = + trailingRef.current?.querySelector('button') ?? null; + pendingMenuIntentRef.current = () => + props.onStartRename( + { + kind: 'session', + id: props.session.id, + name: props.session.name, + }, + opener, + ); }, - opener, - ); - }, - }, - { - label: props.session.isArchived ? copy.unarchive : copy.archive, - icon: props.session.isArchived ? ArchiveRestore : Archive, - onClick: () => - runRowAction('archive', () => - props.session.isArchived - ? actions.onUnarchive(props.session.id) - : actions.onArchive(props.session.id), - ), - }, - { type: 'divider' }, - { - label: copy.delete, - icon: Trash2, - onClick: () => { - pendingMenuIntentRef.current = () => - runRowAction('delete', () => actions.onDelete(props.session.id)); - }, - }, - ]} + }, + // Archive is where the rail stops. Deleting 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. It lives in Settings › 已归档任务 instead — + // reachable only for a task already archived, which is the step + // that makes the intent deliberate. + { + label: props.session.isArchived ? copy.unarchive : copy.archive, + icon: props.session.isArchived ? ArchiveRestore : Archive, + onClick: () => + runRowAction('archive', () => + props.session.isArchived + ? actions.onUnarchive(props.session.id) + : actions.onArchive(props.session.id), + ), + }, + ] + } /> ); diff --git a/packages/ui/src/session-list-panel.tsx b/packages/ui/src/session-list-panel.tsx index 71bc4502b2..1f7522162c 100644 --- a/packages/ui/src/session-list-panel.tsx +++ b/packages/ui/src/session-list-panel.tsx @@ -23,7 +23,6 @@ import { } from '@astryxdesign/core/SegmentedControl'; import { SideNav } from '@astryxdesign/core/SideNav'; import { SessionHistoryList } from './session-history-list.js'; -import { SessionSelectionBar } from './session-selection-bar.js'; import { useSessionRailChrome, type SessionViewMode, @@ -129,25 +128,7 @@ export function SessionListPanel() { } footer={} > - {/* The selection bar belongs to the LIST, not to the chrome, and is - rendered here rather than in `topContent` for that reason. SideNav - draws one hairline under the whole top region, so a bar up there - landed above that line — grouped with 按时间 / 按项目 and cut off - from the rows it governs, with its own hairline making a second line - 9px from the first. - - It is `position: sticky` inside the scroller instead, so it still - does not scroll away while the user marks rows further down. - - `SESSION_HISTORY_LIST` keeps its element identity through this - fragment, so a selection change still skips the ~1,000 fibers under - it (#4109). */} - {!collapsed ? ( - <> - - {SESSION_HISTORY_LIST} - - ) : null} + {!collapsed ? SESSION_HISTORY_LIST : null}
); diff --git a/packages/ui/src/session-rail-context.tsx b/packages/ui/src/session-rail-context.tsx index 17bdcc3f85..74cdf0ab68 100644 --- a/packages/ui/src/session-rail-context.tsx +++ b/packages/ui/src/session-rail-context.tsx @@ -93,64 +93,98 @@ export interface SessionRailChrome { }; } +/** What a click on a session row means, decided by the modifier held with it. */ +export type SessionRowPick = + /** + * Plain click, and the adoption a menu makes when it opens on an unpicked + * row: the set becomes exactly this row. Opening the task is the row's own + * business — this names what happens to the SET. + */ + | 'replace' + /** ⌘/Ctrl: add or remove this one row, leaving the rest of the set alone. */ + | 'toggle' + /** Shift: pick the contiguous run between the anchor and this row. */ + | 'range'; + /** - * What a ROW reads: whether the mode is on, whether this row is marked, and the - * two ways a row changes that. + * The commands a picked set can be asked for, held apart from the set itself so + * their identity never moves. * - * Split from `SessionRailSelection` because a context consumer re-renders - * whenever the value it subscribes to changes, and `memo` cannot stop it. The - * wide value carries `listedSessionIds`, which is derived from the catalog and - * gets a fresh identity on a session switch — a row reading that would - * re-render along with every other row for a switch that changed two of them. - * The e2e render contract measures exactly this and budgets 2 rows (#4109). - * - * This half moves only when the selection itself does. + * Rows receive these as a prop rather than reading them from context. A context + * consumer re-renders whenever the value it subscribes to changes and `memo` + * cannot stop it, and the set DOES change on an ordinary session switch now + * that a plain click picks the row it opens — every row would redraw for a + * switch that changed two of them, which is what the rail's render contract + * budgets against (#4109). */ -export interface SessionRailRowSelection { - active: boolean; - selectedIds: ReadonlySet; - onToggleRow(sessionId: string, selected: boolean): void; - onEnter(sessionId?: string): void; +export interface SessionRailSelectionCommands { + /** + * One click on one row. `orderedSessionIds` is the rail's rendered order, + * read from the DOM at the moment of the click, so a range knows how far it + * may reach without any row having to carry the list around. + */ + pick(input: { + sessionId: string; + pick: SessionRowPick; + orderedSessionIds: readonly string[]; + /** The open task, which anchors a range when no click has set an anchor. */ + openSessionId?: string; + }): void; + /** Drops the picks. The open row stays open, and stays painted. */ + clear(): void; + /** + * Narrows the set to its intersection with `sessionIds`, anchor included. + * + * For the menu, and only for the menu: a sweep may act only on rows the user + * could see and act on at the moment they pressed. A row does not have to + * leave the rail to stop being one — collapsing its project keeps it mounted + * and merely `inert`, and a session that becomes a shared projection loses + * its actions in place — so `pruneSessionSelection` against the catalog, which + * still lists both, cannot answer this. + * + * The invariant it buys: THE MENU MEANS THE SET IT NAMES. At the instant a + * menu opens, the set becomes exactly the rows the user can see and act on, + * and a pick that was folded away or turned into a shared projection leaves + * it then — permanently, whether the menu is used or dismissed. Reopening the + * project does not bring it back, because the set no longer holds it. + * + * Not narrowed when the project actually collapses, and not derived during + * render, for the same reason: collapse is uncontrolled state inside Astryx's + * `SideNavItem`, which 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, and it is + * also the single entrance to `archiveSelected` and `flagSelected` — so it is + * where the two are reconciled. + */ + retain(sessionIds: readonly string[]): void; + /** + * The rail's only sweep. There is no delete here: it cannot be undone, and it + * belongs to Settings › 已归档任务, which can only reach a task that was + * archived first. + */ + archiveSelected(): void | Promise; + /** Pins or unpins the whole picked set. */ + flagSelected(flagged: boolean): void | Promise; } /** - * What the selection BAR reads: the row half plus everything only the bar - * needs — what "all" means, the sweeps, and whether one is running. + * The rail's multi-select. * - * A THIRD context, for the reason the chrome is a second one: it changes as the - * user marks rows while the list does not, and folding it into - * `SessionRailData` would give that value a new identity per click — the - * ~1,000-fiber render that split exists to prevent (#4109). + * A SECOND context beside `SessionRailData`, for the reason the chrome is a + * third one: it changes as the user picks rows while the list does not, and + * folding it into the data value would give that value a new identity per + * click — the ~1,000-fiber render that split exists to prevent (#4109). * - * Absent means the rail has no multi-select: rows navigate, nothing marks, and - * a surface that never wired it up renders exactly as before. + * Absent means the rail has no multi-select: rows navigate, nothing is picked, + * and a surface that never wired it up renders exactly as before. */ export interface SessionRailSelection { - /** - * Whether the rail is in selection mode: rows carry a checkbox and the master - * row is above them. Distinct from an empty `selectedIds` — unticking the - * master box selects none, it does not leave. - */ - active: boolean; selectedIds: ReadonlySet; - /** Every row the rail is listing, in rendered order. What "all" means. */ - listedSessionIds: readonly string[]; - onToggleRow(sessionId: string, selected: boolean): void; - onEnter(sessionId?: string): void; - /** Leaves the mode and drops what was marked. */ - onExit(): void; - onToggleAll(selected: boolean): void; - onArchiveSelected(): void | Promise; - onDeleteSelected(): void | Promise; - /** A sweep is running. The commands disable while one is, so a second click - * cannot ask for the same set twice. */ - busy?: boolean; + commands: SessionRailSelectionCommands; } const SessionRailDataContext = createContext(null); const SessionRailChromeContext = createContext(null); const SessionRailSelectionContext = createContext(null); -const SessionRailRowSelectionContext = createContext(null); /** * `chrome` is optional so the list can be rendered on its own — a test or a @@ -161,22 +195,13 @@ export function SessionRailProvider(props: { data: SessionRailData; chrome?: SessionRailChrome; selection?: SessionRailSelection; - /** - * The rows' half. Supplied separately rather than derived here so its - * identity is the producer's business: deriving it in this component would - * rebuild it on every render of the tree above, which is the churn the split - * exists to avoid. - */ - rowSelection?: SessionRailRowSelection; children?: ReactNode; }) { return ( - - {props.children} - + {props.children} @@ -202,8 +227,3 @@ export function useSessionRailChrome(): SessionRailChrome { export function useSessionRailSelection(): SessionRailSelection | null { return useContext(SessionRailSelectionContext); } - -/** What a row reads. Null when the rail has no multi-select. */ -export function useSessionRailRowSelection(): SessionRailRowSelection | null { - return useContext(SessionRailRowSelectionContext); -} diff --git a/packages/ui/src/session-selection-bar.tsx b/packages/ui/src/session-selection-bar.tsx deleted file mode 100644 index 5c36c440bf..0000000000 --- a/packages/ui/src/session-selection-bar.tsx +++ /dev/null @@ -1,106 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { Button } from '@astryxdesign/core/Button'; -import { CheckboxInput } from '@astryxdesign/core/CheckboxInput'; -import { getConversationCopy } from './conversation-copy.js'; -import { ICON_SIZE, Archive, Trash2 } from './icons.js'; -import { useUiLocale } from './locale-context.js'; -import { useSessionRailSelection } from './session-rail-context.js'; - -/** - * The rail's selection mode, headed. - * - * The master box sits directly above the rows it governs and in the same - * leading column as theirs, so "all of these" is a claim the eye can check - * rather than a label to be trusted. It is `indeterminate` whenever some but - * not all listed rows are marked — the one state a plain checked/unchecked pair - * cannot express, and the usual state during a selection. - * - * "All" means every row the rail is listing, which is what sits under the box. - * Not every task in the catalog: a box that silently included rows behind a - * collapsed project would name a number the user never agreed to. - * - * The commands are `secondary`, not `ghost`: ghost renders as bare text, and a - * label with no container beside a checkbox and a count does not read as - * something to press. Both carry the same container so neither is the primary — - * one of them is destructive and must not be emphasised by accident. - * - * TWO ROWS, because one does not fit. The rail is 180px at its narrowest and - * 260px by default; the first attempt put count and three text buttons on one - * line, which measured 228px of a 244px bar and squeezed the count to 12px, - * where it wrapped one character per line. Vertical space in a rail is cheap. - */ -export function SessionSelectionBar() { - const selection = useSessionRailSelection(); - const copy = getConversationCopy(useUiLocale()).sessions; - if (!selection?.active) return null; - const listed = selection.listedSessionIds; - const count = selection.selectedIds.size; - const busy = selection.busy === true; - const allMarked = listed.length > 0 && listed.every((id) => selection.selectedIds.has(id)); - const master: boolean | 'indeterminate' = count === 0 ? false : allMarked ? true : 'indeterminate'; - return ( -
-
- selection.onToggleAll(checked)} - /> - {/* `aria-live` so the count reaches a screen reader as it changes: the - bar is not focused while the user is ticking rows, so nothing else - would say how many are marked. */} - - {copy.selectedCount(count, listed.length)} - -
-
-
-
- ); -} diff --git a/packages/ui/stories/session-list-panel.stories.tsx b/packages/ui/stories/session-list-panel.stories.tsx index d1bb023a70..32f89c84ad 100644 --- a/packages/ui/stories/session-list-panel.stories.tsx +++ b/packages/ui/stories/session-list-panel.stories.tsx @@ -79,7 +79,6 @@ const rowActions: NonNullable = { onArchive: noop, onUnarchive: noop, onRename: noop, - onDelete: noop, }; function panelProps(input: { diff --git a/packages/ui/stories/session-rail-harness.tsx b/packages/ui/stories/session-rail-harness.tsx index b3dc878164..9a654bc7ef 100644 --- a/packages/ui/stories/session-rail-harness.tsx +++ b/packages/ui/stories/session-rail-harness.tsx @@ -22,12 +22,20 @@ import { SessionRailProvider, type SessionRailChrome, type SessionRailData, + type SessionRailSelection, } from '../src/session-rail-context.js'; export type SessionRailStoryProps = Partial & Partial & Pick & - Pick; + Pick & { + /** + * The multi-select context, which the app supplies from + * `useSessionSelection`. Named apart from `selection` because that one is + * already the shell's NavSelection. + */ + railSelection?: SessionRailSelection; + }; /** * The rail, described as one flat bag of state. @@ -71,7 +79,11 @@ export function SessionRail(props: SessionRailStoryProps) { workHubEntry: props.workHubEntry, }; return ( - + );