UX Audit

Appearance

[UX] customer-portal — Trade-in machines ​

Draft from /ux-audit on 2026-07-30 (unattended batch run). Not filed. Repo: Adalen-Truck/customer-portal · Branch: develop @ 693a76c · Files reviewed: 18 Patterns: media/image-upload

Summary ​

The list and detail pages are competent; the create page is not. Selecting or removing the main image sends a PUT that the API expands into field ?? null for every other column, so an ordinary "add a photo" click silently deletes every text field already autosaved to the draft — the one finding here that is unambiguously a blocker. Around it sits a cluster of image-handling defects: the draft never rehydrates its photos, "Remove photo" only removes the local preview while the server copy rides along into the submission, and there is no upload progress at all.

Good news on the two things the assignment flagged: this flow does not repeat the warranty-submission A11Y-03 blocker (the hidden file inputs at TradeInMachineCreatePage.vue:402 and :444 are driven by real <button type="button"> proxies, so they are keyboard-reachable), and remote images correctly use <SafeImage> throughout. FORM-09, however, fails harder here than it did in warranty.

Project-level, already filed — not repeated below:

  • MSG-02 — getErrorMessage prefers the raw backend string; reached on every failure path in this feature. See PROJECT-LEVEL.md.
  • NAV-03 — one static document title app-wide. See PROJECT-LEVEL.md.
  • A11Y-04 — partially disproven: PageLayout (packages/ui/src/components/PageLayout.vue:103) does emit a real <h1>, and all three pages use it, so these pages have exactly one <h1>. Only the PrimeVue Card/Panel section-title elements remain unconfirmed (see Unverified). Worth correcting in PROJECT-LEVEL.md.

Findings ​

1. Selecting or removing the main image wipes every other field from the saved draft — Blocker · FORM-09 ​

Where: apps/web/src/pages/TradeInMachineCreatePage.vue:164-168 and :179; server-side cause at apps/api/src/modules/trade-in-submissions/trade-in-submissions.service.ts:55-67

What: onMainImageSelected persists the photo with an image-only body:

ts
await tradeInSubmissionsUpdate({
    path: { id: draftId.value },
    body: { image: mainImageBase64() },   // ← only key sent
    throwOnError: true
});

The service's update builds an explicit full-row object rather than a partial one:

ts
model: body.model ?? null,
year: body.year ?? null,
serialNumber: body.serialNumber ?? null,
priceExpected: body.priceExpected ?? null,
hours: body.hours ?? null,
location: body.location ?? null,
comment: body.comment ?? null,
// Only touch the main image when it is actually included — autosave PUTs
// omit it, and we must not wipe a previously-set photo.
...(body.image !== undefined ? { image: body.image } : {})

The author saw this exact hazard and guarded one direction (image) while leaving the other seven fields unguarded. An image-only PUT therefore writes NULL over model, year, serial number, expected price, hours, location and comment. removeMainImage (:179) does the same via { image: "" }, and is additionally fire-and-forget with .catch(() => {}), so it cannot even report the write it just made.

Why it matters: the ordinary sequence — fill in the form, watch the "Saving…" chip confirm it is stored, then attach the machine photo — destroys the stored copy. Local component state still shows the values, so nothing looks wrong. The user only discovers it after a reload, a closed tab, or a dropped session, at which point the draft is empty and they retype everything. Clicking the X on the main image wipes the draft with no follow-up save at all. The happy path survives only by accident, because handleSubmit:242 flushes saveDraft() before submitting and re-sends the in-memory values.

Ranked Blocker rather than High because this is not a data-loss risk: it is a deterministic, silent deletion triggered by a normal action, and it defeats the entire purpose of the draft-autosave feature the page is built around.

Fix: make the server update partial — spread only the keys actually present in the body, mirroring the guard already written for image:

ts
const patch: Prisma.TradeInSubmissionUncheckedUpdateInput = {};
for (const k of ["model","year","serialNumber","priceExpected","hours","location","comment","visibleInPortal","image"] as const) {
    if (body[k] !== undefined) patch[k] = body[k];
}

Belt and braces on the client: have onMainImageSelected / removeMainImage send { ...buildBody(), image: … } instead of an image-only body.

2. A reloaded draft loses its main image and every uploaded photo — High · FORM-09 ​

