From 4488806f9d893c633757ab3d589e55be169c1e5a Mon Sep 17 00:00:00 2001 From: Ali Al Dallal Date: Fri, 28 Aug 2026 21:27:15 -0400 Subject: [PATCH 1/2] fix: the Extensions list gets its industry-standard treatment -- grouped, noun-named, contained details, no escaping knobs Five hands-on-review findings against Settings > Extensions: - The toggle knob's own compositor layer could paint outside its row after scrolling with a disclosure open; contain: paint on the row scopes every descendant's paint (and gives it a containing block) so this can never happen structurally. - The list groups into Knowledge/Files/Drawing sections (registry `group`, now shared by tray tools and tool-less nouns via the new AtlasNounGroup type) instead of repeating the group word on every row's own meta line. - Row titles read as the noun ("Card", "Pencil") via a new nounName field, separate from the command-verb label ("Add a card") that still drives the palette/tray tooltips. - The bare On/Off status text next to each switch is hidden (it was always aria-hidden in Primer's own markup, so nothing is lost for screen readers). - The Annotate tray group's own collapsed trigger now derives its face icon from the first enabled tool in the group instead of a hardcoded paintbrush, so disabling that tool never leaves a stale glyph. atlasNounRegistry.ts split along its own real seam (board-object content registry vs. the tray-tool registry) to stay under the 500-line file limit, re-exported in full so no other importer moves. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012im1JxQQV2ahnXzZDdVmZq --- frontend/e2e/settings-extensions.spec.ts | 120 +++++++++- frontend/src/atlas/AtlasCreationTray.tsx | 10 +- frontend/src/atlas/atlasBoardObjectContent.ts | 196 +++++++++++++++++ .../src/atlas/atlasNounDeclarationFields.json | 9 +- .../atlas/atlasNounDeclarationFields.test.ts | 1 + frontend/src/atlas/atlasNounRegistry.ts | 208 +++--------------- frontend/src/atlas/tools/areaTool.ts | 3 +- frontend/src/atlas/tools/cardTool.ts | 1 + frontend/src/atlas/tools/diagramNoun.ts | 3 + frontend/src/atlas/tools/eraserTool.ts | 3 +- frontend/src/atlas/tools/imageTool.ts | 1 + frontend/src/atlas/tools/laserTool.ts | 1 + frontend/src/atlas/tools/noteTool.ts | 1 + frontend/src/atlas/tools/pencilTool.ts | 1 + frontend/src/atlas/tools/shapeTool.ts | 1 + frontend/src/atlas/tools/sheetNoun.ts | 3 + frontend/src/atlas/tools/tableTool.ts | 1 + frontend/src/views/ExtensionRow.tsx | 34 ++- .../src/views/ExtensionsSection.module.css | 32 +++ frontend/src/views/ExtensionsSection.tsx | 48 +++- frontend/src/views/extensionMeta.test.ts | 20 +- frontend/src/views/extensionMeta.ts | 57 +++-- userdocs/llms-full.txt | 6 +- userdocs/reference/extending-the-canvas.md | 5 +- userdocs/reference/register-a-canvas-tool.md | 1 + 25 files changed, 533 insertions(+), 233 deletions(-) create mode 100644 frontend/src/atlas/atlasBoardObjectContent.ts diff --git a/frontend/e2e/settings-extensions.spec.ts b/frontend/e2e/settings-extensions.spec.ts index d334dcee..b9e1306b 100644 --- a/frontend/e2e/settings-extensions.spec.ts +++ b/frontend/e2e/settings-extensions.spec.ts @@ -5,14 +5,18 @@ import { deleteViaContextMenu, shapeDrawPoints, shapeObjects } from './fixtures/ import { paletteDialog } from './fixtures/palette' // Settings > Extensions (goal 0237 S2, extended by goal 0237 S3's -// rider): a registry-derived list of every registered canvas NOUN -- -// every tray tool plus every tool-less noun (diagram, sheet -- native -// file-drop only, no tray button), each toggleable off. Shared pool: -// the only global state this spec writes (Shape's own disabled flag) -// is restored to its default (enabled) before the file ends, same -// cleanup discipline display-density.spec.ts already establishes for a -// Settings toggle in the shared pool; every board object created here -// is deleted here. +// rider and its own hands-on-review follow-up): a registry-derived +// list of every registered canvas NOUN -- every tray tool plus every +// tool-less noun (diagram, sheet -- native file-drop only, no tray +// button), each toggleable off. Grouped into three sections +// (Knowledge/Files/Drawing, one per registry `group` -- goal 0237 S3's +// review rider); each row's own title is the noun ("Shape"), never the +// command-verb phrase ("Draw a shape") that still surfaces in the tray +// tooltip and command palette. Shared pool: the only global state this +// spec writes (Shape's own disabled flag) is restored to its default +// (enabled) before the file ends, same cleanup discipline +// display-density.spec.ts already establishes for a Settings toggle in +// the shared pool; every board object created here is deleted here. // // Disabling diagram/sheet gates useAtlasNativeFileDrop.ts's own drop // routing (a disabled drop falls through to the plain-card path). The @@ -45,6 +49,9 @@ test('A row expands to show its description, an honest reach line, and the app v await openExtensionsSection(page) const shapeRow = page.locator('[data-testid="extensions-row"][data-extension-id="shape"]') + // The row's own title is the bare noun, never the tray/palette's own + // command-verb phrase ("Draw a shape"). + await expect(shapeRow.getByTestId('extensions-row-title')).toHaveText('Shape') const expanded = shapeRow.getByTestId('extensions-row-expanded') await expect(expanded).toBeHidden() @@ -57,12 +64,109 @@ test('A row expands to show its description, an honest reach line, and the app v await expect(shapeRow.getByTestId('extensions-row-description')).toHaveText('Draws a rectangle, ellipse, or arrow.') await expect(shapeRow.getByTestId('extensions-row-reach')).toHaveText('Reaches nothing outside Mill.') await expect(shapeRow.getByTestId('extensions-row-version')).toHaveText(/^Ships with Mill v/) + // The group chip survives in the expanded view even though the + // collapsed meta line below no longer repeats it. + await expect(expanded.getByText('Drawing', { exact: true })).toBeVisible() // Collapses again on a second click of the same summary. await shapeRow.locator('summary').click() await expect(expanded).toBeHidden() }) +test('The list groups into three sections; every row title is a noun, and the collapsed meta line never repeats the group', async ({ page }) => { + await page.goto('/') + await openExtensionsSection(page) + + // Three sections, registry-derived, in knowledge/files/drawing order. + const knowledge = page.getByTestId('extensions-group-knowledge') + const files = page.getByTestId('extensions-group-file') + const drawing = page.getByTestId('extensions-group-annotate') + await expect(knowledge.getByRole('heading', { name: 'Knowledge' })).toBeVisible() + await expect(files.getByRole('heading', { name: 'Files' })).toBeVisible() + await expect(drawing.getByRole('heading', { name: 'Drawing' })).toBeVisible() + + // card/note/area/table land in Knowledge; image/diagram/sheet in + // Files (image/diagram/sheet are the file-backed family); the + // freehand-marking tools in Drawing. + for (const id of ['card', 'note', 'area', 'table']) { + await expect(knowledge.locator(`[data-testid="extensions-row"][data-extension-id="${id}"]`)).toBeVisible() + } + for (const id of ['image', 'diagram', 'sheet']) { + await expect(files.locator(`[data-testid="extensions-row"][data-extension-id="${id}"]`)).toBeVisible() + } + for (const id of ['pencil', 'eraser', 'laser', 'shape']) { + await expect(drawing.locator(`[data-testid="extensions-row"][data-extension-id="${id}"]`)).toBeVisible() + } + + // Every row's own title is the bare noun, one word. + const expectedTitles: Record = { + card: 'Card', note: 'Note', area: 'Area', table: 'Table', image: 'Image', + pencil: 'Pencil', eraser: 'Eraser', laser: 'Laser', shape: 'Shape', + diagram: 'Diagram', sheet: 'Sheet', + } + for (const [id, title] of Object.entries(expectedTitles)) { + const row = page.locator(`[data-testid="extensions-row"][data-extension-id="${id}"]`) + await expect(row.getByTestId('extensions-row-title')).toHaveText(title) + } + + // The collapsed meta line states source/edit-route facts, never the + // group word its own section heading already carries. + const imageMeta = page.locator('[data-testid="extensions-row"][data-extension-id="image"]').getByTestId('extensions-row-meta') + await expect(imageMeta).toHaveText('Backed by a file · Opens in your default app') + const tableMeta = page.locator('[data-testid="extensions-row"][data-extension-id="table"]').getByTestId('extensions-row-meta') + await expect(tableMeta).toHaveText('Live view of a List · Edits in place') + const shapeMeta = page.locator('[data-testid="extensions-row"][data-extension-id="shape"]').getByTestId('extensions-row-meta') + await expect(shapeMeta).toHaveText('Stored on the board') +}) + +test('The collapsed meta line stays single-line at 1000px viewport width', async ({ page }) => { + await page.setViewportSize({ width: 1000, height: 660 }) + await page.goto('/') + await openExtensionsSection(page) + + // diagram's own meta line is the longest in the list (a file source + // plus its per-object edit-route resolver's own generic phrase) -- + // the stress case for wrapping. + const diagramMeta = page.locator('[data-testid="extensions-row"][data-extension-id="diagram"]').getByTestId('extensions-row-meta') + await expect(diagramMeta).toBeVisible() + const box = await diagramMeta.boundingBox() + if (!box) throw new Error('extensions-row-meta has no bounding box') + // A single line of this small-text token is well under 24px tall; + // two wrapped lines would roughly double it. + expect(box.height).toBeLessThan(24) +}) + +test('The toggle knob stays contained within its own row, even scrolled with a disclosure open (regression: it painted over the sticky search bar)', async ({ page }) => { + await page.setViewportSize({ width: 1000, height: 660 }) + await page.goto('/') + await openExtensionsSection(page) + + // Scrolling to a row well below the fold pins the sticky search bar + // (.filterRow, SettingsView.module.css) to the top of the scroll + // pane -- the exact live condition the bug needed. + const sheetRow = page.locator('[data-testid="extensions-row"][data-extension-id="sheet"]') + await sheetRow.scrollIntoViewIfNeeded() + await sheetRow.locator('summary').click() + await expect(sheetRow.getByTestId('extensions-row-expanded')).toBeVisible() + + const searchBar = page.getByTestId('settings-filter') + await expect(searchBar).toBeVisible() + const box = await searchBar.boundingBox() + if (!box) throw new Error('settings-filter has no bounding box') + const point = { x: box.x + box.width / 2, y: box.y + box.height / 2 } + + const hit = await page.evaluate(({ x, y }) => { + const el = document.elementFromPoint(x, y) + return { + isSearchBar: el?.closest('[data-testid="settings-filter"]') !== null, + isToggleKnob: el?.className?.toString().includes('ToggleKnob') ?? false, + } + }, point) + + expect(hit.isToggleKnob).toBe(false) + expect(hit.isSearchBar).toBe(true) +}) + test('Turn all off empties the tray of every non-built-in tool; turn all on restores them', async ({ page }) => { await page.goto('/') await openExtensionsSection(page) diff --git a/frontend/src/atlas/AtlasCreationTray.tsx b/frontend/src/atlas/AtlasCreationTray.tsx index fe72e58c..aef256ae 100644 --- a/frontend/src/atlas/AtlasCreationTray.tsx +++ b/frontend/src/atlas/AtlasCreationTray.tsx @@ -124,6 +124,14 @@ export function AtlasCreationTray({ armedTool, locked, onToggle, tablePickerOpen ) const primaryTools = PRIMARY_GROUP_ORDER.flatMap((group) => tools.filter((tool) => tool.group === group)) const annotateTools = tools.filter((tool) => tool.group === 'annotate') + // The collapsed group trigger's own face glyph (goal 0237 S3's review + // rider): derived from the first ENABLED annotate tool rather than a + // hardcoded icon, so disabling that tool from Settings > Extensions + // never leaves the trigger showing a glyph for a tool that's no + // longer in the group at all. Falls back to the generic paintbrush + // only when every annotate tool is disabled and the drawer would + // have nothing left to show anyway. + const AnnotateGroupIcon = annotateTools[0]?.icon ?? PaintbrushIcon const annotateGroupAnchorRef = useRef(null) // The Annotate group's own disclosure state (goal 0224). At most ONE // AnchoredOverlay for the whole annotate family is ever mounted at a @@ -354,7 +362,7 @@ export function AtlasCreationTray({ armedTool, locked, onToggle, tablePickerOpen setManualOpen((open) => !open) }} > - + Extensions describes a noun with no tray tool at all, +// while atlasNounRegistry.ts keeps the tray-tool registry itself +// (AtlasToolShape, registerNoun). Every symbol here is still reached +// through '../atlasNounRegistry' by every existing importer -- +// atlasNounRegistry.ts re-exports this module in full, so this split +// is invisible to every consumer outside these two files. + +// AtlasBoardObjectKind -- the persisted BoardObject.Kind values that +// route through the shared 'atlas-object' renderer. Deliberately NOT +// the same set as a tool's own id: pencilTool's own id is 'pencil' but +// its placed instance is Kind 'ink' (its own commit call names it), so +// content resolution below keys off THIS set, read from object.Kind, +// never off a tool id. +export type AtlasBoardObjectKind = 'shape' | 'image' | 'ink' | 'table' | 'diagram' | 'sheet' + +// AtlasNounContent -- a Kind's own placed-instance rendering (goal +// 0215 S3): the content component AtlasBoardObjectNode.tsx mounts, the +// locale key for its wrapper's aria-label, and whether that wrapper +// carries img semantics (false only for table -- its own grid holds +// real interactive descendants, which img's ARIA role forbids). +// mirrorVersion (goal 0232 S1's file-backed preview/open/watch +// contract) bumps once for every live disk-change AtlasBoardObjectNode +// observes on this object's own mirrored file -- required, not +// optional, so a fileBacked Component can react to it via a plain +// useEffect dependency without also having to declare its own +// useAtlasMirrorChanged subscription (AtlasBoardObjectNode is now the +// ONE place that subscribes, per Kind's own fileBacked declaration +// below). A Component whose Kind is fileBacked: false receives it too +// (it just never changes) rather than a second, optional prop shape. +export interface AtlasNounContent { + // object/mirrorVersion stay required (every Kind receives them, + // whether or not it reads them -- the existing "declare honestly even + // when meaningless" convention this file already documents for + // dragBand/resizable/etc). mirrorContent/fetchListProjection/ + // repickMirror (ADR-0046, goal 0244 S1b) are the kernel reads/writes + // a fileBacked or provider-backed Kind's own Component needs, now + // supplied by the host (AtlasBoardObjectNode.tsx) as props instead of + // the Component importing AtlasService directly -- the import the + // extensions/ cruiser rule forbids. Optional, unlike every other + // field on this interface, for one reason: AtlasMirrorImageContent.test.tsx + // (goal 0243's regression pin) constructs a registered Component + // directly with no host at all, and omitting these three must still + // resolve to each one's own honest "not loaded"/no-op state rather + // than a compile error. + Component: ComponentType<{ + object: BoardObject + mirrorVersion: number + mirrorContent?: MirrorReadState + fetchListProjection?: (id: string) => Promise + repickMirror?: (path: string) => Promise + }> + ariaLabelKey: string + role: 'img' | undefined + // source / editRoute (ADR-0046, goal 0244): the two seams this Kind + // declares about its own artifact -- where it lives, and which door + // edits it. Optional (unlike Component/ariaLabelKey/role above) + // because a Kind with no external artifact at all (shape's own + // Payload-only geometry) has no honest ObjectSource member to declare + // yet, and a Kind not yet migrated onto the edit law has no EditRoute. + // Deliberately nested inside this `content` shape rather than a new + // top-level AtlasToolShapeBase field -- AtlasToolShapeBase's own field + // SET is a separately frozen contract (atlasNounDeclarationFields.json's + // exhaustiveness check). + source?: ObjectSource + // editRoute (ADR-0046, goal 0244 S1): a static route or a per-object + // RESOLVER -- see EditRouteDecl's own header for why a single Kind + // (diagram) needs the function form. + editRoute?: EditRouteDecl + // extension (goal 0237 S3 rider): Settings > Extensions row metadata + // for a NOUN WITH NO TRAY TOOL. A tool-bearing noun already carries + // icon/label/description on its own AtlasToolShapeBase descriptor + // (registerNoun, atlasNounRegistry.ts), so this stays undefined + // there; a tool-less noun (diagram, sheet -- file-drop only, no + // AtlasToolShape to declare these on) sets it directly in its own + // registerBoardObjectContent call so the Extensions list can render + // an honest row for it too, mirroring goal 0211's own description- + // field precedent. Presence of this field is exactly how + // toolLessNounExtensions() below finds a tool-less noun worth + // listing. + extension?: ExtensionRowMeta +} + +// ExtensionRowMeta -- the fields ExtensionRow.tsx needs for a tool-less +// noun's own row that AtlasToolShapeBase would otherwise supply. +// disableScopeNote is REQUIRED (never optional): a tool-less noun has +// no tray button to hide, so its own disable toggle gates a narrower, +// noun-specific seam (file-drop routing, and for diagram the embedded- +// editor door) -- the row must always say so rather than silently +// implying the same tray-wide scope a tool row's toggle has. +export interface ExtensionRowMeta { + icon: Icon + label: string + description: string + disableScopeNote: string + capabilities?: readonly string[] + // group (goal 0237 S3's Extensions-list review rider): the same tray + // cluster AtlasToolShapeBase.group declares, REQUIRED for the same + // reason -- Settings > Extensions groups every row into one of three + // sections regardless of whether it has a tray button, and a + // tool-less noun that omitted this would silently vanish from every + // section instead of landing in an honest one. Both of today's + // tool-less nouns (diagram, sheet) are file-drop-only artifacts, the + // same family Image's own 'file' group already names. + group: AtlasNounGroup +} + +// AtlasBoardObjectContent -- AtlasNounContent plus the board-facts +// AtlasBoardObjectNode.tsx also resolves per Kind; kept as the +// registry's own stored shape so a lookup returns everything the +// renderer needs in one call. fileBacked (goal 0232 S1): does this +// Kind's own Payload.mirrorPath name a real external file this content +// previews -- the ONE flag that drives both the live-watch subscription +// above and the object.openInDefaultApp command's own honest +// enablement (useAtlasObjectMenu.ts), so a new file-backed family's +// entire platform-provided contract is this one boolean plus reading +// mirrorVersion, never its own watch/open wiring. ADR-0046 (goal 0244 +// S0): for a Kind that declares `source`, fileBacked is DERIVED from it +// (`source.kind === 'file'`) by registerBoardObjectContent below rather +// than independently settable -- a Kind with no source still declares +// this field directly (shape/ink today), so the field itself stays +// required. +export interface AtlasBoardObjectContent extends AtlasNounContent { + dragBand: boolean + fileBacked: boolean +} + +const boardObjectContentRegistry = new Map() + +// registerBoardObjectContent -- the honest home for a noun with no +// tray tool at all (diagram: file-drop only, goal 0179 S2). Called +// either directly by a tool-less noun's own registration file, or by +// registerNoun (atlasNounRegistry.ts) on behalf of a tool descriptor +// that declares `boardObjectKind`/`content`. Throws on a duplicate +// Kind so two sources can never silently overwrite each other's +// content. +export function registerBoardObjectContent(kind: AtlasBoardObjectKind, content: AtlasBoardObjectContent): void { + if (boardObjectContentRegistry.has(kind)) { + throw new Error(`atlas board-object kind "${kind}" already has a registered content renderer -- check frontend/src/atlas/tools/`) + } + // fileBacked derivation (ADR-0046, goal 0244 S0): once a Kind + // declares `source`, that union is the single source of truth for + // whether it is file-backed -- the caller's own literal fileBacked + // value (still required by the type) is superseded here rather than + // read back, so the two can never silently disagree. + const resolved = content.source ? { ...content, fileBacked: content.source.kind === 'file' } : content + boardObjectContentRegistry.set(kind, resolved) +} + +// boardObjectContentFor -- the ONE lookup AtlasBoardObjectNode.tsx uses +// to resolve a placed object's own content/ariaLabel/role/dragBand, +// replacing its former per-Kind hand branch. Accepts a plain string +// (BoardObject.Kind is untyped on the wire) and returns undefined for +// an unregistered Kind rather than throwing, since a render path must +// stay recoverable even against bad/legacy data. +export function boardObjectContentFor(kind: string): AtlasBoardObjectContent | undefined { + return boardObjectContentRegistry.get(kind as AtlasBoardObjectKind) +} + +// ToolLessNounExtension -- one entry of toolLessNounExtensions() below, +// with `extension` already narrowed to non-optional (the filter that +// builds this array is the one place that check happens, so every +// consumer downstream gets a guaranteed ExtensionRowMeta instead of +// re-checking for undefined itself). +export interface ToolLessNounExtension { + kind: AtlasBoardObjectKind + content: AtlasBoardObjectContent + extension: ExtensionRowMeta +} + +// toolLessNounExtensions -- every registered noun with NO AtlasToolShape +// of its own (diagram, sheet: file-drop only) that has declared +// Extensions-row metadata, so Settings > Extensions (ExtensionsSection.tsx) +// can list it alongside every tray tool. A tool-bearing noun's content +// also lives in this same registry (registerNoun, atlasNounRegistry.ts, +// folds it in) but never sets `extension`, so it's excluded here -- it +// already gets its own row from ATLAS_TOOLS directly. Sorted by kind +// for a stable, deterministic row order independent of +// import.meta.glob's own alphabetical file-discovery order. +export function toolLessNounExtensions(): ToolLessNounExtension[] { + const found: ToolLessNounExtension[] = [] + for (const [kind, content] of boardObjectContentRegistry.entries()) { + if (content.extension) found.push({ kind, content, extension: content.extension }) + } + return found.sort((a, b) => a.kind.localeCompare(b.kind)) +} diff --git a/frontend/src/atlas/atlasNounDeclarationFields.json b/frontend/src/atlas/atlasNounDeclarationFields.json index 7244b593..96d7903f 100644 --- a/frontend/src/atlas/atlasNounDeclarationFields.json +++ b/frontend/src/atlas/atlasNounDeclarationFields.json @@ -12,7 +12,12 @@ { "field": "label", "legalValues": "a string", - "meaning": "the button/command text. By convention every in-tree noun sources this from identityOf(id).commandLabel rather than restating it, but the field itself accepts any string." + "meaning": "the button/command text. By convention every in-tree noun sources this from identityOf(id).commandLabel rather than restating it, but the field itself accepts any string. NEVER read as this noun's row title in Settings > Extensions -- see nounName below." + }, + { + "field": "nounName", + "legalValues": "a string", + "meaning": "the bare noun a user would call this thing (\"Card\", \"Pencil\"), read only by Settings > Extensions' row title. Kept separate from label because label is a command verb phrase (\"Add a card\") and a row title needs the noun, not the verb." }, { "field": "description", @@ -77,7 +82,7 @@ { "field": "content", "legalValues": "an object with Component (a React component accepting { object, mirrorVersion }), ariaLabelKey (a string), and role ('img' or undefined) -- or null", - "meaning": "this noun's own placed-instance content contribution. registerNoun() feeds it into the board-object content registry (atlasNounRegistry.ts's registerBoardObjectContent) whenever boardObjectKind is non-null, killing AtlasBoardObjectNode.tsx's former per-Kind hand branch. A tool-less noun (diagram, sheet) calls registerBoardObjectContent directly instead of declaring this field at all, since it has no AtlasToolShape to satisfy -- it instead sets this same content shape's own optional `extension` member (icon, label, description, disableScopeNote) so Settings > Extensions can still render an honest row for it. mirrorVersion bumps on a live disk change to a fileBacked Kind's own mirrored file (see fileBacked below) -- a non-file-backed Component simply ignores it." + "meaning": "this noun's own placed-instance content contribution. registerNoun() feeds it into the board-object content registry (atlasNounRegistry.ts's registerBoardObjectContent) whenever boardObjectKind is non-null, killing AtlasBoardObjectNode.tsx's former per-Kind hand branch. A tool-less noun (diagram, sheet) calls registerBoardObjectContent directly instead of declaring this field at all, since it has no AtlasToolShape to satisfy -- it instead sets this same content shape's own optional `extension` member (icon, label, description, disableScopeNote, group) so Settings > Extensions can still render an honest, correctly-sectioned row for it. mirrorVersion bumps on a live disk change to a fileBacked Kind's own mirrored file (see fileBacked below) -- a non-file-backed Component simply ignores it." }, { "field": "capabilities", diff --git a/frontend/src/atlas/atlasNounDeclarationFields.test.ts b/frontend/src/atlas/atlasNounDeclarationFields.test.ts index 5e5181af..31612049 100644 --- a/frontend/src/atlas/atlasNounDeclarationFields.test.ts +++ b/frontend/src/atlas/atlasNounDeclarationFields.test.ts @@ -24,6 +24,7 @@ const EXHAUSTIVE_FIELD_KEYS = { id: true, icon: true, label: true, + nounName: true, description: true, shortcutKey: true, tray: true, diff --git a/frontend/src/atlas/atlasNounRegistry.ts b/frontend/src/atlas/atlasNounRegistry.ts index f3fd23e9..5fd462d6 100644 --- a/frontend/src/atlas/atlasNounRegistry.ts +++ b/frontend/src/atlas/atlasNounRegistry.ts @@ -1,12 +1,18 @@ import type { ComponentType } from 'react' import type { Icon } from '@primer/octicons-react' -import type { BoardObject } from '../../bindings/github.com/alicoding/mill/internal/domain/atlas/models' -import type { ListProjection } from '../../bindings/github.com/alicoding/mill/internal/services/atlassvc/models' import { ATLAS_TOOL_IDENTITIES, type AtlasToolIdentity, type AtlasToolInteraction } from '../shared/atlasToolIdentity' import type { AtlasStyleField } from './atlasStyleVocabulary' import type { FrameBox } from './useAtlasDragFiling' -import type { EditRouteDecl, ObjectSource } from './objectSeams' -import type { MirrorReadState } from './useAtlasObjectMirrorRead' +import { registerBoardObjectContent, type AtlasBoardObjectKind, type AtlasNounContent } from './atlasBoardObjectContent' + +// The board-object CONTENT registry (AtlasNounContent, ExtensionRowMeta, +// registerBoardObjectContent, toolLessNounExtensions, ...) lives in its +// own file, atlasBoardObjectContent.ts (architecture.md's 500-line file +// limit split this registry's own real seam along) -- re-exported here +// IN FULL so every existing `from '../atlasNounRegistry'` import keeps +// resolving unchanged; only this file and atlasBoardObjectContent.ts +// know the split happened. +export * from './atlasBoardObjectContent' // The frontend twin of composition/registry.go's RegisterNodeType // (ADR-0006, goal 0180 slice 1): each canvas noun's own fat descriptor @@ -17,6 +23,16 @@ import type { MirrorReadState } from './useAtlasObjectMirrorRead' // array every noun is appended to. export type { AtlasToolInteraction } +// AtlasNounGroup -- which tray cluster a noun belongs to (goal 0224's +// tray-restructure slice), shared by AtlasToolShapeBase.group below AND +// ExtensionRowMeta.group -- one union, so a tool-bearing noun and a +// tool-less noun declare the SAME three values rather than two +// independently-typed fields that could drift apart. Settings > +// Extensions' section grouping (views/ExtensionsSection.tsx) reads +// this field off every row, tool-bearing or not, never a hand-curated +// per-id list. +export type AtlasNounGroup = 'knowledge' | 'file' | 'annotate' + // Session-only cache seeding a newly created object's own style // (colour/size, ...) -- never persisted document data. export type AtlasToolStyleDefaults = Record @@ -32,177 +48,23 @@ export type AtlasToolStyleDefaults = Record // renderer source a `resizable: true` answer must hold true against. export type AtlasBoardNodeType = 'atlas-note' | 'atlas-sticky' | 'atlas-group' | 'atlas-object' | null -// AtlasBoardObjectKind -- the persisted BoardObject.Kind values that -// route through the shared 'atlas-object' renderer. Deliberately NOT -// the same set as a tool's own id: pencilTool's own id is 'pencil' but -// its placed instance is Kind 'ink' (its own commit call names it), so -// content resolution below keys off THIS set, read from object.Kind, -// never off a tool id. -export type AtlasBoardObjectKind = 'shape' | 'image' | 'ink' | 'table' | 'diagram' | 'sheet' - -// AtlasNounContent -- a Kind's own placed-instance rendering (goal -// 0215 S3): the content component AtlasBoardObjectNode.tsx mounts, the -// locale key for its wrapper's aria-label, and whether that wrapper -// carries img semantics (false only for table -- its own grid holds -// real interactive descendants, which img's ARIA role forbids). -// mirrorVersion (goal 0232 S1's file-backed preview/open/watch -// contract) bumps once for every live disk-change AtlasBoardObjectNode -// observes on this object's own mirrored file -- required, not -// optional, so a fileBacked Component can react to it via a plain -// useEffect dependency without also having to declare its own -// useAtlasMirrorChanged subscription (AtlasBoardObjectNode is now the -// ONE place that subscribes, per Kind's own fileBacked declaration -// below). A Component whose Kind is fileBacked: false receives it too -// (it just never changes) rather than a second, optional prop shape. -export interface AtlasNounContent { - // object/mirrorVersion stay required (every Kind receives them, - // whether or not it reads them -- the existing "declare honestly even - // when meaningless" convention this file already documents for - // dragBand/resizable/etc). mirrorContent/fetchListProjection/ - // repickMirror (ADR-0046, goal 0244 S1b) are the kernel reads/writes - // a fileBacked or provider-backed Kind's own Component needs, now - // supplied by the host (AtlasBoardObjectNode.tsx) as props instead of - // the Component importing AtlasService directly -- the import the - // extensions/ cruiser rule forbids. Optional, unlike every other - // field on this interface, for one reason: AtlasMirrorImageContent.test.tsx - // (goal 0243's regression pin) constructs a registered Component - // directly with no host at all, and omitting these three must still - // resolve to each one's own honest "not loaded"/no-op state rather - // than a compile error. - Component: ComponentType<{ - object: BoardObject - mirrorVersion: number - mirrorContent?: MirrorReadState - fetchListProjection?: (id: string) => Promise - repickMirror?: (path: string) => Promise - }> - ariaLabelKey: string - role: 'img' | undefined - // source / editRoute (ADR-0046, goal 0244): the two seams this Kind - // declares about its own artifact -- where it lives, and which door - // edits it. Optional (unlike Component/ariaLabelKey/role above) - // because a Kind with no external artifact at all (shape's own - // Payload-only geometry) has no honest ObjectSource member to declare - // yet, and a Kind not yet migrated onto the edit law has no EditRoute. - // Deliberately nested inside this `content` shape rather than a new - // top-level AtlasToolShapeBase field -- AtlasToolShapeBase's own field - // SET is a separately frozen contract (atlasNounDeclarationFields.json's - // exhaustiveness check). - source?: ObjectSource - // editRoute (ADR-0046, goal 0244 S1): a static route or a per-object - // RESOLVER -- see EditRouteDecl's own header for why a single Kind - // (diagram) needs the function form. - editRoute?: EditRouteDecl - // extension (goal 0237 S3 rider): Settings > Extensions row metadata - // for a NOUN WITH NO TRAY TOOL. A tool-bearing noun already carries - // icon/label/description on its own AtlasToolShapeBase descriptor - // (registerNoun below), so this stays undefined there; a tool-less - // noun (diagram, sheet -- file-drop only, no AtlasToolShape to - // declare these on) sets it directly in its own registerBoardObjectContent - // call so the Extensions list can render an honest row for it too, - // mirroring goal 0211's own description-field precedent. Presence of - // this field is exactly how toolLessNounExtensions() below finds a - // tool-less noun worth listing. - extension?: ExtensionRowMeta -} - -// ExtensionRowMeta -- the fields ExtensionRow.tsx needs for a tool-less -// noun's own row that AtlasToolShapeBase would otherwise supply. -// disableScopeNote is REQUIRED (never optional): a tool-less noun has -// no tray button to hide, so its own disable toggle gates a narrower, -// noun-specific seam (file-drop routing, and for diagram the embedded- -// editor door) -- the row must always say so rather than silently -// implying the same tray-wide scope a tool row's toggle has. -export interface ExtensionRowMeta { - icon: Icon - label: string - description: string - disableScopeNote: string - capabilities?: readonly string[] -} - -// AtlasBoardObjectContent -- AtlasNounContent plus the board-facts -// AtlasBoardObjectNode.tsx also resolves per Kind; kept as the -// registry's own stored shape so a lookup returns everything the -// renderer needs in one call. fileBacked (goal 0232 S1): does this -// Kind's own Payload.mirrorPath name a real external file this content -// previews -- the ONE flag that drives both the live-watch subscription -// above and the object.openInDefaultApp command's own honest -// enablement (useAtlasObjectMenu.ts), so a new file-backed family's -// entire platform-provided contract is this one boolean plus reading -// mirrorVersion, never its own watch/open wiring. ADR-0046 (goal 0244 -// S0): for a Kind that declares `source`, fileBacked is DERIVED from it -// (`source.kind === 'file'`) by registerBoardObjectContent below rather -// than independently settable -- a Kind with no source still declares -// this field directly (shape/ink today), so the field itself stays -// required. -export interface AtlasBoardObjectContent extends AtlasNounContent { - dragBand: boolean - fileBacked: boolean -} - -const boardObjectContentRegistry = new Map() - -// registerBoardObjectContent -- the honest home for a noun with no -// tray tool at all (diagram: file-drop only, goal 0179 S2). Called -// either directly by a tool-less noun's own registration file, or by -// registerNoun below on behalf of a tool descriptor that declares -// `boardObjectKind`/`content`. Throws on a duplicate Kind so two -// sources can never silently overwrite each other's content. -export function registerBoardObjectContent(kind: AtlasBoardObjectKind, content: AtlasBoardObjectContent): void { - if (boardObjectContentRegistry.has(kind)) { - throw new Error(`atlas board-object kind "${kind}" already has a registered content renderer -- check frontend/src/atlas/tools/`) - } - // fileBacked derivation (ADR-0046, goal 0244 S0): once a Kind - // declares `source`, that union is the single source of truth for - // whether it is file-backed -- the caller's own literal fileBacked - // value (still required by the type) is superseded here rather than - // read back, so the two can never silently disagree. - const resolved = content.source ? { ...content, fileBacked: content.source.kind === 'file' } : content - boardObjectContentRegistry.set(kind, resolved) -} - -// boardObjectContentFor -- the ONE lookup AtlasBoardObjectNode.tsx uses -// to resolve a placed object's own content/ariaLabel/role/dragBand, -// replacing its former per-Kind hand branch. Accepts a plain string -// (BoardObject.Kind is untyped on the wire) and returns undefined for -// an unregistered Kind rather than throwing, since a render path must -// stay recoverable even against bad/legacy data. -export function boardObjectContentFor(kind: string): AtlasBoardObjectContent | undefined { - return boardObjectContentRegistry.get(kind as AtlasBoardObjectKind) -} - -// ToolLessNounExtension -- one entry of toolLessNounExtensions() below, -// with `extension` already narrowed to non-optional (the filter that -// builds this array is the one place that check happens, so every -// consumer downstream gets a guaranteed ExtensionRowMeta instead of -// re-checking for undefined itself). -export interface ToolLessNounExtension { - kind: AtlasBoardObjectKind - content: AtlasBoardObjectContent - extension: ExtensionRowMeta -} - -// toolLessNounExtensions -- every registered noun with NO AtlasToolShape -// of its own (diagram, sheet: file-drop only) that has declared -// Extensions-row metadata, so Settings > Extensions (ExtensionsSection.tsx) -// can list it alongside every tray tool. A tool-bearing noun's content -// also lives in this same registry (registerNoun folds it in below) but -// never sets `extension`, so it's excluded here -- it already gets its -// own row from ATLAS_TOOLS directly. Sorted by kind for a stable, -// deterministic row order independent of import.meta.glob's own -// alphabetical file-discovery order. -export function toolLessNounExtensions(): ToolLessNounExtension[] { - const found: ToolLessNounExtension[] = [] - for (const [kind, content] of boardObjectContentRegistry.entries()) { - if (content.extension) found.push({ kind, content, extension: content.extension }) - } - return found.sort((a, b) => a.kind.localeCompare(b.kind)) -} - interface AtlasToolShapeBase { icon: Icon + // label: the command/button text, sourced from identityOf(id).commandLabel + // (a verb phrase -- "Add a note", "Draw a shape") -- read by the + // command palette (shared/commands.ts) and every tray tooltip/aria-label. + // NEVER read by Settings > Extensions' row title (goal 0237 S3's + // review rider) -- that reads nounName below instead, since a single + // field can't honestly serve both "what click does" (a verb) and + // "what this thing is called" (a noun) at once. label: string + // nounName: the bare noun a user would call this thing -- "Card", + // "Note", "Pencil" -- read ONLY by Settings > Extensions' row title + // (views/extensionMeta.ts's toolRowSource). A tool-less noun has no + // separate field for this at all: its own `extension.label` + // (ExtensionRowMeta) already IS the noun, since it has no command + // verb phrase to disambiguate from in the first place. + nounName: string // description (goal 0211's plugin-manager UX slice): a one-sentence, // user-vocabulary summary of what this noun does, read by the // Extensions section's per-row disclosure (views/ExtensionsSection.tsx, @@ -226,7 +88,7 @@ interface AtlasToolShapeBase { // AtlasCreationTray.tsx's own TRAY_GROUP_ORDER renders every cluster // from this field -- reversible by editing one tool's declaration, // never a hand-enumerated JSX reshuffle. - group: 'knowledge' | 'file' | 'annotate' + group: AtlasNounGroup styleDefaults?: AtlasToolStyleDefaults // styleFields (goal 0209): this noun's own declared styleable // properties, drawn from atlasStyleVocabulary.ts's closed diff --git a/frontend/src/atlas/tools/areaTool.ts b/frontend/src/atlas/tools/areaTool.ts index 4f5f5ba9..8dc3aadc 100644 --- a/frontend/src/atlas/tools/areaTool.ts +++ b/frontend/src/atlas/tools/areaTool.ts @@ -12,7 +12,8 @@ export const areaTool = { id: areaIdentity.id, icon: SquareIcon, label: areaIdentity.commandLabel, - description: 'Groups nearby cards and notes into a labeled frame.', + nounName: 'Area', + description: 'Draws an area to group nearby cards and notes into a labeled frame.', shortcutKey: areaIdentity.shortcutKey, tray: 'quick', // Spatial organization of knowledge, not drawing (goal 0224's diff --git a/frontend/src/atlas/tools/cardTool.ts b/frontend/src/atlas/tools/cardTool.ts index 54c96aa3..0ebbc895 100644 --- a/frontend/src/atlas/tools/cardTool.ts +++ b/frontend/src/atlas/tools/cardTool.ts @@ -16,6 +16,7 @@ export const cardTool = { id: cardIdentity.id, icon: FileIcon, label: cardIdentity.commandLabel, + nounName: 'Card', description: 'Adds a typed card of knowledge you can link, tag, and search.', shortcutKey: cardIdentity.shortcutKey, tray: 'quick', diff --git a/frontend/src/atlas/tools/diagramNoun.ts b/frontend/src/atlas/tools/diagramNoun.ts index f9bada5e..a1e6d90a 100644 --- a/frontend/src/atlas/tools/diagramNoun.ts +++ b/frontend/src/atlas/tools/diagramNoun.ts @@ -57,5 +57,8 @@ registerBoardObjectContent('diagram', { label: 'Diagram', description: 'View and edit diagrams — draw.io files open in the real editor.', disableScopeNote: 'Turning this off stops new diagrams from landing on drop and closes the built-in editor. Diagrams already on the board keep working.', + // File-drop-only, the same family Image's own 'file' group already + // names (atlasNounRegistry.ts's AtlasNounGroup). + group: 'file', }, }) diff --git a/frontend/src/atlas/tools/eraserTool.ts b/frontend/src/atlas/tools/eraserTool.ts index ad5e73fb..56f54254 100644 --- a/frontend/src/atlas/tools/eraserTool.ts +++ b/frontend/src/atlas/tools/eraserTool.ts @@ -32,7 +32,8 @@ export const eraserTool = { id: eraserIdentity.id, icon: TrashIcon, label: eraserIdentity.commandLabel, - description: 'Erases whatever you drag over.', + nounName: 'Eraser', + description: 'Erases whatever you drag over on the board.', shortcutKey: eraserIdentity.shortcutKey, tray: 'quick', // The freehand-marking family (goal 0224's disposition table) -- diff --git a/frontend/src/atlas/tools/imageTool.ts b/frontend/src/atlas/tools/imageTool.ts index 3c1b3ddc..4c793415 100644 --- a/frontend/src/atlas/tools/imageTool.ts +++ b/frontend/src/atlas/tools/imageTool.ts @@ -32,6 +32,7 @@ export const imageTool = { id: imageIdentity.id, icon: ImageIcon, label: imageIdentity.commandLabel, + nounName: 'Image', description: 'Adds an image from your files or the clipboard.', shortcutKey: imageIdentity.shortcutKey, tray: 'quick', diff --git a/frontend/src/atlas/tools/laserTool.ts b/frontend/src/atlas/tools/laserTool.ts index e8cf6a90..1b23f463 100644 --- a/frontend/src/atlas/tools/laserTool.ts +++ b/frontend/src/atlas/tools/laserTool.ts @@ -25,6 +25,7 @@ export const laserTool = { id: laserIdentity.id, icon: ZapIcon, label: laserIdentity.commandLabel, + nounName: 'Laser', description: 'Points at things with a fading trail. Nothing is saved.', shortcutKey: laserIdentity.shortcutKey, tray: 'quick', diff --git a/frontend/src/atlas/tools/noteTool.ts b/frontend/src/atlas/tools/noteTool.ts index 19b9644a..d0e00e52 100644 --- a/frontend/src/atlas/tools/noteTool.ts +++ b/frontend/src/atlas/tools/noteTool.ts @@ -26,6 +26,7 @@ export const noteTool = { id: noteIdentity.id, icon: NoteIcon, label: noteIdentity.commandLabel, + nounName: 'Note', description: 'Adds a quick sticky note for jotting text on the board.', shortcutKey: noteIdentity.shortcutKey, tray: 'quick', diff --git a/frontend/src/atlas/tools/pencilTool.ts b/frontend/src/atlas/tools/pencilTool.ts index c005fc2a..ba586583 100644 --- a/frontend/src/atlas/tools/pencilTool.ts +++ b/frontend/src/atlas/tools/pencilTool.ts @@ -39,6 +39,7 @@ export const pencilTool = { id: pencilIdentity.id, icon: PencilIcon, label: pencilIdentity.commandLabel, + nounName: 'Pencil', description: 'Draws a freehand ink stroke on the board.', shortcutKey: pencilIdentity.shortcutKey, tray: 'quick', diff --git a/frontend/src/atlas/tools/shapeTool.ts b/frontend/src/atlas/tools/shapeTool.ts index 5f7d6c4a..757d32be 100644 --- a/frontend/src/atlas/tools/shapeTool.ts +++ b/frontend/src/atlas/tools/shapeTool.ts @@ -62,6 +62,7 @@ export const shapeTool = { id: shapeIdentity.id, icon: DiamondIcon, label: shapeIdentity.commandLabel, + nounName: 'Shape', description: 'Draws a rectangle, ellipse, or arrow.', shortcutKey: shapeIdentity.shortcutKey, tray: 'quick', diff --git a/frontend/src/atlas/tools/sheetNoun.ts b/frontend/src/atlas/tools/sheetNoun.ts index a9fb9680..49425d25 100644 --- a/frontend/src/atlas/tools/sheetNoun.ts +++ b/frontend/src/atlas/tools/sheetNoun.ts @@ -52,5 +52,8 @@ registerBoardObjectContent('sheet', { label: 'Sheet', description: 'Preview spreadsheets and CSV files dropped onto the board.', disableScopeNote: 'Turning this off stops new sheets from landing on drop. Sheets already on the board keep working, including opening in your default app.', + // File-drop-only, the same family Image's own 'file' group already + // names (atlasNounRegistry.ts's AtlasNounGroup). + group: 'file', }, }) diff --git a/frontend/src/atlas/tools/tableTool.ts b/frontend/src/atlas/tools/tableTool.ts index 698b4516..3f27d863 100644 --- a/frontend/src/atlas/tools/tableTool.ts +++ b/frontend/src/atlas/tools/tableTool.ts @@ -24,6 +24,7 @@ export const tableTool = { id: tableIdentity.id, icon: TableIcon, label: tableIdentity.commandLabel, + nounName: 'Table', description: 'Adds a live table backed by a Configure List.', shortcutKey: tableIdentity.shortcutKey, tray: 'quick', diff --git a/frontend/src/views/ExtensionRow.tsx b/frontend/src/views/ExtensionRow.tsx index 96cdbcce..55144902 100644 --- a/frontend/src/views/ExtensionRow.tsx +++ b/frontend/src/views/ExtensionRow.tsx @@ -12,11 +12,16 @@ import styles from './ExtensionsSection.module.css' const ATLAS_CONCEPTS_DOCS_PAGE = 'concepts/atlas.md' // ExtensionRow -- one row of Settings > Extensions, collapsed to -// icon/label/meta by default, expanding (native
, see +// icon/noun-title/meta by default, expanding (native
, see // ExtensionsSection.module.css's own header comment) into a registry- -// derived detail panel: description, meta chips, an optional disable- -// scope note, the honest reach line, the app's own build version, and -// the shared Docs link. Every value here is READ off the noun's own +// derived detail panel: description, meta chips (including the group, +// dropped from the collapsed meta line since the section heading above +// already states it once), an optional disable-scope note, the honest +// reach line, the app's own build version, and the shared Docs link. +// The row's own title is the noun (`row.label`, sourced from nounName +// for a tray tool -- goal 0237 S3's review rider), never the command +// verb phrase ("Add a note") that surfaces elsewhere (tray tooltips, +// the command palette). Every value here is READ off the noun's own // registered descriptor, normalized into ExtensionRowSource by // extensionMeta.ts's toolRowSource/toolLessRowSource -- nothing here is // hand-curated per extension, and this component never itself branches @@ -31,7 +36,15 @@ export function ExtensionRow({ row, builtIn, enabled, appVersion, onToggle }: { const { t } = useTranslation('views') const Icon = row.icon const labelId = `extension-row-label-${row.id}` - const meta = [row.group ? groupLabel(row.group) : null, sourceLabel(row.source), editRouteLabel(row.editRoute)] + // The collapsed row's own meta line never repeats the group word + // (goal 0237 S3's review rider) -- the section heading above every + // row already says it once; source/editRoute are the only per-row + // facts left. The expanded view's own chip list is a different + // reading context (scanned one row at a time, the heading scrolled + // out of view by then) and keeps the group chip. + const summaryMeta = [sourceLabel(row.source), editRouteLabel(row.editRoute)] + .filter((m): m is string => m !== null) + const chips = [groupLabel(row.group), sourceLabel(row.source), editRouteLabel(row.editRoute)] .filter((m): m is string => m !== null) return ( @@ -41,17 +54,17 @@ export function ExtensionRow({ row, builtIn, enabled, appVersion, onToggle }: { - {row.label} - {meta.length > 0 && ( - {meta.join(' · ')} + {row.label} + {summaryMeta.length > 0 && ( + {summaryMeta.join(' · ')} )}
{descriptionLabel(row)} - {meta.length > 0 && ( + {chips.length > 0 && ( - {meta.map((chip) => )} + {chips.map((chip) => )} )} {row.disableScopeNote && ( @@ -87,6 +100,7 @@ export function ExtensionRow({ row, builtIn, enabled, appVersion, onToggle }: { checked={enabled} onChange={onToggle} size="small" + className={styles.toggle} data-testid="extensions-row-toggle" /> )} diff --git a/frontend/src/views/ExtensionsSection.module.css b/frontend/src/views/ExtensionsSection.module.css index e5815136..c4aedce7 100644 --- a/frontend/src/views/ExtensionsSection.module.css +++ b/frontend/src/views/ExtensionsSection.module.css @@ -8,12 +8,25 @@ browser's own details-open toggle, since that activation behavior is scoped to any click landing inside regardless of the descendant's own React event handling. */ +/* contain: paint scopes every descendant's own painted output -- + including the ToggleSwitch's absolutely-positioned, transform- + transitioning knob in .rowAction below -- to this row's own border + box, and makes the row the containing block for any descendant that + paints as its own compositor layer. Without it, the knob's own + layer painted OUTSIDE the row entirely once the settings pane was + scrolled with a row's
open (reproduced live: the knob + rendered over the sticky search bar several rows above, hit-tested + via document.elementFromPoint at the search bar's own coordinates). + position: relative is required for contain: paint's containing-block + effect to apply as expected across engines. */ .row { display: flex; align-items: flex-start; justify-content: space-between; gap: var(--base-size-8); width: 100%; + position: relative; + contain: paint; } .details { flex: 1; @@ -41,6 +54,25 @@ flex-shrink: 0; padding-top: 2px; } +/* ToggleSwitch renders its own visible "On"/"Off" text next to the + switch by default (Primer's own buttonLabelOn/buttonLabelOff, + defaulting to those two words) -- but that span always carries + aria-hidden="true" in Primer's own markup, so it never reaches a + screen reader in the first place; the switch's real accessible + label/state comes from aria-labelledby (this row's own title) plus + the native button's aria-pressed. Hiding it here is a pure visual + change -- bare switches, state carried by the switch itself -- with + no accessible-name loss. Targets the aria-hidden attribute directly + rather than one of Primer's own hashed CSS-module class names, so it + keeps working across a @primer/react version bump. */ +.toggle > span[aria-hidden='true'] { + display: none; +} +/* Indent + a subtle left rule (mirrors Primer's own TreeView indent + guide, TreeViewItemLevelLine's border-right convention) keeps the + expanded disclosure reading as part of its own row rather than a + free-floating block, even mid-scroll with several other rows above + and below it. */ .expanded { margin: var(--base-size-8) 0 0 24px; padding-left: var(--base-size-8); diff --git a/frontend/src/views/ExtensionsSection.tsx b/frontend/src/views/ExtensionsSection.tsx index 00f86838..4810f123 100644 --- a/frontend/src/views/ExtensionsSection.tsx +++ b/frontend/src/views/ExtensionsSection.tsx @@ -6,7 +6,8 @@ import { toolLessNounExtensions } from '../atlas/atlasNounRegistry' import { SettingsService } from '../shared/bindings' import { refreshDisabledExtensions, useExtensionEnablementStore } from '../shared/extensionEnablementStore' import { ExtensionRow } from './ExtensionRow' -import { toolLessRowSource, toolRowSource, type ExtensionRowSource } from './extensionMeta' +import { groupSectionLabel, toolLessRowSource, toolRowSource, type ExtensionRowSource } from './extensionMeta' +import type { AtlasNounGroup } from '../atlas/atlasNounRegistry' import styles from '../shared/ListCard.module.css' // Settings > Extensions (goal 0237 S2, extended by goal 0211's plugin- @@ -42,6 +43,14 @@ const EXTENSION_ROWS: ExtensionRowSource[] = [ ] const NON_BUILT_IN_IDS: string[] = EXTENSION_ROWS.filter((r) => r.id !== CARD_TOOL_ID).map((r) => r.id) +// The list's own three sections (goal 0237 S3's review rider -- +// "group the list, stop repeating the group"), in the same +// knowledge-then-file-then-annotate order AtlasCreationTray.tsx's own +// PRIMARY_GROUP_ORDER renders the tray in. Rows within each section +// keep EXTENSION_ROWS' own registry order (a stable filter, never a +// re-sort) -- never a hand-curated per-id array. +const SECTION_ORDER: AtlasNounGroup[] = ['knowledge', 'file', 'annotate'] + export default function ExtensionsSection() { const { t } = useTranslation('views') const disabledIds = useExtensionEnablementStore((s) => s.disabledExtensionIds) @@ -89,17 +98,32 @@ export default function ExtensionsSection() { - {EXTENSION_ROWS.map((row) => ( - - toggle(row.id, enabled)} - /> - - ))} + {SECTION_ORDER.map((group) => { + const rows = EXTENSION_ROWS.filter((row) => row.group === group) + if (rows.length === 0) return null + return ( + + {/* as="h3" is REQUIRED here -- Primer's own GroupHeading + throws ("requires a heading level") without it once the + parent ActionList carries role="list" (this one does, + for the assistive-tech list semantics every other row + in Settings already gets). h3 nests correctly under the + page's own h2 section headings (SettingsView.tsx). */} + {groupSectionLabel(group)} + {rows.map((row) => ( + + toggle(row.id, enabled)} + /> + + ))} + + ) + })} ) diff --git a/frontend/src/views/extensionMeta.test.ts b/frontend/src/views/extensionMeta.test.ts index f57c18e2..4043a78d 100644 --- a/frontend/src/views/extensionMeta.test.ts +++ b/frontend/src/views/extensionMeta.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest' import { ATLAS_TOOLS } from '../atlas/atlasTools' import { toolLessNounExtensions } from '../atlas/atlasNounRegistry' import { - descriptionLabel, editRouteLabel, groupLabel, reachLabel, sourceLabel, versionLabel, + descriptionLabel, editRouteLabel, groupLabel, groupSectionLabel, reachLabel, sourceLabel, versionLabel, toolLessRowSource, toolRowSource, } from './extensionMeta' @@ -14,6 +14,14 @@ describe('groupLabel', () => { }) }) +describe('groupSectionLabel', () => { + it('maps every declared group to its section-heading text, plural where the chip is singular', () => { + expect(groupSectionLabel('knowledge')).toBe('Knowledge') + expect(groupSectionLabel('file')).toBe('Files') + expect(groupSectionLabel('annotate')).toBe('Drawing') + }) +}) + describe('sourceLabel', () => { it('maps every ObjectSource kind', () => { expect(sourceLabel({ kind: 'board-local' })).toBe('Stored on the board') @@ -75,24 +83,25 @@ describe('versionLabel', () => { }) describe('toolRowSource (goal 0237 S3 rider)', () => { - it('normalizes a tray tool into a row, carrying its group and content declarations', () => { + it('normalizes a tray tool into a row, reading nounName (not the command-verb label) for the title', () => { const shape = ATLAS_TOOLS.find((t) => t.id === 'shape')! const row = toolRowSource(shape) expect(row).toEqual({ id: 'shape', icon: shape.icon, - label: shape.label, + label: shape.nounName, description: shape.description, group: shape.group, source: shape.content?.source, editRoute: shape.content?.editRoute, capabilities: shape.capabilities, }) + expect(row.label).toBe('Shape') }) }) describe('toolLessRowSource (goal 0237 S3 rider)', () => { - it('normalizes a tool-less noun into a row, with no group chip and its own disableScopeNote', () => { + it('normalizes a tool-less noun into a row, carrying its own group and disableScopeNote', () => { const diagram = toolLessNounExtensions().find((e) => e.kind === 'diagram')! const row = toolLessRowSource(diagram) expect(row).toEqual({ @@ -100,11 +109,12 @@ describe('toolLessRowSource (goal 0237 S3 rider)', () => { icon: diagram.extension.icon, label: diagram.extension.label, description: diagram.extension.description, + group: diagram.extension.group, source: diagram.content.source, editRoute: diagram.content.editRoute, capabilities: diagram.extension.capabilities, disableScopeNote: diagram.extension.disableScopeNote, }) - expect(row.group).toBeUndefined() + expect(row.group).toBe('file') }) }) diff --git a/frontend/src/views/extensionMeta.ts b/frontend/src/views/extensionMeta.ts index d30b6560..727f5437 100644 --- a/frontend/src/views/extensionMeta.ts +++ b/frontend/src/views/extensionMeta.ts @@ -1,7 +1,7 @@ import type { Icon } from '@primer/octicons-react' import type { EditRouteDecl, ObjectSource } from '../atlas/objectSeams' import type { AtlasToolShape } from '../atlas/atlasTools' -import type { ToolLessNounExtension } from '../atlas/atlasNounRegistry' +import type { AtlasNounGroup, ToolLessNounExtension } from '../atlas/atlasNounRegistry' // Pure enum -> user-vocabulary mapping for the Extensions section // (Settings > Extensions). Kept in its own file, apart from @@ -16,17 +16,19 @@ import type { ToolLessNounExtension } from '../atlas/atlasNounRegistry' // normalized from either a tray tool (AtlasToolShape) or a tool-less // noun (ToolLessNounExtension) by the two builders below -- so the row // component itself never branches on which kind of noun it's showing. -// group/description/capabilities/disableScopeNote stay optional: a -// tool-less noun has no tray cluster to report (there is no `group` -// chip for a noun with no tray at all) and only IT ever sets -// disableScopeNote, since only it needs to say its toggle's scope -// differs from a tray tool's. +// group is REQUIRED (goal 0237 S3's Extensions-list review rider): +// every row, tool-bearing or not, now belongs to one of the three +// Settings > Extensions sections (ExtensionsSection.tsx), so a row +// with no honest group would silently vanish from every one of them. +// description/capabilities/disableScopeNote stay optional: only a +// tool-less noun ever sets disableScopeNote, since only it needs to +// say its toggle's scope differs from a tray tool's. export interface ExtensionRowSource { id: string icon: Icon label: string description?: string - group?: AtlasToolShape['group'] + group: AtlasNounGroup source?: ObjectSource editRoute?: EditRouteDecl capabilities?: readonly string[] @@ -37,11 +39,15 @@ export interface ExtensionRowSource { // already did before goal 0237 S3's rider (ExtensionsSection.tsx used // to read AtlasToolShape fields directly); this is that same read, // pulled out so it composes with toolLessRowSource below into one list. +// `label` reads nounName, not the tool's own command-verb `label` (goal +// 0237 S3's Extensions-list review rider) -- the row title is what a +// user calls this thing ("Card"), never what clicking it does ("Add a +// card"). export function toolRowSource(tool: AtlasToolShape): ExtensionRowSource { return { id: tool.id, icon: tool.icon, - label: tool.label, + label: tool.nounName, description: tool.description, group: tool.group, source: tool.content?.source, @@ -51,16 +57,18 @@ export function toolRowSource(tool: AtlasToolShape): ExtensionRowSource { } // toolLessRowSource -- a tool-less noun's own row (goal 0237 S3 rider): -// icon/label/description/disableScopeNote come from its declared -// `extension` (atlasNounRegistry.ts's ExtensionRowMeta), source/ -// editRoute from the same content declaration a tray tool's row reads, -// and no `group` -- there is no tray cluster to report honestly. +// icon/label/description/disableScopeNote/group come from its declared +// `extension` (atlasNounRegistry.ts's ExtensionRowMeta) -- `label` is +// already the noun itself here (diagram/sheet have no separate command +// verb to disambiguate from) -- source/editRoute come from the same +// content declaration a tray tool's row reads. export function toolLessRowSource({ kind, content, extension }: ToolLessNounExtension): ExtensionRowSource { return { id: kind, icon: extension.icon, label: extension.label, description: extension.description, + group: extension.group, source: content.source, editRoute: content.editRoute, capabilities: extension.capabilities, @@ -68,10 +76,14 @@ export function toolLessRowSource({ kind, content, extension }: ToolLessNounExte } } -// groupLabel -- AtlasToolShape.group's own three clusters, restated as -// short chip text a user would recognize (never the tray's own -// internal cluster name). -export function groupLabel(group: AtlasToolShape['group']): string { +// groupLabel -- AtlasNounGroup's own three clusters, restated as short +// chip text a user would recognize (never the tray's own internal +// cluster name). Used only by the expanded row's own chip list +// (ExtensionRow.tsx) -- the collapsed row's meta line reads +// groupSectionLabel below instead, since the section heading above +// every row already says this once (goal 0237 S3's review rider: +// repeating it a second time on every row read as noise). +export function groupLabel(group: AtlasNounGroup): string { switch (group) { case 'knowledge': return 'Knowledge' case 'file': return 'File' @@ -79,6 +91,19 @@ export function groupLabel(group: AtlasToolShape['group']): string { } } +// groupSectionLabel -- the same three clusters, restated as the +// SECTION heading text (goal 0237 S3's review rider: "Knowledge", +// "Files", "Drawing") -- plural where groupLabel's per-row chip stays +// singular ("File"), since a heading names a collection and a chip +// names one row's own attribute. +export function groupSectionLabel(group: AtlasNounGroup): string { + switch (group) { + case 'knowledge': return 'Knowledge' + case 'file': return 'Files' + case 'annotate': return 'Drawing' + } +} + // sourceLabel -- ObjectSource's own three kinds. null for a tool with // no ObjectSource declared at all (card/note/area: no external // artifact, nothing this chip could honestly claim). diff --git a/userdocs/llms-full.txt b/userdocs/llms-full.txt index a2eacf74..85647107 100644 --- a/userdocs/llms-full.txt +++ b/userdocs/llms-full.txt @@ -1407,7 +1407,8 @@ are never omissions. |---|---|---| | `id` | one of the ids declared in shared/atlasToolIdentity.ts's ATLAS_TOOL_IDENTITIES array | the noun's stable identity. registerNoun() throws at module-eval time on a duplicate; assertRegistryAgreesWithIdentity() fails the build if an identity has no matching descriptor, or a descriptor has no matching identity. | | `icon` | any icon component from @primer/octicons-react, typed as Icon | the glyph rendered on this noun's tray/palette button. | -| `label` | a string | the button/command text. By convention every in-tree noun sources this from identityOf(id).commandLabel rather than restating it, but the field itself accepts any string. | +| `label` | a string | the button/command text. By convention every in-tree noun sources this from identityOf(id).commandLabel rather than restating it, but the field itself accepts any string. NEVER read as this noun's row title in Settings > Extensions -- see nounName below. | +| `nounName` | a string | the bare noun a user would call this thing ("Card", "Pencil"), read only by Settings > Extensions' row title. Kept separate from label because label is a command verb phrase ("Add a card") and a row title needs the noun, not the verb. | | `description` | a string, or omitted entirely | a one-sentence, user-vocabulary summary of what this noun does. Read by Settings > Extensions' per-row disclosure; a noun that omits it falls back to its own label there. | | `shortcutKey` | a single-character string, or null | the bare keypress that arms this tool from the board. null for a tool with no bare-key shortcut. | | `tray` | 'quick' or 'palette' | which tray surface renders this tool's button. | @@ -1420,7 +1421,7 @@ are never omissions. | `boardNodeType` | 'atlas-note', 'atlas-sticky', 'atlas-group', 'atlas-object', or null | which shared React Flow node component renders this noun's placed instance. null for a tool whose gesture never persists a renderable instance (eraser, laser). | | `dragBand` | boolean -- REQUIRED, never optional | only load-bearing when boardNodeType is 'atlas-object': does this noun's own content capture pointer events (a grid, a vendored pan/zoom viewer), so the shared renderer needs to add its own chrome band as the drag surface? A noun whose whole body already drags declares false, not omitted. | | `boardObjectKind` | 'shape', 'image', 'ink', 'table', 'diagram', 'sheet', or null | the persisted BoardObject.Kind this noun's own placed instance carries, or null for a tool that never routes through the shared 'atlas-object' renderer. Not always equal to id -- pencil's own placed instance is Kind 'ink' -- so content resolution below keys off this field, read from object.Kind, never off id. | -| `content` | an object with Component (a React component accepting { object, mirrorVersion }), ariaLabelKey (a string), and role ('img' or undefined) -- or null | this noun's own placed-instance content contribution. registerNoun() feeds it into the board-object content registry (atlasNounRegistry.ts's registerBoardObjectContent) whenever boardObjectKind is non-null, killing AtlasBoardObjectNode.tsx's former per-Kind hand branch. A tool-less noun (diagram, sheet) calls registerBoardObjectContent directly instead of declaring this field at all, since it has no AtlasToolShape to satisfy -- it instead sets this same content shape's own optional `extension` member (icon, label, description, disableScopeNote) so Settings > Extensions can still render an honest row for it. mirrorVersion bumps on a live disk change to a fileBacked Kind's own mirrored file (see fileBacked below) -- a non-file-backed Component simply ignores it. | +| `content` | an object with Component (a React component accepting { object, mirrorVersion }), ariaLabelKey (a string), and role ('img' or undefined) -- or null | this noun's own placed-instance content contribution. registerNoun() feeds it into the board-object content registry (atlasNounRegistry.ts's registerBoardObjectContent) whenever boardObjectKind is non-null, killing AtlasBoardObjectNode.tsx's former per-Kind hand branch. A tool-less noun (diagram, sheet) calls registerBoardObjectContent directly instead of declaring this field at all, since it has no AtlasToolShape to satisfy -- it instead sets this same content shape's own optional `extension` member (icon, label, description, disableScopeNote, group) so Settings > Extensions can still render an honest, correctly-sectioned row for it. mirrorVersion bumps on a live disk change to a fileBacked Kind's own mirrored file (see fileBacked below) -- a non-file-backed Component simply ignores it. | | `capabilities` | a readonly array of strings, or omitted entirely | the external reach this noun's own manifest declares. No current noun sets it. Settings > Extensions' reach line reads this field directly, so a future noun's declared capabilities show up there with no other code change. | | `fileBacked` | boolean -- REQUIRED, never optional | does this noun's own placed instance read Payload.mirrorPath as a real external file (goal 0232's file-backed preview/open/watch contract)? true gets the shared live-watch subscription (its own content Component sees mirrorVersion bump) and the object.openInDefaultApp context-menu command uniformly, with no extra wiring of either. A noun with no boardObjectKind at all still declares it, always false. | | `sticky` | boolean -- REQUIRED, never optional | does this tool stay armed after a completed gesture (pencil/eraser/laser -- repeated strokes/passes are the point), or disarm after one? useAtlasToolGesture.ts reads this to decide whether a gesture's own onEnd may call ctx.disarm/disarmUnlessLocked at all -- a sticky tool gets no-ops for both. A non-drag tool still declares it, always false. | @@ -1604,6 +1605,7 @@ export const cardTool = { id: cardIdentity.id, icon: FileIcon, label: cardIdentity.commandLabel, + nounName: 'Card', description: 'Adds a typed card of knowledge you can link, tag, and search.', shortcutKey: cardIdentity.shortcutKey, tray: 'quick', diff --git a/userdocs/reference/extending-the-canvas.md b/userdocs/reference/extending-the-canvas.md index 7066da64..8d401d17 100644 --- a/userdocs/reference/extending-the-canvas.md +++ b/userdocs/reference/extending-the-canvas.md @@ -72,7 +72,8 @@ are never omissions. |---|---|---| | `id` | one of the ids declared in shared/atlasToolIdentity.ts's ATLAS_TOOL_IDENTITIES array | the noun's stable identity. registerNoun() throws at module-eval time on a duplicate; assertRegistryAgreesWithIdentity() fails the build if an identity has no matching descriptor, or a descriptor has no matching identity. | | `icon` | any icon component from @primer/octicons-react, typed as Icon | the glyph rendered on this noun's tray/palette button. | -| `label` | a string | the button/command text. By convention every in-tree noun sources this from identityOf(id).commandLabel rather than restating it, but the field itself accepts any string. | +| `label` | a string | the button/command text. By convention every in-tree noun sources this from identityOf(id).commandLabel rather than restating it, but the field itself accepts any string. NEVER read as this noun's row title in Settings > Extensions -- see nounName below. | +| `nounName` | a string | the bare noun a user would call this thing ("Card", "Pencil"), read only by Settings > Extensions' row title. Kept separate from label because label is a command verb phrase ("Add a card") and a row title needs the noun, not the verb. | | `description` | a string, or omitted entirely | a one-sentence, user-vocabulary summary of what this noun does. Read by Settings > Extensions' per-row disclosure; a noun that omits it falls back to its own label there. | | `shortcutKey` | a single-character string, or null | the bare keypress that arms this tool from the board. null for a tool with no bare-key shortcut. | | `tray` | 'quick' or 'palette' | which tray surface renders this tool's button. | @@ -85,7 +86,7 @@ are never omissions. | `boardNodeType` | 'atlas-note', 'atlas-sticky', 'atlas-group', 'atlas-object', or null | which shared React Flow node component renders this noun's placed instance. null for a tool whose gesture never persists a renderable instance (eraser, laser). | | `dragBand` | boolean -- REQUIRED, never optional | only load-bearing when boardNodeType is 'atlas-object': does this noun's own content capture pointer events (a grid, a vendored pan/zoom viewer), so the shared renderer needs to add its own chrome band as the drag surface? A noun whose whole body already drags declares false, not omitted. | | `boardObjectKind` | 'shape', 'image', 'ink', 'table', 'diagram', 'sheet', or null | the persisted BoardObject.Kind this noun's own placed instance carries, or null for a tool that never routes through the shared 'atlas-object' renderer. Not always equal to id -- pencil's own placed instance is Kind 'ink' -- so content resolution below keys off this field, read from object.Kind, never off id. | -| `content` | an object with Component (a React component accepting { object, mirrorVersion }), ariaLabelKey (a string), and role ('img' or undefined) -- or null | this noun's own placed-instance content contribution. registerNoun() feeds it into the board-object content registry (atlasNounRegistry.ts's registerBoardObjectContent) whenever boardObjectKind is non-null, killing AtlasBoardObjectNode.tsx's former per-Kind hand branch. A tool-less noun (diagram, sheet) calls registerBoardObjectContent directly instead of declaring this field at all, since it has no AtlasToolShape to satisfy -- it instead sets this same content shape's own optional `extension` member (icon, label, description, disableScopeNote) so Settings > Extensions can still render an honest row for it. mirrorVersion bumps on a live disk change to a fileBacked Kind's own mirrored file (see fileBacked below) -- a non-file-backed Component simply ignores it. | +| `content` | an object with Component (a React component accepting { object, mirrorVersion }), ariaLabelKey (a string), and role ('img' or undefined) -- or null | this noun's own placed-instance content contribution. registerNoun() feeds it into the board-object content registry (atlasNounRegistry.ts's registerBoardObjectContent) whenever boardObjectKind is non-null, killing AtlasBoardObjectNode.tsx's former per-Kind hand branch. A tool-less noun (diagram, sheet) calls registerBoardObjectContent directly instead of declaring this field at all, since it has no AtlasToolShape to satisfy -- it instead sets this same content shape's own optional `extension` member (icon, label, description, disableScopeNote, group) so Settings > Extensions can still render an honest, correctly-sectioned row for it. mirrorVersion bumps on a live disk change to a fileBacked Kind's own mirrored file (see fileBacked below) -- a non-file-backed Component simply ignores it. | | `capabilities` | a readonly array of strings, or omitted entirely | the external reach this noun's own manifest declares. No current noun sets it. Settings > Extensions' reach line reads this field directly, so a future noun's declared capabilities show up there with no other code change. | | `fileBacked` | boolean -- REQUIRED, never optional | does this noun's own placed instance read Payload.mirrorPath as a real external file (goal 0232's file-backed preview/open/watch contract)? true gets the shared live-watch subscription (its own content Component sees mirrorVersion bump) and the object.openInDefaultApp context-menu command uniformly, with no extra wiring of either. A noun with no boardObjectKind at all still declares it, always false. | | `sticky` | boolean -- REQUIRED, never optional | does this tool stay armed after a completed gesture (pencil/eraser/laser -- repeated strokes/passes are the point), or disarm after one? useAtlasToolGesture.ts reads this to decide whether a gesture's own onEnd may call ctx.disarm/disarmUnlessLocked at all -- a sticky tool gets no-ops for both. A non-drag tool still declares it, always false. | diff --git a/userdocs/reference/register-a-canvas-tool.md b/userdocs/reference/register-a-canvas-tool.md index 8f70e600..a0eb9945 100644 --- a/userdocs/reference/register-a-canvas-tool.md +++ b/userdocs/reference/register-a-canvas-tool.md @@ -44,6 +44,7 @@ export const cardTool = { id: cardIdentity.id, icon: FileIcon, label: cardIdentity.commandLabel, + nounName: 'Card', description: 'Adds a typed card of knowledge you can link, tag, and search.', shortcutKey: cardIdentity.shortcutKey, tray: 'quick', From 9034c8e21128e5790d0318236cc2d9ab93a53874 Mon Sep 17 00:00:00 2001 From: Ali Al Dallal Date: Sat, 29 Aug 2026 01:06:41 -0400 Subject: [PATCH 2/2] fix: each extensions section is its own labeled list -- valid ARIA both ways Primer's ActionList.Group is structurally unusable here: with list semantics its
  • wrapper hoists the heading and inner
      into the outer list in the accessibility tree (aria-required-children); without list semantics every Item renders as a - + {/* One ActionList PER SECTION rather than ActionList.Group: + with list semantics Primer's Group renders its own
    • wrapper, hoisting the heading and inner +
        into the outer list in the accessibility + tree (aria-required-children, WCAG gate); WITHOUT list + semantics every Item renders as a