From a1d14da8e6efad3e72862f4f659f2a3049e5457f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 06:24:02 +0000 Subject: [PATCH 1/2] feat(ui): give the library the whole screen, and the panel a place to sit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The library was built in M1 for one screen size and never revisited while M3-M6 accreted panels onto it. Two consequences, both reported by the user. The grid sat in `mx-auto max-w-7xl`, so above 1280px the library was a centred column with dead margins either side, and its `grid-cols-2 → sm:3 → lg:4 → xl:6` ramp stopped at six — past that, tiles stretched instead of multiplying. It is full-bleed now, on `auto-fill` tracks that add columns as room appears: 2 on a 360px phone, 4 at 1280, 6 at 1920, 8 at 2560. `auto-fill` rather than `auto-fit` so a half-empty last row stays left-aligned instead of stretching two tiles across the screen, and a `max-w-[25rem]` on the card is what actually caps a tile, since auto-fill tracks reach roughly twice their minimum before a further column fits. Opening an asset was a 896px centred modal with a fixed 320px sidebar and five hard-coded viewport caps — 45vh video, 60vh image, 38vh transcript twice, 90vh dialog — small on a large screen, cramped on a small one, and covering the library either way. It now docks against the right edge from 1024px up, drag-resizable from a `role="separator"` handle (arrow keys and Home/End too, double-click to reset) with the width remembered, and the grid reflows beside it. Narrower than that, it is a full-screen sheet. The media pins to the top and the rest lives in tabs that fill the remaining height, so the transcript gets the panel instead of 38vh; past `@4xl` the media moves beside the tabs instead. Container queries, not viewport breakpoints. Once the panel is open and drag-resizable the viewport no longer describes how much room the grid beside it has, so `xl:` is simply the wrong signal. That is the one new dependency, build-time only. `AssetDetail` baked `fixed inset-0 … bg-black/50` into itself, which is why all three of its callers got modal chrome whether it suited them or not. Positioning moved to `DetailDock`; the content fills whatever box it is handed. `/a/{id}` was the clearest victim — it drew a floating dialog over an empty shell, and is a full-width page now. Its four tests asserted `role="dialog"`, which was asserting the bug, so they assert the heading instead; the stale `usageApi.forAsset` mock beside them, whose field names are not on `UsageTotals` and were hidden by an `as never`, is fixed too. Description and summary auto-grow rather than scrolling inside 80px, and re-measure on a `ResizeObserver` because the panel's width changes under them without the text ever changing. `Tabs` is the first UI primitive here. It keeps hidden panels mounted: `TranscriptPanel` fetches on mount, so rendering only the active tab would refetch the transcript and lose its scroll position every time you checked the description and came back. Verified in a real browser at 390/768/1280/1920/2560, light and dark, with the API stubbed at the network layer — column counts and tile widths come out as designed and nothing throws. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YTm2BUsWF7ufB3fYs9kNgB --- docs/plan-of-attack.md | 28 +- frontend/package-lock.json | 11 + frontend/package.json | 1 + frontend/src/components/AssetCard.tsx | 6 +- frontend/src/components/AssetDetail.tsx | 545 ++++++++++-------- frontend/src/components/DetailDock.test.tsx | 163 ++++++ frontend/src/components/DetailDock.tsx | 103 ++++ frontend/src/components/DocumentTextPanel.tsx | 2 +- frontend/src/components/Tabs.test.tsx | 89 +++ frontend/src/components/Tabs.tsx | 119 ++++ frontend/src/components/TranscriptPanel.tsx | 2 +- frontend/src/test-setup.ts | 32 + frontend/src/utils/useAutoGrow.ts | 45 ++ frontend/src/utils/useMediaQuery.ts | 28 + frontend/src/utils/useResizablePanel.ts | 125 ++++ frontend/src/views/AssetView.test.tsx | 20 +- frontend/src/views/AssetView.tsx | 15 +- frontend/src/views/LibraryView.test.tsx | 17 + frontend/src/views/LibraryView.tsx | 170 +++--- frontend/src/views/SearchView.tsx | 169 +++--- frontend/tailwind.config.js | 7 +- 21 files changed, 1275 insertions(+), 422 deletions(-) create mode 100644 frontend/src/components/DetailDock.test.tsx create mode 100644 frontend/src/components/DetailDock.tsx create mode 100644 frontend/src/components/Tabs.test.tsx create mode 100644 frontend/src/components/Tabs.tsx create mode 100644 frontend/src/utils/useAutoGrow.ts create mode 100644 frontend/src/utils/useMediaQuery.ts create mode 100644 frontend/src/utils/useResizablePanel.ts diff --git a/docs/plan-of-attack.md b/docs/plan-of-attack.md index c7f74a5..c56c6ed 100644 --- a/docs/plan-of-attack.md +++ b/docs/plan-of-attack.md @@ -332,6 +332,9 @@ Non-milestone PRs, so a `git log` that does not match the table above still make database and the docs that go with it; [#9](https://github.com/davior/gam/pull/9) documented `.env` for local dev and made the CSP's Notes origin follow `NOTES_BASE_URL`. +The library and asset-detail layout rework — full-bleed intrinsic grid, the resizable +docked panel, `DetailDock`/`Tabs`, and `/a/{id}` as a page — is also non-milestone: M1 +built that UI for one screen size and M3–M6 accreted panels onto it without revisiting it. ### Outstanding, unscheduled @@ -381,15 +384,22 @@ from a decision, which is the distinction PR bodies do not preserve. - **Search ignores `asset_type` and `limit`.** The backend accepts both (`routers/search.py:51-53`) and `api/search.ts` types them; `SearchView.tsx` passes neither. Results cap at the server default of 30 with no way to page or filter by type. -- **No `/a/{id}` deep link.** `assetsApi.get(id)` exists and is called by nothing, and there - is no route. This is not cosmetic: **GN-4 specifies a Notes→GAM asset reference as a plain - link to `/a/{assetId}`**, so that integration cannot work as designed until the route - exists. The decision to use a link rather than a shortcode was taken partly *because* it - needed no work in Notes — that reasoning assumed this end existed. -- **A 401 mid-session is a dead end.** There is no axios response interceptor; only - `bootstrap()` handles 401, so an expiry during an upload or a search surfaces as an inline - error string and nothing re-authenticates. Relatedly, `signOut()` exists in `stores/auth.ts` - and no component calls it — there is no sign-out control anywhere in the UI. +- **~~No `/a/{id}` deep link.~~** Built in `cdcabbf`, so GN-4's Notes→GAM reference now + resolves. It rendered as a floating dialog over an empty shell until the panel chrome + moved into `DetailDock`; it is a full-width page now. +- **~~A 401 mid-session is a dead end.~~** Closed in `cdcabbf`: `App.tsx` wires + `setUnauthorizedHandler`, and `AppShell` calls `signOut()`. +- **Escape does too much in the asset panel.** `TagInput` (`TagInput.tsx:93`) and the + transcript segment editor (`TranscriptPanel.tsx:259`) both handle Escape without calling + `stopPropagation`, so it reaches `DetailDock`'s window listener as well. Dismissing a tag + suggestion menu, or abandoning a half-typed correction to a transcript line, therefore + closes the whole panel. Predates the panel — the old modal had the same listener — and was + left alone during the layout rework rather than widening that change. +- **The full-screen sheet is not a trapped modal.** Below 1024px the panel covers the screen + and claims `aria-modal`, but nothing traps focus inside it, nothing returns focus to the + card that opened it, and the library behind is not `inert`. Tab walks out into a grid the + user cannot see. Docked, none of this applies, which is why it was not urgent enough to + fold into the layout work. - **The SRS is not in this repository.** M6–M8 are specified against FR numbers (8.1.3, 9.1.4, 10.1.4, 11.1.1/2) that appear in this document and in code comments, in a source no session can read. Either commit it beside these docs or stop citing it; a requirement diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 03e06c2..c97fa0c 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -16,6 +16,7 @@ "zustand": "^5.0.2" }, "devDependencies": { + "@tailwindcss/container-queries": "^0.1.1", "@testing-library/jest-dom": "^6.6.3", "@testing-library/react": "^16.1.0", "@testing-library/user-event": "^14.5.2", @@ -1591,6 +1592,16 @@ "win32" ] }, + "node_modules/@tailwindcss/container-queries": { + "version": "0.1.1", + "resolved": "https://registry.npmjs.org/@tailwindcss/container-queries/-/container-queries-0.1.1.tgz", + "integrity": "sha512-p18dswChx6WnTSaJCSGx6lTmrGzNNvm2FtXmiO6AuA1V4U5REyoqwmT6kgAsIMdjo07QdAfYXHJ4hnMtfHzWgA==", + "dev": true, + "license": "MIT", + "peerDependencies": { + "tailwindcss": ">=3.2.0" + } + }, "node_modules/@testing-library/dom": { "version": "10.4.1", "resolved": "https://registry.npmjs.org/@testing-library/dom/-/dom-10.4.1.tgz", diff --git a/frontend/package.json b/frontend/package.json index 15bd5ed..68bacb0 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -22,6 +22,7 @@ "zustand": "^5.0.2" }, "devDependencies": { + "@tailwindcss/container-queries": "^0.1.1", "@testing-library/jest-dom": "^6.6.3", "@testing-library/react": "^16.1.0", "@testing-library/user-event": "^14.5.2", diff --git a/frontend/src/components/AssetCard.tsx b/frontend/src/components/AssetCard.tsx index de99b22..f1a792a 100644 --- a/frontend/src/components/AssetCard.tsx +++ b/frontend/src/components/AssetCard.tsx @@ -21,7 +21,11 @@ export default function AssetCard({ asset, onActivate, selected = false }: Props // and read unpredictably; `aria-pressed` says the same thing about the one // control that is actually here. aria-pressed={selected} - className={`card group flex flex-col overflow-hidden p-0 text-left ${ + // max-w is what actually caps a tile. The grid's auto-fill tracks stretch to + // roughly twice their minimum before a further column fits, which at the widest + // step would be ~440px; the grid's `justify-items-center` keeps a capped tile + // centred in its track rather than leaving the gutter all on one side. + className={`card group flex w-full max-w-[25rem] flex-col overflow-hidden p-0 text-left ${ selected ? 'ring-2 ring-blue-600 dark:ring-blue-400' : '' }`} > diff --git a/frontend/src/components/AssetDetail.tsx b/frontend/src/components/AssetDetail.tsx index bd068a9..df7b42d 100644 --- a/frontend/src/components/AssetDetail.tsx +++ b/frontend/src/components/AssetDetail.tsx @@ -1,5 +1,15 @@ -import { useCallback, useEffect, useRef, useState } from 'react' -import { Eye, FileText, Link2, ScanText, Trash2, X } from 'lucide-react' +import { useCallback, useEffect, useState } from 'react' +import { + Eye, + FileText, + Info, + Link2, + Mic, + Pencil, + ScanText, + Trash2, + X, +} from 'lucide-react' import type { Asset } from '@/api/assets' import { tagsApi } from '@/api/tags' import { enrichmentApi } from '@/api/enrichment' @@ -8,12 +18,14 @@ import { apiErrorMessage } from '@/api/client' import { useLibraryStore } from '@/stores/library' import { useTagStore } from '@/stores/tags' import { formatBytes, formatDate, formatDimensions, formatDuration } from '@/utils/format' +import { useAutoGrow } from '@/utils/useAutoGrow' import AssetThumb from '@/components/AssetThumb' import DocumentTextPanel from '@/components/DocumentTextPanel' import EmbedButton from '@/components/EmbedButton' import EnrichmentButton from '@/components/EnrichmentButton' import SuggestionPanel from '@/components/SuggestionPanel' import TagInput from '@/components/TagInput' +import Tabs, { type TabSpec } from '@/components/Tabs' import TranscriptPanel from '@/components/TranscriptPanel' /** Audio and video can be transcribed; nothing else has speech in it. */ @@ -22,11 +34,20 @@ const SPEECH_TYPES = new Set(['audio', 'video']) /** Documents have text read out of them instead — the same idea, a different source. */ const TEXT_TYPE = 'document' +/** + * Types with something to look at. They get a media well worth giving height to, and + * they are the only ones that gain anything from the side-by-side layout — a waveform-less + * audio bar beside a transcript would just be a tall black rectangle. + */ +const VISUAL_TYPES = new Set(['video', 'image']) + interface Props { asset: Asset onClose: () => void /** Seconds to start playback at — a search hit opening at the moment it matched. */ startAt?: number + /** What the close control says. `/a/:id` goes back to the library rather than closing. */ + closeLabel?: string } /** A row of the metadata table, rendered only when there is something to show. */ @@ -40,7 +61,21 @@ function Fact({ label, value }: { label: string; value: string }) { ) } -export default function AssetDetail({ asset, onClose, startAt }: Props) { +/** + * Everything about one asset, and nothing about where it sits. + * + * Positioning belongs to `DetailDock` — this fills whatever box it is handed, whether + * that is a panel docked beside the library, a full-screen sheet on a phone, or the + * whole of `/a/:id`. The root is a container query context, so the layout answers to the + * width it actually has rather than the viewport's: drag the panel past `@4xl` and the + * media moves beside the tabs instead of sitting above them. + */ +export default function AssetDetail({ + asset, + onClose, + startAt, + closeLabel = 'Close', +}: Props) { const update = useLibraryStore((s) => s.update) const remove = useLibraryStore((s) => s.remove) const setAssetTags = useLibraryStore((s) => s.setAssetTags) @@ -56,42 +91,46 @@ export default function AssetDetail({ asset, onClose, startAt }: Props) { const [confirmingDelete, setConfirmingDelete] = useState(false) const [copied, setCopied] = useState(false) - const panelRef = useRef(null) + const descriptionRef = useAutoGrow(description) + const summaryRef = useAutoGrow(summary) + // The detail view owns the player element so the transcript can drive it. Passing a // ref down beats lifting playback state up: seeking is imperative, and mirroring // currentTime into React state on every frame would re-render the whole panel // sixty times a second. - const playerRef = useRef(null) + const [player, setPlayer] = useState(null) const [currentTime, setCurrentTime] = useState(0) // Seek once the player has enough metadata to accept it. Setting currentTime before // the browser knows the duration is silently ignored, which is the difference between // a search result that opens at the right moment and one that opens at zero. - const seekOnLoad = useCallback( - (player: HTMLVideoElement | HTMLAudioElement | null) => { - playerRef.current = player - if (!player || startAt === undefined) return + const attachPlayer = useCallback( + (element: HTMLVideoElement | HTMLAudioElement | null) => { + setPlayer(element) + if (!element || startAt === undefined) return const apply = () => { - player.currentTime = startAt + element.currentTime = startAt } - if (player.readyState >= 1) { + if (element.readyState >= 1) { apply() } else { - player.addEventListener('loadedmetadata', apply, { once: true }) + element.addEventListener('loadedmetadata', apply, { once: true }) } }, [startAt] ) - const seekTo = useCallback((seconds: number) => { - const player = playerRef.current - if (!player) return - player.currentTime = seconds - void player.play()?.catch(() => { - // Autoplay can be refused; the seek still happened, which is what was asked for. - }) - }, []) + const seekTo = useCallback( + (seconds: number) => { + if (!player) return + player.currentTime = seconds + void player.play()?.catch(() => { + // Autoplay can be refused; the seek still happened, which is what was asked for. + }) + }, + [player] + ) // Re-seed when a different asset opens in the same panel, or the fields would keep // showing the previous one's values. @@ -112,17 +151,6 @@ export default function AssetDetail({ asset, onClose, startAt }: Props) { ensureTagsLoaded() }, [ensureTagsLoaded]) - useEffect(() => { - const onKey = (event: KeyboardEvent) => { - if (event.key === 'Escape') onClose() - } - window.addEventListener('keydown', onKey) - return () => window.removeEventListener('keydown', onKey) - }, [onClose]) - - // What enrichment has cost on this asset. Re-read whenever the asset changes, which - // includes after a job finishes — EnrichmentButton refreshes it, and that bumps - // `metadata_modified_date`, so this picks up the new spend without its own poll. // The /a/{id} URL, which is what GN-4 says a Notes document should link to. Built from // `window.location.origin` rather than a configured base: whichever host the user is // looking at is the one their colleague can reach too. @@ -140,6 +168,9 @@ export default function AssetDetail({ asset, onClose, startAt }: Props) { } }, [asset.id]) + // What enrichment has cost on this asset. Re-read whenever the asset changes, which + // includes after a job finishes — EnrichmentButton refreshes it, and that bumps + // `metadata_modified_date`, so this picks up the new spend without its own poll. const [usage, setUsage] = useState(null) useEffect(() => { let current = true @@ -211,234 +242,259 @@ export default function AssetDetail({ asset, onClose, startAt }: Props) { } } - return ( -
{ - if (e.target === e.currentTarget) onClose() - }} - > -
+
+ + setName(e.target.value)} + /> +
+ +
+ +