Where: apps/web/src/pages/TradeInMachineCreatePage.vue:89-108; DTO at apps/api/src/modules/trade-in-submissions/trade-in-submissions.dto.ts:96-110

What: onMounted rehydrates the seven text fields from the draft and stops there. It cannot do better: TradeInSubmissionResponse carries neither image nor photos (contrast TradeInMachineDetailResponse, which has both — apps/web/src/api/types.gen.ts:1649-1667). mainImage and uploadedPhotos start empty on every mount.

Why it matters: three distinct harms, all invisible to the user. (a) After a reload the photo tiles are gone but the photos are still attached server-side, so the user re-picks and re-uploads and the submission silently accumulates duplicates. (b) The main image is still stored, so the page shows the "Add" placeholder for a machine that already has a main image. (c) The draft is a per-user singleton (getOrCreateDraft, trade-in-submissions.service.ts:35-50), so a user who abandons one machine's draft and later starts a different machine inherits the abandoned photos — invisibly, with no way to see or remove them — and they are submitted to Aadalen as photos of the new machine.

The pattern's own testing guidance is explicit here: "Confirm that state survives refresh, navigation, or retry in the way users would expect."

Fix: add image and a photos: {documentId, url}[] array to TradeInSubmissionResponse (the data already exists — see finding 3), and hydrate mainImage/uploadedPhotos from it in onMounted. Separately, surface a "this is a resumed draft / start over" affordance so an inherited draft is a deliberate choice rather than a surprise.

3. "Remove photo" removes only the preview; the photo stays on the submission — High · proposed CONTROL-EFFECT (see Baseline additions) ​

Where: apps/web/src/pages/TradeInMachineCreatePage.vue:216-223

What: the code comments its own defeat:

ts
// NOTE: the upload endpoint returns 204 with no documentId, so we cannot map a
// preview back to a server-side photo for deletion. Removing here only drops the
// local preview; the already-uploaded photo stays on the submission.

The premise is wrong. DocumentsService.uploadPortalNative returns a DocumentResponse with an id (apps/api/src/modules/documents/documents.service.ts:340-346); TradeInSubmissionsService.uploadPhoto throws it away to return void (trade-in-submissions.service.ts:72-75), and the controller pins the response to 204 (trade-in-submissions.controller.ts:47-48). The delete endpoint that would consume that id already exists and is already wired (trade-in-submissions.controller.ts:63-75; client type at apps/web/src/api/types.gen.ts:4271-4281), as does GET /entities/:entityType/:entityId/documents for listing them (documents.controller.ts:65). The id the frontend says it lacks is discarded one layer up.

Why it matters: a button labelled "Remove photo" that visibly removes the photo, while the photo is in fact submitted to Aadalen for a commercial offer, is a control whose observable effect contradicts its label. A user who accidentally uploads the wrong machine — or a photo they did not intend to send — has no way to take it back and no way to know they failed. Combined with finding 2, the photo is not even visible on the next visit.

Ranked High rather than Blocker because the create flow still completes; the harm is an unremovable artefact plus false feedback, not a broken submission.

Fix: return 201 with the DocumentResponse from uploadPhoto, keep the documentId alongside each preview, and have removePhotoPreview call tradeInSubmissionsDeletePhoto before dropping the tile — reporting failure rather than swallowing it.

4. If the draft fails to load, the form still renders and every save silently does nothing — High · MSG-03, MSG-04 ​

Where: apps/web/src/pages/TradeInMachineCreatePage.vue:102-107, with the downstream guards at :112, :142, :192, :232

What: the onMounted catch shows a 4–5s toast and then falls through to isLoadingDraft.value = false, so the full editable form renders with draftId === null. From there every action is a silent no-op: saveDraft returns at :112, photo handlers return at :142/:192, and — worst — handleSubmit returns at :232 before setting isSubmitting or raising any toast. Pressing "Submit machine" does literally nothing, with no feedback, for as long as the page is open.

Why it matters: the toast has vanished by the time the user finishes filling in six fields and attaching photos. They then press the primary action repeatedly against a form that cannot save and cannot submit, and lose all of it. The error text says only "Could not load the draft" — no cause, no next step, no retry, no route back to the list. That is MSG-03 (say what to do next) and MSG-04 (a dead end must offer a route out) failing together, and it violates the repo's own apps/web/CLAUDE.md rule 7, "No silent failures".

