docs(uat): SHIPPED annotations for PR11 (picker polish + currency + breadcrumb)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-05-21 18:07:40 +02:00
parent 2bcf544cbc
commit 901fc363a5

View File

@@ -298,10 +298,10 @@ _Component refactors, multi-file edits, single-service tweaks, new validators._
> - **Effort:** ~1.5h for email (UI + endpoint + audit + dev-redirect honour). +1-2h each for the sibling integration test-ping buttons if pursued in the same pass. Captured 2026-05-18 from UAT.
> - **YachtPicker: opening returns no yachts (empty `q` → empty list); should return a default list** — _src/app/api/v1/yachts/autocomplete/handlers.ts:10-12_ + _src/components/yachts/yacht-picker.tsx:56-60_ — the autocomplete handler short-circuits with `{ data: [] }` when `q` is empty: `if (!q) { return NextResponse.json({ data: [] }); }`. The picker fires the query the moment it opens with `debounced=''` → user opens, sees empty state, has to start typing before any options appear. Dead-end UX.
> - **Fix:** (a) handler: when `q` is empty, return the top 20-30 yachts for the port (most-recently-updated default; if `ownerType`/`ownerId` query params are provided, filter server-side to that owner). Trivial — just drop the early-return and pass `q` as optional to the `autocomplete()` service, which defaults to an empty search term meaning "no name filter". (b) Picker: extend the query string to include the owner filter so server-side filtering works (currently the picker filters client-side post-fetch, which means a yacht owned by someone other than the current `ownerFilter` may not even reach the client if it's outside the default-20). (c) UX nicety: the picker's `placeholder` could include "or search…" so the user knows typing also works.
> - **Effort:** ~30 min. Captured 2026-05-18 from UAT.
> - **Effort:** ~30 min. Captured 2026-05-18 from UAT. **SHIPPED (a) in 2bcf544:** autocomplete handler drops the early-return; service returns top 20 most-recently-updated yachts when `q` is empty. (b) owner-side server filtering remains client-side as before; (c) deferred.
> - **YachtPicker: selected yacht renders as `Yacht <uuid-prefix>` when not in the autocomplete results** — _src/components/yachts/yacht-picker.tsx:75-79_ — the trigger button label is `match?.name ?? `Yacht ${value.slice(0, 8)}``— the fallback fires whenever the currently-selected yacht isn't in`rawOptions`(e.g. picker was opened with a pre-set value from a URL param / parent default and the autocomplete results don't include it, OR the user typed a search that filtered it out). Result: reps see`"Yacht 3bd83076"` instead of the yacht's name.
> - **Fix:** add a second `useQuery` keyed on `['yacht-detail-label', value]` that fetches `/api/v1/yachts/{value}?fields=name` when `value` is set AND not present in `rawOptions`. Use its result as the fallback label in priority order: `match?.name ?? fallbackQuery.data?.name ?? `Yacht ${value.slice(0, 8)}``. Cache hit on repeat opens; tiny request. (b) Also pre-select the currently-managed yacht as the default `value`for any picker rendered in a context where "the current yacht" makes sense — that's a parent-prop concern; this picker handles whatever`value` it's given. (c) Sweep for the same pattern in other pickers (ClientPicker, CompanyPicker, BerthPicker if they exist) — same root cause + same fix shape.
> - **Effort:** ~20 min per picker; ~1h with the sweep. Captured 2026-05-18 from UAT.
> - **Effort:** ~20 min per picker; ~1h with the sweep. Captured 2026-05-18 from UAT. **SHIPPED (YachtPicker) in 2bcf544:** fallback `useQuery(['yacht-detail-label', value])` against `/api/v1/yachts/{value}` enabled only when value isn't in `rawOptions`. ClientPicker/CompanyPicker sweep deferred until UAT confirms the same pattern needs fixing there.
> - **CommandList (cmdk) inside a Popover: scroll caps short of the bottom — applies to ALL dropdowns using the Command primitive** — _src/components/ui/command.tsx:57-75_ — `CommandList` has `max-h-[300px] overflow-y-auto overscroll-contain` plus a custom wheel handler (lines 68-72) that re-implements scrolling because "native wheel scrolling is intercepted by the focus-scope and never reaches the cmdk list" (per the inline comment). User reports they can scroll a short distance, then the list stops responding before reaching the bottom — and notes this is the case for **every dropdown on the drawer they're looking at**, so it's the shared primitive, not a per-picker bug.
> - **Suspected causes (likely a combination):**
> - **(i) cmdk auto-scroll-to-highlighted-item** fights the manual scroll: when the user wheels past the currently-highlighted item, cmdk's internal handler snaps the scroll back so the highlighted item stays visible. Net effect: user can scroll up to a few items past the highlight, then it bounces back. **Fix attempt:** on wheel/scroll, clear the cmdk highlight (or set it to a non-highlighted state) so cmdk doesn't re-snap. cmdk exposes a `value` prop on `Command` for controlled-highlight; set it to `undefined` on scroll, restore on hover/keyboard nav.
@@ -436,7 +436,7 @@ _Component refactors, multi-file edits, single-service tweaks, new validators._
> - **(c) Layout polish: top-align the back-chevron** — _topbar.tsx:59_ — change the wrapping `<div className="min-w-0 flex items-center gap-1.5">` to `items-start` so even if the breadcrumb does wrap, the back-button stays top-aligned with the first crumb line instead of vertical-centering across the wrapped block. Also worth considering: hide the back-button when meaningful breadcrumbs are visible (the breadcrumb's parent link already does "go back"; two affordances is one too many). ~10 min.
> - **Topbar grid sizing observation:** topbar columns are `[minmax(0,1fr)_minmax(360px,640px)_minmax(0,1fr)]` — left slot competes for space with the centered search bar's `minmax(360px,640px)`. When search hits its max width, left slot is squeezed → breadcrumb wraps sooner. Consider bumping to `minmax(0,1.5fr)` OR letting the search shrink below 360px when needed. Optional, evaluate after (a)+(b) land.
> - **Effort:** ~15 min for (a), ~45 min for (b), ~10 min for (c). Bundle ~1h. Captured 2026-05-18 from UAT.
> - **BulkAddBerthsWizard: currency field should use `<CurrencySelect>` (already exists, used elsewhere)** — _src/components/admin/bulk-add-berths-wizard.tsx_ (the `priceCurrency` `<Input>` in the apply-to-all row at ~lines 282-290, and the per-row instance below it) — currently a free-text `<Input>` that uppercases on blur, defaulting to `USD`. Reps can type any string (including invalid codes); no auto-complete; no consistency with other forms. The `<CurrencySelect>` component already exists at _src/components/shared/currency-select.tsx_, backed by the curated `SUPPORTED_CURRENCIES` list in _src/lib/utils/currency.ts_, and is used by the single-berth edit form (_berth-form.tsx:414_) + the expense form dialog (_expense-form-dialog.tsx:238_). Quick fix: import `CurrencySelect`, replace both the apply-to-all and per-row currency inputs with the dropdown bound to the same handlers (`applyToAll('priceCurrency', v)` / `setRowField(idx, 'priceCurrency', v)`). ~10 min. Captured 2026-05-18 from UAT.
> - **BulkAddBerthsWizard: currency field should use `<CurrencySelect>` (already exists, used elsewhere)** — _src/components/admin/bulk-add-berths-wizard.tsx_ (the `priceCurrency` `<Input>` in the apply-to-all row at ~lines 282-290, and the per-row instance below it) — currently a free-text `<Input>` that uppercases on blur, defaulting to `USD`. Reps can type any string (including invalid codes); no auto-complete; no consistency with other forms. The `<CurrencySelect>` component already exists at _src/components/shared/currency-select.tsx_, backed by the curated `SUPPORTED_CURRENCIES` list in _src/lib/utils/currency.ts_, and is used by the single-berth edit form (_berth-form.tsx:414_) + the expense form dialog (_expense-form-dialog.tsx:238_). Quick fix: import `CurrencySelect`, replace both the apply-to-all and per-row currency inputs with the dropdown bound to the same handlers (`applyToAll('priceCurrency', v)` / `setRowField(idx, 'priceCurrency', v)`). ~10 min. Captured 2026-05-18 from UAT. **SHIPPED in 2bcf544.**
> - **BulkAddBerthsWizard + single-berth editor: toggleable input units (ft/m) for dimension fields** — _src/components/admin/bulk-add-berths-wizard.tsx_ (the "Width (ft)" / "Length (ft)" / "Draft (ft)" inline-table headers + input parsing), _src/components/berths/berth-form.tsx_ (or equivalent single-edit) — the wizard's column headers and input parsing are hard-coded to feet. The schema supports per-dimension entry-unit discriminators (`lengthUnit`, `widthUnit`, `draftUnit` on `berths`, all defaulting to `'ft'`) plus separate `_M` numeric columns where metres-original values live — but neither the bulk wizard nor the single editor lets the rep pick which unit they're typing in. Reps who think in metres convert manually and the entry-unit discriminator never gets set.
> - **Fix:** (a) add a small `ft | m` toggle in the wizard header (and on the single-berth edit form) that flips the column header labels (e.g. "Width (ft)" → "Width (m)") and the parser. The toggle should default to whichever unit the user's `dimensionUnit` preference is set to (see the Dimensions-column-toggle finding earlier — same preference). (b) On submit, if entered unit is `'m'`, convert to ft for the stored numeric (`berths.lengthM` is the canonical metres column; `lengths.lengthFt` would be the feet column — verify the actual column names) AND set `lengthUnit='m'` so downstream document generation honours the rep's original input. Same for width / draft / nominalBoatSize / waterDepth. (c) Reuse the `src/lib/utils/dimensions.ts` helper from the Dimensions-column finding so conversion is centralized.
> - **Why this matters beyond UX:** document-generation merge fields (EOI / contract) already pull entry-unit values per `effectiveDimensionUnit` so the legal doc matches the rep's intent. Hard-coding ft on input silently coerces metric reps' values through a mental conversion, then renders the resulting ft figure on documents — losing fidelity for European customers.
@@ -447,7 +447,7 @@ _Component refactors, multi-file edits, single-service tweaks, new validators._
> - **Action item:** check whether `docks` / `pontoons` / `marina_sections` table exists in the schema (`grep -r "docks\|pontoons" src/lib/db/schema/`); shape the fix accordingly. If no dedicated table, the wizard fix is trivial; if there is one, decide admin-only vs in-wizard-create with the team. Captured 2026-05-18 from UAT.
> - **DropdownMenu content stretches to fill viewport — cap it** — _src/components/ui/dropdown-menu.tsx:66_ — the shadcn `DropdownMenuContent` primitive uses `max-h-(--radix-dropdown-menu-content-available-height)` (Radix's CSS variable that exposes the room between the trigger and the viewport edge). On long lists the menu visually stretches all the way to the viewport bottom even though the items don't need that height; reads as a wall of options. Internal `overflow-y-auto` is already on so scrolling works. Fix: replace the Radix `max-h-(...)` token with a fixed `max-h-96` (384px) or `max-h-[28rem]` (448px) so the menu caps at a comfortable height regardless of available space, scrolling internally for longer lists. Global change in the base primitive — affects every dropdown in the app, which is the right call (no consumer currently relies on the "fill the viewport" behaviour). ~2 min. If a specific dropdown needs the old behaviour, it can pass `className="max-h-[var(--radix-dropdown-menu-content-available-height)]"` to opt back in. Captured 2026-05-18 from UAT. **SHIPPED in c6dcf49.**
> - **DocumentsHub aside column: flush-left with the app sidebar (kill the AppShell padding for this page)** — _src/components/documents/documents-hub.tsx:246_ + _src/components/layout/app-shell.tsx:113-121_ — the desktop `<main>` wrapper applies `px-6 pt-3 pb-6` to all dashboard pages, so the DocumentsHub two-pane (`ResizablePanelGroup` with the `<aside>` folder column on the left) gets 24px of whitespace between the global app sidebar and its own border. The folder column should sit flush against the app sidebar — it reads as "an extension of the navigation," not "a card inside the page." Fix (surgical): change DocumentsHub's root `<div className="h-full">` at line 246 to `<div className="h-full -mx-6 -mt-3 -mb-6">` (mirror the AppShell desktop padding so the hub renders full-bleed inside the main viewport). Add a comment explaining the intentional escape. The right-pane content keeps its own internal `p-4` so it doesn't run flush with the viewport edge. **Alternative (cleaner long-term):** make the AppShell padding route-aware via a prop on `<main>` (or a layout-level opt-out for hub-style pages); but (a) is the right call until a second page needs the same treatment. ~5 min for the negative-margin fix. Captured 2026-05-18 from UAT.
> - **DocumentsHub: hide breadcrumb on root "All documents" view, move PageHeader up to fill the space** — _src/components/documents/documents-hub.tsx:196-209_ — the top row currently always renders the `FolderBreadcrumb` (and conditionally the `NewDocumentMenu` when a folder is selected); on the root view (`selectedFolderId === undefined`) the breadcrumb shows only a "Home / All documents" label with no useful navigation, eating vertical space above the `PageHeader` that already says "Documents" + description. Fix: wrap the entire breadcrumb row at line 196-209 in `{selectedFolderId !== undefined && ( … )}` so the row is gone on the root; the PageHeader becomes the top element. When the rep navigates into a folder, the row reappears with both breadcrumb + NewDocumentMenu (the existing folder views don't render PageHeader, so the breadcrumb is the wayfinding cue). ~5 min. Captured 2026-05-18 from UAT.
> - **DocumentsHub: hide breadcrumb on root "All documents" view, move PageHeader up to fill the space** — _src/components/documents/documents-hub.tsx:196-209_ — the top row currently always renders the `FolderBreadcrumb` (and conditionally the `NewDocumentMenu` when a folder is selected); on the root view (`selectedFolderId === undefined`) the breadcrumb shows only a "Home / All documents" label with no useful navigation, eating vertical space above the `PageHeader` that already says "Documents" + description. Fix: wrap the entire breadcrumb row at line 196-209 in `{selectedFolderId !== undefined && ( … )}` so the row is gone on the root; the PageHeader becomes the top element. When the rep navigates into a folder, the row reappears with both breadcrumb + NewDocumentMenu (the existing folder views don't render PageHeader, so the breadcrumb is the wayfinding cue). ~5 min. Captured 2026-05-18 from UAT. **SHIPPED in 2bcf544.**
> - **Residential InterestsTab: whole row should navigate to the interest, not just the "View" link** — _src/components/residential/residential-client-tabs.tsx:273-289_ — current `<li>` lays out `[stage chip] [preferences/notes truncated text] [View → link]` and only the "View" text on the right is clickable. The whole row should be a target, matching the idiom used in the main client's `InterestRowItem` (`src/components/clients/client-interests-tab.tsx:53`) — the entire card is a `<button>`/`<Link>` so reps can tap anywhere. Fix: wrap the `<li>`'s flex container in `<Link href={…}>` (`className="block w-full"` to preserve layout), drop the trailing "View" link, add `hover:bg-muted/50` to make the affordance discoverable. ~10 min. Captured 2026-05-18 from UAT. **SHIPPED in c6dcf49.**
> - **Residential namespace breadcrumb link is 404** — _src/components/layout/breadcrumbs.tsx_ (the breadcrumb generator splits the URL and makes every segment a link) + missing _src/app/(dashboard)/[portSlug]/residential/page.tsx_ — on any `/{portSlug}/residential/clients` or `/{portSlug}/residential/interests` page, the breadcrumb renders "Residential" as a link to `/{portSlug}/residential` but no `page.tsx` exists at that path (only `clients/` and `interests/` subdirectories). Clicking the breadcrumb yields a 404. Two reasonable fixes:
> - **(a) Quickest:** create `src/app/(dashboard)/[portSlug]/residential/page.tsx` as a server component that calls `redirect(`/${portSlug}/residential/clients`)`. Single file, ~5 min, breadcrumb works immediately. Same pattern works for any other namespace-only segment that lacks a real landing page.