From a2762d3b6a723f1add15e4cfc635b858237ba427 Mon Sep 17 00:00:00 2001 From: Alex Kuleshov Date: Sun, 26 Jul 2026 17:18:24 -0500 Subject: [PATCH] feat(ux): dedupe page links and collapse tree row actions behind an overflow menu Links pane extractResolvedLinks emitted one entry per link occurrence, so a body that referenced the same page twice produced duplicate Outgoing links rows, and duplicate Backlinks rows on the target page, with colliding React keys. Dedupe by resolved target path on the server, and defensively in LinkInfo so the pane stays correct against an older backend. Tree navigation Replace the seven hover-only icon buttons on every tree row with a single overflow menu at all breakpoints. The trigger keeps its slot reserved, so revealing it no longer reflows the row or re-truncates the title, and the destructive action no longer sits next to the label. The previous max-md:hidden guard never won the cascade against group-hover:flex, so tapping a row on mobile exposed all eight controls at once. Mobile-first and accessibility - Sidebar search tab swaps the panel in place instead of also raising the modal search dialog on top of it. - Editor unsaved-changes and conflict prompts render through ModalCard, so they get a focus trap, Escape and dialog semantics; editor hotkeys no longer fire from behind an open prompt. - Form fields and the code editor render at 16px on phones to stop iOS Safari auto-zoom, and editor autofocus is gated behind a fine pointer so the keyboard cannot cover the save bar. - Markdown tables scroll inside their own container instead of dragging the article sideways. - 44px touch targets across tree rows, editor toolbar, links pane, history actions and modal dismiss. - Asset manager rows wrap instead of overflowing the dialog; rename moves to its own line and gains a cancel action. - sidebarVisible follows breakpoint changes instead of going stale on rotation. - Chat sends on Enter, keeps Shift+Enter for a newline, and exposes the transcript as a live region. - User deletion uses the in-app confirm dialog instead of window.confirm. Co-Authored-By: Claude Opus 5 (1M context) --- frontend/playwright/ui-smoke.spec.ts | 2 +- frontend/src/components/ModalCard.tsx | 13 +- frontend/src/components/TreeNodeItem.tsx | 334 ++++++------------ .../features/assets/AssetManagerDialog.tsx | 67 ++-- .../src/features/chat/SpaceChatPage.test.tsx | 20 ++ frontend/src/features/chat/SpaceChatPage.tsx | 17 +- .../features/editor/MarkdownCodeEditor.tsx | 12 +- .../src/features/editor/MarkdownToolbar.tsx | 2 +- .../src/features/editor/PageEditor.test.tsx | 2 +- frontend/src/features/editor/PageEditor.tsx | 144 +++++--- frontend/src/features/layout/AppLayout.tsx | 5 +- frontend/src/features/links/LinkInfo.test.tsx | 81 +++++ frontend/src/features/links/LinkInfo.tsx | 54 ++- .../src/features/preview/MarkdownPreview.tsx | 9 + frontend/src/features/sidebar/Sidebar.tsx | 34 +- .../sidebar/SidebarTreeInteraction.test.tsx | 70 +++- frontend/src/features/toolbar/Toolbar.tsx | 22 +- .../users/UserManagementPage.test.tsx | 3 +- .../src/features/users/UserManagementPage.tsx | 45 ++- frontend/src/features/wiki/WikiShell.tsx | 16 +- frontend/src/index.css | 127 ++++++- frontend/src/lib/useMediaQuery.ts | 39 ++ .../service/WikiApplicationService.java | 13 +- .../service/WikiApplicationServiceTest.java | 31 ++ 24 files changed, 776 insertions(+), 386 deletions(-) create mode 100644 frontend/src/lib/useMediaQuery.ts diff --git a/frontend/playwright/ui-smoke.spec.ts b/frontend/playwright/ui-smoke.spec.ts index ffb2657..ba27050 100644 --- a/frontend/playwright/ui-smoke.spec.ts +++ b/frontend/playwright/ui-smoke.spec.ts @@ -51,7 +51,7 @@ test('renders main wiki shell instead of a white screen', async ({ page }) => { await expect(page.getByRole('heading', { name: 'GolemCore Brain' })).toBeVisible() await expect(page.getByRole('banner').getByRole('button', { name: 'Search' })).toBeVisible() await expect(page.getByTestId('sidebar')).toBeVisible() - await expect(page.getByRole('button', { name: 'Tree' })).toBeVisible() + await expect(page.getByRole('tab', { name: 'Tree' })).toBeVisible() const bodyText = await page.locator('body').innerText() expect(bodyText.trim().length).toBeGreaterThan(20) diff --git a/frontend/src/components/ModalCard.tsx b/frontend/src/components/ModalCard.tsx index 6d6b831..a86fe39 100644 --- a/frontend/src/components/ModalCard.tsx +++ b/frontend/src/components/ModalCard.tsx @@ -17,6 +17,7 @@ */ import * as Dialog from '@radix-ui/react-dialog' +import clsx from 'clsx' import { X } from 'lucide-react' import type { PropsWithChildren, ReactNode } from 'react' @@ -26,6 +27,8 @@ interface ModalCardProps extends PropsWithChildren { description?: string onOpenChange: (open: boolean) => void footer?: ReactNode + /** Widens the card for side-by-side content such as a conflict diff. */ + wide?: boolean } export function ModalCard({ @@ -35,12 +38,18 @@ export function ModalCard({ onOpenChange, children, footer, + wide = false, }: ModalCardProps) { return ( - +
@@ -52,7 +61,7 @@ export function ModalCard({ ) : null}
- +
diff --git a/frontend/src/components/TreeNodeItem.tsx b/frontend/src/components/TreeNodeItem.tsx index cd3da29..7fa7194 100644 --- a/frontend/src/components/TreeNodeItem.tsx +++ b/frontend/src/components/TreeNodeItem.tsx @@ -16,9 +16,9 @@ * Contact: alex@kuleshov.tech */ -import { ChevronDown, ChevronRight, Copy, FileText, Folder, FolderOpen, List, MoreVertical, Move, Pencil, Plus, Repeat2, Trash2 } from 'lucide-react' +import * as DropdownMenu from '@radix-ui/react-dropdown-menu' +import { ChevronDown, ChevronRight, Copy, FileText, Folder, FolderOpen, FolderPlus, List, MoreVertical, Move, Pencil, Plus, Repeat2, Trash2 } from 'lucide-react' import clsx from 'clsx' -import { useState } from 'react' import type { MouseEvent } from 'react' import type { WikiNodeKind, WikiTreeNode } from '../types' @@ -58,7 +58,6 @@ export function TreeNodeItem({ onSort, onConvert, }: TreeNodeItemProps) { - const [mobileActionsOpen, setMobileActionsOpen] = useState(false) const isActive = node.path === activePath const isOpen = node.kind !== 'PAGE' && openPaths.includes(node.path) const hasChildren = node.children.length > 0 @@ -68,6 +67,8 @@ export function TreeNodeItem({ : node.kind === 'SECTION' && !hasChildren ? 'PAGE' : null + const canCreateHere = canCreate && node.kind !== 'PAGE' + const hasActions = canEdit || canCreateHere const handleNavigate = (event: MouseEvent) => { event.stopPropagation() @@ -82,7 +83,7 @@ export function TreeNodeItem({
  • {node.kind === 'PAGE' ? ( - + ) : ( )} - {canEdit || (canCreate && node.kind !== 'PAGE') ? ( - <> -
    - {canEdit ? ( - <> - - - - {node.kind !== 'PAGE' ? ( - - ) : null} - {convertTargetKind ? ( - - ) : null} - - - ) : null} - {canCreate && node.kind !== 'PAGE' ? ( - <> - - - - ) : null} -
    -
    + {hasActions ? ( + + - {mobileActionsOpen ? ( -
    - {canEdit ? ( - <> - - - - {node.kind !== 'PAGE' ? ( - - ) : null} - {convertTargetKind ? ( - - ) : null} - - - ) : null} - {canCreate && node.kind !== 'PAGE' ? ( - <> - - - - ) : null} -
    - ) : null} -
    - +
    {node.kind !== 'PAGE' && hasChildren && isOpen ? ( diff --git a/frontend/src/features/assets/AssetManagerDialog.tsx b/frontend/src/features/assets/AssetManagerDialog.tsx index cb5953f..5d09cf1 100644 --- a/frontend/src/features/assets/AssetManagerDialog.tsx +++ b/frontend/src/features/assets/AssetManagerDialog.tsx @@ -270,9 +270,14 @@ export function AssetManagerDialog({ No assets uploaded yet. ) : null} + {/* + Rows wrap rather than overflow: on a phone the dialog is far narrower than the intrinsic + width of the name column plus the action cluster. Rename gets its own full width line so + the field is never squeezed to a fixed 160px inside a ~260px dialog. + */} {assets.map((asset) => ( -
    -
    +
    +
    {asset.name}
    {asset.contentType} · {Math.round(asset.size / 1024)} KB
    @@ -287,32 +292,24 @@ export function AssetManagerDialog({
    -
    +
    {renderInsertButtons(asset)} - {renameTarget === asset.name ? ( -
    - setRenameValue(event.target.value)} - /> - -
    - ) : ( - - )} +
    + {renameTarget === asset.name ? ( +
    + setRenameValue(event.target.value)} + aria-label={`New name for ${asset.name}`} + /> + + +
    + ) : null}
    ))}
    diff --git a/frontend/src/features/chat/SpaceChatPage.test.tsx b/frontend/src/features/chat/SpaceChatPage.test.tsx index 5671359..ef17786 100644 --- a/frontend/src/features/chat/SpaceChatPage.test.tsx +++ b/frontend/src/features/chat/SpaceChatPage.test.tsx @@ -134,6 +134,26 @@ describe('SpaceChatPage', () => { }) }) + it('sends on Enter and keeps Shift+Enter for a new line', async () => { + render( + + + , + ) + + await screen.findByLabelText('Chat model') + const question = screen.getByLabelText('Question') + + fireEvent.change(question, { target: { value: 'Line one' } }) + fireEvent.keyDown(question, { key: 'Enter', shiftKey: true }) + expect(chatWithSpaceMock).not.toHaveBeenCalled() + + fireEvent.keyDown(question, { key: 'Enter' }) + await waitFor(() => { + expect(chatWithSpaceMock).toHaveBeenCalledWith('Line one', [], 'chat-model', null, 1) + }) + }) + it('lets users choose among enabled chat models', async () => { render( diff --git a/frontend/src/features/chat/SpaceChatPage.tsx b/frontend/src/features/chat/SpaceChatPage.tsx index db18c36..fb8c95d 100644 --- a/frontend/src/features/chat/SpaceChatPage.tsx +++ b/frontend/src/features/chat/SpaceChatPage.tsx @@ -105,7 +105,12 @@ export function SpaceChatPage() {
    -
    +
    {messages.length === 0 ? (
    Ask a question about this space
    @@ -167,9 +172,19 @@ export function SpaceChatPage() { className="field-input min-h-24 resize-y" value={draft} placeholder="Ask a question about this space…" + aria-describedby="chat-send-hint" onChange={(event) => setDraft(event.target.value)} + // Enter sends, Shift+Enter adds a newline — the convention every chat UI uses. + onKeyDown={(event) => { + if (event.key !== 'Enter' || event.shiftKey || !canSend) { + return + } + event.preventDefault() + event.currentTarget.form?.requestSubmit() + }} /> +

    Press Enter to send, Shift and Enter for a new line.

    -
    +
    diff --git a/frontend/src/features/editor/PageEditor.test.tsx b/frontend/src/features/editor/PageEditor.test.tsx index 005e568..11fe477 100644 --- a/frontend/src/features/editor/PageEditor.test.tsx +++ b/frontend/src/features/editor/PageEditor.test.tsx @@ -157,7 +157,7 @@ describe('PageEditor', () => { ) fireEvent.click(screen.getByRole('button', { name: 'Edit metadata' })) - expect(screen.getByText('Metadata')).toBeInTheDocument() + expect(screen.getByRole('heading', { name: 'Metadata' })).toBeInTheDocument() expect(screen.getByText('Unsaved changes')).toBeInTheDocument() expect(screen.getByRole('button', { name: 'Save changes' })).toBeEnabled() diff --git a/frontend/src/features/editor/PageEditor.tsx b/frontend/src/features/editor/PageEditor.tsx index 322c8bc..48726bc 100644 --- a/frontend/src/features/editor/PageEditor.tsx +++ b/frontend/src/features/editor/PageEditor.tsx @@ -25,6 +25,7 @@ import { toast } from 'sonner' import { uploadAsset } from '../../lib/api' import { editorPathToRoute, normalizeWikiPath, pathToRoute } from '../../lib/paths' import { InsertWikiLinkDialog } from '../../components/InsertWikiLinkDialog' +import { ModalCard } from '../../components/ModalCard' import { AssetManagerDialog } from '../assets/AssetManagerDialog' import { buildDefaultMarkdownForAsset } from '../assets/assetMarkdown' import { MarkdownPreview } from '../preview/MarkdownPreview' @@ -177,6 +178,11 @@ export function PageEditor() { useEffect(() => { const handleKeyDown = (event: KeyboardEvent) => { + // A blocking prompt owns the keyboard; re-firing save/close from behind it would act on + // state the author is still being asked about. + if (showUnsavedDialog || showConflictDialog) { + return + } const modifier = event.metaKey || event.ctrlKey if (modifier && event.key.toLowerCase() === 's') { event.preventDefault() @@ -354,20 +360,30 @@ export function PageEditor() { /{page.path}
    + {/* + The fixed mobile bar has roughly 100px per column, which is not enough for the full + labels. Each button keeps its full accessible name and shows a short label on phones + so nothing wraps onto a second line. + */}
    @@ -492,60 +510,78 @@ export function PageEditor() { onSelect={handleInsertWikiLink} /> - {showUnsavedDialog ? ( -
    -
    -

    Unsaved changes

    -

    - You have unsaved changes. Do you want to discard them and continue? -

    -
    - - -
    -
    -
    - ) : null} - - {showConflictDialog && conflict ? ( -
    -
    -

    Page changed in another session

    -

    - The latest saved version was updated at {formatTimestamp(conflict.updatedAt)}. Reload it to discard your draft, or rebase your draft onto the latest revision for a manual merge. -

    -
    -
    -
    Your draft
    -
    /{(page.parentPath ? `${page.parentPath}/` : '') + slug}
    -
    {title}
    - 
    {content || '(empty)'}
    -
    -
    -
    Latest saved version
    -
    /{conflict.path}
    -
    {conflict.title}
    - 
    {conflict.content || '(empty)'}
    -
    + {/* + Both prompts guard destructive outcomes, so they run through ModalCard (Radix Dialog) to + get a focus trap, Escape-to-dismiss and dialog semantics instead of a bare overlay div. + */} + { + if (!open) { + handleCancelNavigation() + } + }} + footer={( + <> + + + + )} + > +

    + Discarding leaves the last saved version of this page untouched. +

    +     + + { + if (!open) { + setShowConflictDialog(false) + } + }} + footer={( + <> + + + + + )} + > + {conflict ? ( +
    +
    +
    Your draft
    +
    /{(page.parentPath ? `${page.parentPath}/` : '') + slug}
    +
    {title}
    + 
    {content || '(empty)'}
    -
    - - - +
    +
    Latest saved version
    +
    /{conflict.path}
    +
    {conflict.title}
    + 
    {conflict.content || '(empty)'}
    -
    - ) : null} + ) : null} +     ) } diff --git a/frontend/src/features/layout/AppLayout.tsx b/frontend/src/features/layout/AppLayout.tsx index bc39266..d867510 100644 --- a/frontend/src/features/layout/AppLayout.tsx +++ b/frontend/src/features/layout/AppLayout.tsx @@ -47,7 +47,6 @@ interface AppLayoutProps { onConvert: (path: string, targetKind: Exclude) => void onExpandAll: () => void onCollapseAll: () => void - onOpenSearch: () => void currentUsername?: string | null canManageUsers: boolean canAccessAccount: boolean @@ -80,7 +79,6 @@ export function AppLayout({ onConvert, onExpandAll, onCollapseAll, - onOpenSearch, currentUsername, canManageUsers, canAccessAccount, @@ -111,6 +109,8 @@ export function AppLayout({ className="app-layout__sidebar-toggle-button" onClick={onToggleSidebar} aria-label="Toggle Sidebar" + aria-expanded={sidebarVisible} + aria-controls="sidebar-container" > @@ -173,7 +173,6 @@ export function AppLayout({ onConvert={onConvert} onExpandAll={onExpandAll} onCollapseAll={onCollapseAll} - onOpenSearch={onOpenSearch} imageVersion={displayImageVersion} />
    diff --git a/frontend/src/features/links/LinkInfo.test.tsx b/frontend/src/features/links/LinkInfo.test.tsx index 1408ae7..478b1f0 100644 --- a/frontend/src/features/links/LinkInfo.test.tsx +++ b/frontend/src/features/links/LinkInfo.test.tsx @@ -190,4 +190,85 @@ describe('LinkInfo', () => { expect(screen.getByRole('link', { name: 'Runbook' })).toHaveAttribute('href', '/brain/guides/runbook') expect(screen.getByRole('link', { name: 'Checklist' })).toHaveAttribute('href', '/brain/shared/checklist') }) + + it('lists a repeatedly referenced page only once', () => { + useViewerStore.setState({ + linkStatus: { + backlinks: [ + { + fromPageId: 'guides/runbook', + fromPath: 'guides/runbook', + fromTitle: 'Runbook', + toPageId: 'guides/setup', + toPath: 'guides/setup', + toTitle: 'Setup', + broken: false, + }, + { + fromPageId: 'guides/runbook', + fromPath: 'guides/runbook', + fromTitle: 'Runbook', + toPageId: 'guides/setup', + toPath: 'guides/setup', + toTitle: 'Setup', + broken: false, + }, + ], + brokenIncoming: [], + outgoings: [ + { + fromPageId: 'guides/setup', + fromPath: 'guides/setup', + fromTitle: 'Setup', + toPageId: 'shared/checklist', + toPath: 'shared/checklist', + toTitle: 'Checklist', + broken: false, + }, + { + fromPageId: 'guides/setup', + fromPath: 'guides/setup', + fromTitle: 'Setup', + toPageId: 'shared/checklist', + toPath: 'shared/checklist', + toTitle: 'Checklist', + broken: false, + }, + ], + brokenOutgoings: [ + { + fromPageId: 'guides/setup', + fromPath: 'guides/setup', + fromTitle: 'Setup', + toPageId: null, + toPath: 'shared/missing', + toTitle: 'Missing', + broken: true, + }, + { + fromPageId: 'guides/setup', + fromPath: 'guides/setup', + fromTitle: 'Setup', + toPageId: null, + toPath: 'shared/missing', + toTitle: 'Missing', + broken: true, + }, + ], + }, + history: [], + }) + + render( + + + , + ) + + expect(screen.getAllByRole('link', { name: 'Runbook' })).toHaveLength(1) + expect(screen.getAllByRole('link', { name: 'Checklist' })).toHaveLength(1) + expect(screen.getAllByText('Missing')).toHaveLength(1) + expect(screen.getByText('Backlinks').parentElement).toHaveTextContent('1') + expect(screen.getByText('Outgoing links').parentElement).toHaveTextContent('2') + }) }) diff --git a/frontend/src/features/links/LinkInfo.tsx b/frontend/src/features/links/LinkInfo.tsx index 1499bc1..a1e5d2c 100644 --- a/frontend/src/features/links/LinkInfo.tsx +++ b/frontend/src/features/links/LinkInfo.tsx @@ -27,7 +27,7 @@ import { pathToRoute } from '../../lib/paths' import { useTreeStore } from '../../stores/tree' import { useUiStore } from '../../stores/ui' import { useViewerStore } from '../../stores/viewer' -import type { WikiPageHistoryVersion } from '../../types' +import type { WikiLinkStatusItem, WikiPageHistoryVersion } from '../../types' import { MarkdownPreview } from '../preview/MarkdownPreview' import { buildLineDiff } from './historyDiff' @@ -38,6 +38,25 @@ function formatTimestamp(timestamp?: string) { return new Date(timestamp).toLocaleString() } +/** + * Collapses link rows that point at the same page. A body may reference the same target several + * times, and every reference resolves to an identical row; listing it once keeps the pane readable + * and keeps React keys unique. + */ +function dedupeLinks(items: WikiLinkStatusItem[], keyOf: (item: WikiLinkStatusItem) => string) { + const seen = new Set() + const unique: Array<{ key: string; item: WikiLinkStatusItem }> = [] + for (const item of items) { + const key = keyOf(item) + if (seen.has(key)) { + continue + } + seen.add(key) + unique.push({ key, item }) + } + return unique +} + export function LinkInfo() { const page = useViewerStore((state) => state.page) const linkStatus = useViewerStore((state) => state.linkStatus) @@ -120,6 +139,19 @@ export function LinkInfo() { removed: diffLines.filter((line) => line.type === 'removed').length, }), [diffLines]) + const backlinks = useMemo( + () => dedupeLinks(linkStatus?.backlinks ?? [], (item) => item.fromPath || item.fromPageId || item.fromTitle || ''), + [linkStatus], + ) + const outgoings = useMemo( + () => dedupeLinks(linkStatus?.outgoings ?? [], (item) => item.toPath || item.toPageId || item.toTitle || ''), + [linkStatus], + ) + const brokenOutgoings = useMemo( + () => dedupeLinks(linkStatus?.brokenOutgoings ?? [], (item) => item.toPath || item.toTitle || ''), + [linkStatus], + ) + if (!linkStatus) { return null } @@ -130,15 +162,15 @@ export function LinkInfo() {
    Backlinks - {linkStatus.backlinks.length} + {backlinks.length}
    - {linkStatus.backlinks.length === 0 ? ( + {backlinks.length === 0 ? (

    No pages reference this page.

    ) : (
      - {linkStatus.backlinks.map((item) => ( -
    • + {backlinks.map(({ key, item }) => ( +
    • {item.fromPath ? ( {item.fromTitle ?? item.fromPath} ) : ( @@ -153,15 +185,15 @@ export function LinkInfo() {
      Outgoing links - {linkStatus.outgoings.length + linkStatus.brokenOutgoings.length} + {outgoings.length + brokenOutgoings.length}
      - {linkStatus.outgoings.length === 0 && linkStatus.brokenOutgoings.length === 0 ? ( + {outgoings.length === 0 && brokenOutgoings.length === 0 ? (

      No outgoing links on this page.

      ) : (
        - {linkStatus.outgoings.map((item) => ( -
      • + {outgoings.map(({ key, item }) => ( +
      • {item.toPath ? ( {item.toTitle ?? item.toPath} @@ -170,8 +202,8 @@ export function LinkInfo() { )}
        • ))} - {linkStatus.brokenOutgoings.map((item) => ( -
      • + {brokenOutgoings.map(({ key, item }) => ( +
      • {item.toTitle}
        • diff --git a/frontend/src/features/preview/MarkdownPreview.tsx b/frontend/src/features/preview/MarkdownPreview.tsx index b90a9f4..1242b00 100644 --- a/frontend/src/features/preview/MarkdownPreview.tsx +++ b/frontend/src/features/preview/MarkdownPreview.tsx @@ -88,6 +88,15 @@ export function MarkdownPreview({ content, path, darkMode, assetVersion }: Markd video: (props: React.VideoHTMLAttributes) => (