Fix: on draft-load failure render an error state instead of the form — a <Message severity="error"> with a Retry button that re-runs the fetch and a link back to trade-in-machines.index. As a floor, make handleSubmit toast when draftId is null rather than returning silently.

5. Filters and the result count only see the loaded page, not the data set — High · proposed FILTER-SCOPE (see Baseline additions) ​

Where: apps/web/src/pages/TradeInMachinesPage.vue:67-88, :29-37, :298; paging at packages/domain-helpers/src/create-domain-helpers.ts:86

What: useTradeInMachineList() is called with no options, so it server-paginates at pageSize: 50 and rows holds one page. Only free-text search reaches the API (trade-in-machines.collection.ts:13-24). Visibility, location, year range and price range are all applied client-side over that single page (filteredData, :67-88), while total and totalPages still come from the server. Two consequences compound it: the location dropdown is built from the loaded page only (locationOptions, :29-37), so its contents change as you page; and the summary line reads t("tradeIn.list.showing", { filtered, total }) — "Showing 3 of 412" — mixing a page-scoped numerator with a data-set-scoped denominator.

Why it matters: a user filtering for machines between 300 000 and 500 000 NOK sees the matches from the first 50 rows and is told that is what exists. Machines on later pages never appear unless the user happens to page into them and re-reads the filtered view. Pagination controls remain active and offer pages that appear inconsistent with the active filter. The empty state — "No machines match the current filters." — asserts a fact about the whole data set that the code only checked against 50 rows. This is a list of assets with money attached; a wrong "none found" is a real commercial answer.

Fix: pass the filters through useList({ filters }) — the helper already supports it (create-domain-helpers.ts:41-46, :99-105) and already resets to page 1 on change — and add the corresponding query params to tradeInMachinesList. Source locationOptions from a dedicated endpoint or a distinct-values query, not from the current page. Until the API supports it, the count must not mix scopes.

6. On mobile, list rows are click-only Cards with no keyboard or screen-reader affordance — High · A11Y-03 ​

Where: apps/web/src/pages/TradeInMachinesPage.vue:209-213

What: the #card slot renders <Card class="... cursor-pointer" @click="handleRowClick(...)"> — no role, no tabindex, no @keydown. This is the page's own gap rather than the library's: DataTable's table path does it correctly (packages/ui/src/components/DataTable.vue:613-616 sets role="button", tabindex="0" and @keydown.enter), while the card path wraps the slot in an inert <div class="h-full"> (DataTable.vue:544-561) and leaves interactivity to the consumer.

Why it matters: forceCardOnMobile makes the card list the only rendering at ≤760px whenever a #card slot exists, so on a phone there is no table to fall back to. A keyboard or switch-control user cannot open any trade-in machine at all, and a screen reader announces the card as static text with no indication it is actionable. The repo is explicitly mobile-first (apps/web/CLAUDE.md §12), which makes this the primary path, not an edge case.

Fix: give the card an accessible trigger — role="button", tabindex="0", @keydown.enter/@keydown.space and an :aria-label naming the machine — or better, fix it once by having DataTable's card wrapper apply the same treatment it already applies to <tr> when onRowClick is set. The latter almost certainly fixes the same defect across every other list page in the app.

7. No field has a programmatically associated label — Medium · FORM-01 ​

Where: apps/web/src/pages/TradeInMachineCreatePage.vue:305, :315, :327, :337, :348, :360, :372, :413, :454; also apps/web/src/pages/TradeInMachinesPage.vue:261, :279

What: every label is a bare <label class="text-sm font-semibold text-text"> with no for, and the <InputText> / <InputTextarea> sits as a sibling rather than a child. Whatever id PrimeVue does or does not generate internally is irrelevant — an unassociated <label> cannot bind from the label side.

Why it matters: screen readers announce nine unlabelled edit fields on the create form and four on the list filters; the only cue left is the placeholder, which disappears on focus and which FORM-01 explicitly disallows as the sole label. Clicking a label also fails to focus its field, which costs every user a larger tap target on mobile. The media/image-upload reference markup makes the same point in miniature: <label for="upload-input"> + <input id="upload-input">.

