Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions apps/desktop/e2e/parent-session-deletion.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,11 @@ test('deleting a parent task archives its linked subagent task', async ({
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();
await archivedTasks
.getByRole('button', { name: `彻底删除「${PARENT_REMOVAL_PARENT_NAME}」`, exact: true })
.click();
const confirm = page.getByRole('alertdialog', {
name: `删除 "${PARENT_REMOVAL_PARENT_NAME}"`,
});
Expand All @@ -75,4 +74,16 @@ test('deleting a parent task archives its linked subagent task', async ({
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);

const unarchive = archivedTasks.getByRole('button', {
name: `取消归档「${PARENT_REMOVAL_CHILD_NAME}」`,
});
const remove = archivedTasks.getByRole('button', {
name: `彻底删除「${PARENT_REMOVAL_CHILD_NAME}」`,
});
await expect(unarchive).toBeVisible();
await expect(remove).toBeVisible();
await expect(unarchive.locator('svg[aria-hidden="true"]')).toHaveCount(1);
await expect(remove.locator('svg[aria-hidden="true"]')).toHaveCount(1);
await expect(archivedTasks.getByRole('button', { name: /更多操作/ })).toHaveCount(0);
});
18 changes: 9 additions & 9 deletions apps/desktop/src/renderer/locales/settings-tasks-copy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ export type SettingsTasksCopy = {
purgeUnverified: string;
noMatchTitle: string;
noMatchBody: string;
moreActions(name: string): string;
restore: string;
restoreTask(name: string): string;
unarchive: string;
unarchiveTask(name: string): string;
delete: string;
deleteTask(name: string): string;
emptyTitle: string;
emptyBody: string;
};
Expand All @@ -80,10 +80,10 @@ const SETTINGS_TASKS_COPY_BY_LOCALE = {
purgeUnverified: '任务已删除,但无法读取列表确认结果。请重新打开本页查看。',
noMatchTitle: '没有匹配的任务',
noMatchBody: '换个关键词试试。',
moreActions: (name: string) => `「${name}」的更多操作`,
restore: '恢复',
restoreTask: (name: string) => `恢复「${name}」`,
unarchive: '取消归档',
unarchiveTask: (name: string) => `取消归档「${name}」`,
delete: '彻底删除',
deleteTask: (name: string) => `彻底删除「${name}」`,
emptyTitle: '没有已归档的任务',
emptyBody: '在侧栏里归档一个任务后,可以在这里恢复或彻底删除它。',
},
Expand Down Expand Up @@ -115,10 +115,10 @@ const SETTINGS_TASKS_COPY_BY_LOCALE = {
purgeUnverified: 'The tasks were deleted, but the list could not be read back to confirm. Reopen this page to check.',
noMatchTitle: 'No matching tasks',
noMatchBody: 'Try a different search.',
moreActions: (name: string) => `More actions for ${name}`,
restore: 'Restore',
restoreTask: (name: string) => `Restore ${name}`,
unarchive: 'Unarchive',
unarchiveTask: (name: string) => `Unarchive ${name}`,
delete: 'Delete',
deleteTask: (name: string) => `Delete ${name}`,
emptyTitle: 'Nothing archived',
emptyBody: 'Archive a task from the rail to restore or permanently delete it here.',
},
Expand Down
26 changes: 14 additions & 12 deletions apps/desktop/src/renderer/settings/tasks-settings-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import { useCallback, useMemo, useState } from 'react';
import type { ProjectRecord } from '@maka/core/project';
import { formatCompactTimestamp } from '@maka/core/relative-time';
import { runtimeHostProfileUsesHostWorkspace } from '@maka/runtime-host/profile-kind';
import { Button, EmptyState, MoreMenu, useMountedRef, useToast, useUiLocale } from '@maka/ui';
import { Archive, ICON_SIZE, Search } from '@maka/ui/icons';
import { Button, EmptyState, IconButton, useMountedRef, useToast, useUiLocale } from '@maka/ui';
import { Archive, ICON_SIZE, Search, Trash2, Unarchive } from '@maka/ui/icons';
import { HStack, StackItem } from '@astryxdesign/core';
import { List, ListItem } from '@astryxdesign/core/List';
import { TextInput } from '@astryxdesign/core/TextInput';
Expand Down Expand Up @@ -236,25 +236,27 @@ export function TasksSettingsPage(props: ArchivedTasksBridge) {
startContent={<Archive size={ICON_SIZE.control} aria-hidden="true" />}
endContent={
<>
<Button
variant="secondary"
<IconButton
variant="ghost"
size="sm"
isDisabled={purging}
clickAction={() => props.onRestore(session.id)}
label={copy.restore}
// Every row's button reads 恢复; only the accessible
// name can say which task it restores.
aria-label={copy.restoreTask(session.name)}
label={copy.unarchiveTask(session.name)}
tooltip={copy.unarchive}
icon={<Unarchive size={ICON_SIZE.control} aria-hidden="true" />}
/>
{/* No 打开 here. An archived task has no rail row to
land on, and giving it one would make "the open task
is always visible in the rail" an invariant the rail
does not otherwise hold. Restore first. */}
<MoreMenu
label={copy.moreActions(session.name)}
does not otherwise hold. Unarchive first. */}
<IconButton

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: permanent delete is now one click, in the same variant="ghost" and size="sm" as the unarchive button directly beside it, distinguished only by the glyph. The confirmation alertdialog still stands and the e2e asserts it, so this is recoverable and not blocking. But the overflow menu used to be the deceleration, and the rail's own comment about deletion being the one action that cannot be undone is the reason that surface exists. Worth giving it a destructive-toned icon, or putting some space between it and the safe action, so the two do not read as interchangeable at 32px.

variant="ghost"
size="sm"
isDisabled={purging}
items={[{ label: copy.delete, onClick: () => props.onDelete(session.id) }]}
clickAction={() => props.onDelete(session.id)}
label={copy.deleteTask(session.name)}
tooltip={copy.delete}
icon={<Trash2 size={ICON_SIZE.control} aria-hidden="true" />}
/>
</>
}
Expand Down
2 changes: 1 addition & 1 deletion docs/astryx-surface-file-inventory.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ Wiki bar: Design Conventions · API Use-the-System · Theming · Container Paddi
| `apps/desktop/src/renderer/settings/settings-status-summary-filter.tsx` | settings-module | Button | aligned — uses Astryx (Button) | aligned |
| `apps/desktop/src/renderer/settings/settings-surface.tsx` | settings-module | Badge, Banner, Button, IconButton, Layout, LayoutContent, LayoutHeader, LayoutPanel, Selector, SideNav, SideNavItem, SideNavSection | aligned — uses Astryx (Badge, Banner, Button, IconButton, Layout, LayoutContent, LayoutHeader, LayoutPanel) | aligned |
| `apps/desktop/src/renderer/settings/subagent-settings-page.tsx` | settings-page | Badge, Banner, Button, EmptyState, HStack, IconButton, Selector, Switch, TextArea, TextInput, VStack | aligned — uses Astryx (Badge, Banner, Button, EmptyState, HStack, IconButton, Selector, Switch) | aligned |
| `apps/desktop/src/renderer/settings/tasks-settings-page.tsx` | settings-page | Button, EmptyState, HStack, List, ListItem, MoreMenu, StackItem, TextInput | aligned — uses Astryx (Button, EmptyState, HStack, List, ListItem, MoreMenu, StackItem, TextInput) | aligned |
| `apps/desktop/src/renderer/settings/tasks-settings-page.tsx` | settings-page | Button, EmptyState, HStack, IconButton, List, ListItem, StackItem, TextInput | aligned — uses Astryx (Button, EmptyState, HStack, IconButton, List, ListItem, StackItem, TextInput) | aligned |
| `apps/desktop/src/renderer/settings/usage-settings-page.tsx` | settings-page | Banner, Button, Card, EmptyState, SegmentedControl, SegmentedControlItem, Selector, Switch, Tab, TabList, Table, TextInput, Tooltip | aligned — uses Astryx (Banner, Button, Card, EmptyState, SegmentedControl, SegmentedControlItem, Selector, Switch) | aligned |
| `apps/desktop/src/renderer/settings/web-search-settings-page.tsx` | settings-page | Banner, Button, EmptyState, Link, Selector, StatusDot, Switch, TextInput | aligned — uses Astryx (Banner, Button, EmptyState, Link, Selector, StatusDot, Switch, TextInput) | aligned |
| `apps/desktop/src/renderer/styles.css` | styles | n/a (css) | aligned — no off-rhythm control heights flagged | aligned |
Expand Down
39 changes: 39 additions & 0 deletions packages/ui/src/__tests__/icons.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* 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 assert from 'node:assert/strict';
import test from 'node:test';
import type { ReactElement } from 'react';
import { renderToStaticMarkup } from 'react-dom/server';
import { parseHTML } from 'linkedom';
import { Archive, Unarchive } from '../icons.js';

function iconPathData(icon: ReactElement): string[] {
const { document } = parseHTML(renderToStaticMarkup(icon));
return [...document.querySelectorAll('path')].map((path) => path.getAttribute('d') ?? '');
}

test('archive and unarchive use one tray with inverse arrows', () => {
const archive = iconPathData(<Archive />);
const unarchive = iconPathData(<Unarchive />);

assert.deepEqual(archive.slice(0, 2), unarchive.slice(0, 2));
assert.deepEqual(archive.slice(2), ['M12 3v14', 'm7 12 5 5 5-5']);
assert.deepEqual(unarchive.slice(2), ['M12 17V3', 'm7 8 5-5 5 5']);
});
22 changes: 20 additions & 2 deletions packages/ui/src/icons.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,28 @@
* UI icons.
*/

import { createLucideIcon, type IconNode } from 'lucide-react';

export type { LucideIcon, LucideProps } from 'lucide-react';

const ARCHIVE_TRAY_NODES: IconNode = [
['path', { d: 'M4 12v7a2 2 0 0 0 2 2h12a2 2 0 0 0 2-2v-7', key: 'tray' }],
['path', { d: 'M2 12h20', key: 'tray-edge' }],
];

/** A matched pair: the same tray with the arrow entering or leaving it. */
export const Archive = createLucideIcon('archive', [

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: this file's own header two lines up says it is the single seam between Maka call sites and the underlying library, and that generic UI icons come directly from lucide-react. Defining glyphs here makes it a drawing surface too, and the header is unchanged. DESIGN.md also lists "another icon system" in its Don't list; two glyphs are not a system yet, but the precedent it sets is that a lucide mark gets redrawn when it is not symmetric enough.

The motivation is fair. Lucide ships no archive-down counterpart, and Archive (lid rect, box, handle) against ArchiveRestore (same lid rect, open box, up arrow) is a pair only in the sense that they share the lid, not an inverse. But that is a design decision worth its own change rather than a passenger on a PR about flattening an overflow menu, and it is not what #4370 asked for.

The blast radius is also unevidenced: this Archive replaces lucide's at every call site, including the rail's row menus (session-history-list.tsx:1139, :1257, :1299) and the settings section icon, while the before/after images cover only the archived list. Smallest path: keep lucide's Archive / ArchiveRestore here and land only the button change, then propose the pair separately. If it stays, please add the rail menu before/after and update the header comment to describe what this file now does.

...ARCHIVE_TRAY_NODES,
['path', { d: 'M12 3v14', key: 'arrow-stem' }],
['path', { d: 'm7 12 5 5 5-5', key: 'arrow-head' }],
]);

export const Unarchive = createLucideIcon('unarchive', [
...ARCHIVE_TRAY_NODES,
['path', { d: 'M12 17V3', key: 'arrow-stem' }],
['path', { d: 'm7 8 5-5 5 5', key: 'arrow-head' }],
]);

/**
* The five-rung icon scale. Pick by the role the glyph plays, not by eye.
*
Expand All @@ -54,8 +74,6 @@ export {
AlertCircle,
AlertOctagon,
AlertTriangle,
Archive,
ArchiveRestore,
ArrowDown,
ArrowLeft,
ArrowRight,
Expand Down
6 changes: 3 additions & 3 deletions packages/ui/src/session-history-list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ import {
ICON_SIZE,
AlertTriangle,
Archive,
ArchiveRestore,
FolderOpen,
Pencil,
Pin,
PinOff,
Plug,
SquarePen,
Unarchive,
} from './icons.js';
import { RelativeTime } from './relative-time.js';
import { formatAbsoluteTimestamp } from '@maka/core/relative-time';
Expand Down Expand Up @@ -1101,7 +1101,7 @@ function ProjectItemActions(props: {
? [
{
label: copy.projectRestore,
icon: ArchiveRestore,
icon: Unarchive,
onClick: () => runProjectAction('restore', () => actions.onRestore(project.id)),
},
]
Expand Down Expand Up @@ -1296,7 +1296,7 @@ function SessionItemActions(props: {
// that makes the intent deliberate.
{
label: props.session.isArchived ? copy.unarchive : copy.archive,
icon: props.session.isArchived ? ArchiveRestore : Archive,
icon: props.session.isArchived ? Unarchive : Archive,
onClick: () =>
runRowAction('archive', () =>
props.session.isArchived
Expand Down