Fix: give each control an id (or use PrimeVue's inputId where offered) and point the label's for at it. useId() keeps them unique.

8. Submit is disabled while invalid, and nothing says which field is missing — Medium · FORM-05 ​

Where: apps/web/src/pages/TradeInMachineCreatePage.vue:465-471, with canSubmit at :60-66 and the unreachable guard at :233-236

What: :disabled="!canSubmit" gates the only submit control on five required fields being non-empty. There is no per-field validation message anywhere on the form — the sole signal is a * appended to the label strings in en.yml/nb.yml (modelLabel: "Model *"). The handleSubmit branch that would explain the block — toast.add({ severity: "warn", summary: t("tradeIn.create.requiredError") }) — can never fire, because a disabled button does not emit submit. tradeIn.create.requiredError and expectedPriceNumber are effectively dead strings.

Why it matters: a user who has filled in five of six required fields sees a greyed-out button and no explanation. On a form this long, with the required marker carried in the label text rather than in any validation surface, "which one did I miss" requires re-reading every field. FORM-05 exists precisely because a disabled submit gives the user nothing to act on.

Fix: keep the button enabled, let handleSubmit run its canSubmit check, and additionally mark the offending fields — invalid on the PrimeVue input plus a <Message size="small" severity="error"> bound with aria-describedby.

9. Photo size and format limits are only revealed after they are violated — Medium · FORM-04 ​

Where: apps/web/src/pages/TradeInMachineCreatePage.vue:19, :144-151, :194-201, :404, :446

What: the 2 MB ceiling (MAX_PHOTO_BYTES) and the JPEG/PNG restriction (accept="image/jpeg,image/png") are enforced but never stated. The tiles read only "Add" / "Legg til". Errors — "The photo is too large (max 2 MB)." — appear solely as a toast after the user has already chosen a file.

Why it matters: a modern phone camera produces 3–8 MB JPEGs, so the default outcome for a user photographing a machine on site is rejection after the fact. Repeat that per photo, on mobile data, and the flow becomes trial and error. The constraint is knowable up front; FORM-04 requires it be shown up front. The pattern's reference markup does exactly this: a persistent hint reading "PNG or JPG up to 5 MB." next to the picker.

Fix: render the constraint as static helper text under the "Main image" / "Additional photos" labels (new i18n key, en + nb). Consider client-side downscaling before upload, which would remove the limit as a user-facing concern entirely.

10. Async states in the create form are neither announced nor visible where it counts — Medium · MSG-01, CONTENT-04 ​

Where: apps/web/src/pages/TradeInMachineCreatePage.vue:279-285 (autosave chip) and :203-214 (photo upload)

What: two gaps. (a) The "Saving…" chip is a plain <span> with a spinner and no aria-live, so autosave activity is conveyed visually only; there is no "Saved" resting state either, so the persistence contract is communicated entirely by a transient spinner. (tradeIn.create.draftSaved exists in both locale files but is never rendered.) (b) onPhotoSelected awaits tradeInSubmissionsUploadPhoto with no pending indicator whatsoever — no spinner on the tile, no disabled state, no progress. The "+" tile stays fully live throughout.

Why it matters: uploading a 2 MB photo over mobile data takes seconds during which the UI is indistinguishable from idle. The natural response is to tap "+" again, and since input.value = "" (:191) deliberately permits re-selecting the same file, the second tap uploads a duplicate — which, per finding 3, can never be removed. The media/image-upload anatomy lists "Upload progress state" as a first-class component, and its accessibility guidance requires announcing "errors, loading, or completion… with the right politeness". A screen-reader user currently gets no indication that anything is happening in either case.

Fix: wrap the saving chip in role="status" aria-live="polite" and give it a settled "Saved" state; add a per-tile pending state (skeleton or spinner) for in-flight uploads and disable the "+" tile while one is running.

11. Uploaded photos carry alt="" and every remove button has the same name — Medium · A11Y-05 ​

Where: apps/web/src/pages/TradeInMachineCreatePage.vue:378-382 and :420-424 (alt=""), :384 and :426 (:aria-label="t('tradeIn.create.removePhoto')")

What: the user's own machine photos are marked decorative, and each of the N remove buttons in the gallery exposes the identical accessible name "Remove photo" / "Fjern bilde" with nothing to distinguish index, filename or position.

Why it matters: a screen-reader user hears "Remove photo, button" repeated N times over a set of images they were given no way to tell apart, and must guess which one they are deleting — an unrecoverable action, since re-uploading the right one is possible but removing the wrong one is not (finding 3). This is the pattern's "Treating media as decoration only" anti-pattern applied to content the user themselves supplied. Note the detail page gets the main case right (TradeInMachineDetailPage.vue:168 uses :alt="machine.model").

Fix: name each control by its subject — :aria-label="t('tradeIn.create.removePhotoN', { n: index + 1 })" — and give each preview a meaningful alt (the filename captured at pick time is enough). Keep alt="" only for the Galleria thumbnail strip, where the main image already carries the name.

12. Table headers keep the old language after a locale switch; money is always formatted nb-NO — Medium · CONTENT-01 ​

Where: apps/web/src/pages/TradeInMachinesPage.vue:121-143 and :92-97; apps/web/src/pages/TradeInMachineDetailPage.vue:38-42

What: columns is a plain const whose nine t("tradeIn.list.columns.*") calls are evaluated once during setup, not a computed. The app has a runtime language switcher (apps/web/src/components/LanguageSwitcher.vue:23, ProfilePage.vue:109), so switching locale while on the list leaves every column header in the previous language until the component remounts. Separately, both pages hardcode new Intl.NumberFormat("nb-NO", …) regardless of active locale.

Why it matters: an English-locale user looking at the trade-in list sees "Nummer / Modell / År / Serienr / Timer" above English data, immediately after explicitly asking for English. Key parity is otherwise clean here — I checked every tradeIn.* key in en.yml and nb.yml (lines 788-889) and both are complete with genuine Norwegian translations, no TODO placeholders — so this is a reactivity bug, not a translation gap.

Ranked Medium rather than Low because the user made an explicit request and was half-ignored; it self-heals on navigation, which is the argument for Low.

Fix: wrap columns in computed(() => [...]). Derive the number/currency locale from useI18n().locale (nb → nb-NO, en → en-GB), ideally via a shared money formatter rather than three local copies.

13. The create page subtitle describes the internal architecture to the customer — Low · proposed CONTENT-PLAIN (see Baseline additions) ​

Where: apps/web/src/i18n/messages/en.yml:849 and nb.yml:849

What: tradeIn.create.subtitle reads "Submit a new machine to Dynamics through the command queue." / "Send inn en ny maskin til Dynamics via kommandokøen." The same leak appears in tradeIn.create.success: "Trade-in machine created in Dynamics (command {commandId})."

Why it matters: this is a customer-facing route (trade-in-machines.create, gated on create TradeInMachine), and the reader is a farmer or contractor trading in a machine. "Dynamics" and "the command queue" are Aadalen's internal CRM and job infrastructure; naming them tells the customer nothing and reads as an internal tool leaking out. The correct copy already exists three lines below in submitSuccess: "The machine has been submitted to Aadalen." / "Maskinen er sendt inn til Aadalen."

Fix: rewrite the subtitle in the customer's terms — "Tell us about the machine you want to trade in. We'll come back to you with an offer." — and drop the commandId from any customer-visible string.

Unverified ​

  • A11Y-01 (contrast). Not determinable from code. The muted ramp (text-text-3 on the "Saving…" chip, text-text-2 on card metadata) and the white-on-bg-black/55 remove buttons over arbitrary user photos are the ones worth measuring — the latter has no guaranteed backdrop at all.
  • A11Y-06 (short viewport / mobile keyboard). Needs a rendered page. The create form is long (nine controls plus two photo galleries) and the submit button is not sticky, so behaviour with a keyboard open is worth checking at 320/360/390 px.
  • PrimeVue internals — node_modules is not installed, per the standing caveat in PROJECT-LEVEL.md. Unsettled here: whether Card's #title and Panel's #header render a heading element or a div (A11Y-04 section headings on all three pages); whether Button :loading also sets disabled (FORM-06 on the submit button at :465-471, which would drop focus to <body> mid-submission); and whether Toast renders an aria-live region (MSG-01 — toasts are the only error surface on the create page, so if it does not, findings 4 and 10 get materially worse).
  • A11Y-02 — the remove-photo buttons are h-6 w-6 = exactly 24×24 px, which meets WCAG 2.5.8 at the floor with no margin. Rendered check advisable.

Baseline additions ​

Descriptive IDs with definitions, per the brief — the orchestrator should renumber. Two of these are near-certainly the same rules already proposed under the MSG-06 and FORM-12 collisions listed in PROJECT-LEVEL.md.

  • CONTROL-EFFECT — A control's observable effect must match its label. A control that appears to succeed while the underlying operation was never attempted (or was attempted and failed) is a defect regardless of whether the UI updated. Finding 3. This is the MSG-06(a)/(d) family — "non-throwing result discarded, failure shown as success" — generalised from fetches to any action, and should merge into whatever MSG-06 becomes.

  • FILTER-SCOPE — A filter must apply to the same data set the result count and empty state describe. Client-side filtering over one page of a server-paginated list, presented against a server-side total, reports false negatives. Finding 5. Same rule as FORM-12(c) ("inert controls — filters that never reach the API"), broadened: the filter here is not inert, it is incorrectly scoped, which is harder to notice and worse.

  • PATCH-PARTIAL — A partial update must send and apply only the fields it intends to change. Expanding an absent field to null server-side turns any single-field save into a destructive full-row write. Finding 1. Arguably a backend rule rather than a UX one, but the user-visible consequence is silent data loss during autosave, and autosave is a UX contract. Worth a rule because I expect it wherever draft autosave exists.

  • CONTENT-PLAIN — User-facing copy names things in the user's terms, never the implementing system's. Internal system names, queue/command vocabulary and correlation identifiers do not appear in customer-facing strings. Finding 13. Related to but distinct from the CONTENT-05(c) proposal ("mode switches labelled by situation not implementation").

Rules confirmed passing here, recorded so the corpus keeps its calibration: CONTENT-01 key parity (full en/nb coverage, real translations, no TODO markers), CONTENT-02 ("Add trade-in" → "Add trade-in machine"), FORM-10 (paste unblocked), NAV-01 (no timed redirect — the post-submit router.push at :254 is immediate), NAV-02 (onBeforeUnmount at :225-228 clears the autosave timer and revokes every object URL), and A11Y-03 for the file inputs specifically — see the cross-project note.

Cross-project note ​

The warranty-submission A11Y-03 blocker does not recur here. The hidden file inputs at TradeInMachineCreatePage.vue:402-408 and :444-450 use class="hidden" — the same idiom — but are not wrapped in a <label>. They are driven by real <button type="button"> proxies (:392-400, :435-442) that call input.click(), which is the correct accessible pattern: the button is focusable and named, the input is intentionally out of the tab order. Worth recording as the reference implementation when the warranty finding is filed, since the fix for warranty is to adopt exactly this shape from the same repo.

Likely to recur elsewhere:

  • Finding 6 (click-only mobile cards) — near-certain across every customer-portal list page, because DataTable's card path leaves interactivity to the consumer while the table path handles it. Marketplace (#9), Catalog (#12), Customer assets (#14), Work orders (#18), Partner orders (#19), Locations (#20) and Contacts (#21) all use the same idiom. Recommend promoting to PROJECT-LEVEL.md and fixing once in packages/ui/src/components/DataTable.vue rather than filing it seven more times. It also matches the A11Y-03 row already marked ✗ for all four projects in the alignment table.
  • Finding 5 (page-scoped filters) — the useList + local filteredData shape is generic; any customer-portal list that filters client-side over a 50-row page has it. Worth a targeted grep for filteredData computeds that do not pass filters to useList. tt-time-tracker uses TanStack Table over paginated APIs and is a strong candidate for the same shape.
  • Finding 7 (unassociated labels) — the bare <label class="text-sm font-semibold text-text"> + sibling PrimeVue input idiom appears in both files I read in this feature and is almost certainly the house style. Likely project-wide in customer-portal and, given the shared PrimeVue stack and no FormKit-style automatic association, in tt-time-tracker too. Worth one grep before filing per feature.
  • Finding 1 / PATCH-PARTIAL — a backend shape, so it will not transfer literally, but any project with draft autosave against a PUT endpoint deserves the same check. playout (event drafts) and tt-time-tracker (timesheet entries) are where to look.
  • Finding 12 (non-reactive t() in table column definitions) — applies to playout and customer-portal, the two projects with runtime locale switching. Not applicable to members/tt-time-tracker (no i18n layer).