fix(ux): smoothness follow-ups - detail pages, batches, toasts, and the last edges (#1633)

* fix(ux): smoothness follow-ups - detail pages, batches, toasts, and the last edges Follow-up batch to #1629: the six documented deferred items from dev_docs/loading_states_analysis.md, in the same vocabulary (first-load-only takeovers, background reconcile behind mounted content, row/button-level pending, sequence guards). - Invoice detail pages: kundfaktura and leverantorsfaktura detail no longer blank the whole page for one-field changes. fetchInvoice shows the blocking spinner/skeleton only before the first paint (or when the pager steps to a different invoice); Bokfor / status / finalize / payment / send / Attestera / Markera betald / kreditera refetch behind the mounted page, the acting button shows a spinner-in-button, and the handlers await the refetch so pending covers until the content reflects the new state. The supplier detail's single isProcessing boolean became processingAction so the spinner lands on the clicked button only. (The leverantorsfakturor LIST try/catch/res.ok item was already fixed by #1629.) - useDestructiveConfirm: confirm(opts, action?) can now carry the destructive operation, so the dialog's existing isLoading spinner actually shows while it runs, dismissal is blocked meanwhile, and confirm resolves false if the action throws. Adopted at the /transactions row delete and the supplier- invoice detail delete (which previously permitted duplicate DELETEs with zero feedback). - Batch parallelization: new lib/concurrency.ts mapWithConcurrency (bounded worker pool, order-preserving, tested). /transactions batch categorize / ignore / delete run per-row requests 5 at a time instead of strictly sequentially; the bulkbar counter ticks per completed row. - Toast-spam reduction: batch categorize rows run silent (exit animation, count decrement and state patch stay; no per-row Bokford or generic failure toast) and ONE aggregate toast reports "N bokforda[, M misslyckades]" with a single Angra alla action that pools the same /uncategorize endpoint over every booked row (per-row undo is feasible today, so the aggregate is too). Interactive escalations (SI/CI match suggestions, duplicate warning, activate-account) deliberately keep their dialogs. - Underlag row-click flash: InvoiceInboxWorkspace handleSelect seeds the detail pane synchronously from the clicked list row and starts the document load in parallel with the detail GET (which hydrates on arrival), so a row click never flashes the onboarding/empty state, and a stale-response guard keeps a slow fetch from overwriting a newer selection. - #1629 round-2 edges: /pending holds the loading state when a fetch for a not-yet-loaded tab FAILS (never renders the previous tab's rows under the new tab's header, and never fakes an empty state); /transactions clears transactions/skvRows (+ count/paging) and bumps both fetch sequences on company switch, and loadSkvRows got the same sequence-guard pattern as fetchTransactions. Gates: full vitest suite green (14772 passed), tsc byte-identical to the origin/main baseline (stash-diffed), eslint 0 errors on touched files (warnings identical to baseline), check:guards green, package-lock untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ui): harden action feedback against stale responses and failures Address the seven CodeRabbit findings on #1633: - invoices/[id] + supplier-invoices/[id]: latest-request guard in fetchInvoice (sequence token) so a mutation refresh overlapping pager navigation can never commit invoice A's state under invoice B's URL; the deferred related-document writes are guarded too - supplier-invoices/[id]: try/catch/finally in approve/book/mark-paid/ credit/uncredit so a rejected fetch()/json() clears processingAction instead of leaving every invoice action disabled until reload - transactions: extend the skattekonto sequence guard to the connection-status write so a status response started under the previous company cannot flip the reconnect banner for the new one - transactions: runCategorize resolves { ok, journalEntryId } so the batch aggregate counts a 200-with-null-journal-entry booking (flag flip) as success instead of narrating it as misslyckades; Angra alla only targets rows with an actual verifikat, since the storno endpoint rejects rows without one - transactions: shared undoneIdsRef lets "Angra alla" cancel a pending finishBooking state patch; a fresh booking clears its row's entry so re-booked rows still get their delayed patch - InvoiceInboxWorkspace: monotonic request tokens for the detail and document reads so a same-item reload cannot resolve out of order and paint a stale snapshot or document URL - messages: ICU plural for the success part of both partial batch descriptions in sv and en (1 bokford, not 1 bokforda) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-08-17 14:30:25 +02:00
parent 43a71aec3c
commit 93541d7186
12 changed files with 780 additions and 253 deletions
@@ -360,10 +360,13 @@ export default function InvoiceInboxWorkspace(_props: WorkspaceComponentProps) {
  const [docUrl, setDocUrl] = useState<string | null>(null)
  const [docMime, setDocMime] = useState<string | null>(null)
  const [docState, setDocState] = useState<DocumentLoadState>('none')
-  // Which selection the in-flight document read belongs to. The user can click
-  // another row while it is running, and a late resolution must not paint its
-  // outcome (a URL, or an error) onto the row that is now selected.
-  const docRequestRef = useRef<string | null>(null)
+  // Monotonic tokens for the in-flight detail and document reads. The user
+  // can click another row while one is running, but also re-request the SAME
+  // item (action refreshes, the processing->received re-select effect), so an
+  // id comparison is not enough: only the newest request of each kind may
+  // paint its outcome (a detail snapshot, a URL, or an error) onto the pane.
+  const detailRequestRef = useRef(0)
+  const docRequestRef = useRef(0)
  const [inboxAddress, setInboxAddress] = useState<InboxAddress | null>(null)
  // We asked for the inbox address and did not get an answer we can trust
  // (5xx, network, unparseable). Distinct from a 404, which honestly means no
@@ -727,8 +730,8 @@ export default function InvoiceInboxWorkspace(_props: WorkspaceComponentProps) {
  // document, still loading, ready, or "we could not load it". The pane must
  // never fall back to "Inget underlag bifogat" for a row that has a
  // document_id, which is what the old silent catch produced.
-  const loadDocument = useCallback(async (itemId: string, documentId: string | null) => {
-    docRequestRef.current = itemId
+  const loadDocument = useCallback(async (documentId: string | null) => {
+    const request = ++docRequestRef.current
    setDocUrl(null)
    setDocMime(null)
    if (!documentId) {
@@ -742,13 +745,13 @@ export default function InvoiceInboxWorkspace(_props: WorkspaceComponentProps) {
        { method: 'GET' },
        { timeoutMs: DOCUMENT_FETCH_TIMEOUT_MS, description: `document ${documentId}` },
      )
-      if (docRequestRef.current !== itemId) return
+      if (docRequestRef.current !== request) return
      if (!res.ok) {
        setDocState('error')
        return
      }
      const { data } = await res.json()
-      if (docRequestRef.current !== itemId) return
+      if (docRequestRef.current !== request) return
      const url: string | null = data?.download_url ?? null
      // HTML mail underlag renders via the same-origin inline proxy: it
      // serves text/html with a CSP sandbox header and guaranteed inline
@@ -767,39 +770,66 @@ export default function InvoiceInboxWorkspace(_props: WorkspaceComponentProps) {
    } catch {
      // Timeout, offline, or an unparseable body. The document itself is
      // untouched, so the retry in the preview pane is the whole recovery.
-      if (docRequestRef.current !== itemId) return
+      if (docRequestRef.current !== request) return
      setDocState('error')
    }
  }, [])

  const handleSelect = useCallback(async (id: string) => {
+    const request = ++detailRequestRef.current
    setSelectedId(id)
    setSelectedPurchaseId(null)
-    setSelected(null)
-    setDocUrl(null)
-    setDocMime(null)
-    setDocState('none')
-    docRequestRef.current = id
    // Intentionally no auto-scroll: in the vertical-stack layout (below xl)
    // scrolling the preview into view pushes the list off-screen, and the
    // user has no obvious way back to pick another item. The row-highlight
    // + the preview content update are enough feedback that the tap took.

+    // Seed the detail pane synchronously from the list row already in hand
+    // (fetchItems returns full rows: status, amounts, extracted fields), and
+    // start the document load in parallel with the detail GET. Clearing
+    // `selected` first made every row click flash the no-selection branch
+    // (onboarding card / "Välj en post") for a full round trip, then run a
+    // second serialized round trip before the PDF even started loading.
+    const listRow = items.find((it) => it.id === id) ?? null
+    if (listRow) {
+      setSelected(listRow)
+      void loadDocument(listRow.document_id)
+    } else {
+      // Invalidate any in-flight document read: the pane is being cleared,
+      // and a late resolution must not paint a URL or error onto it.
+      docRequestRef.current++
+      setSelected(null)
+      setDocUrl(null)
+      setDocMime(null)
+      setDocState('none')
+    }
+
    try {
      const res = await fetch(`/api/extensions/ext/invoice-inbox/items/${id}`)
      if (!res.ok) throw await resolveFailure(res)
      const json = await res.json()
      const item = json.data as InboxItem
+      // A newer selection owns the pane now: dropping this response keeps a
+      // slower earlier fetch (same item or another) from overwriting the
+      // newest request's detail snapshot.
+      if (detailRequestRef.current !== request) return
      setSelected(item)
-      await loadDocument(id, item.document_id)
+      if (!listRow) {
+        await loadDocument(item.document_id)
+      } else if (item.document_id !== listRow.document_id) {
+        // The detail row knows a different underlag than the list row we
+        // seeded from (e.g. processing finished between paint and click).
+        void loadDocument(item.document_id)
+      }
    } catch (err) {
+      if (detailRequestRef.current !== request) return
      toast({
        title: 'Kunde inte ladda dokumentet',
        description: failureText(err),
        variant: 'destructive',
      })
    }
-  }, [toast, loadDocument])
+  }, [items, toast, loadDocument])

  // The detail pane renders from its own fetched snapshot (`selected`), so
  // the realtime refetch updates the list row but would leave a selected
@@ -1758,7 +1788,7 @@ export default function InvoiceInboxWorkspace(_props: WorkspaceComponentProps) {
              docMime={docMime}
              isProcessing={!!selected.isPlaceholder}
              loadState={docState}
-              onRetry={() => { void loadDocument(selected.id, selected.document_id) }}
+              onRetry={() => { void loadDocument(selected.document_id) }}
            />
          ) : showOnboarding ? (
            <div className="h-full flex flex-col justify-center px-4 py-6">
@@ -61,10 +61,17 @@ describe('transactions page booking feedback', () => {

  it('lets a completed undo win over the delayed booked-state patch', () => {
    // The 350ms animation timer must not re-apply journal_entry_id after an
-    // Ångra has already storno-reversed the verifikat server-side.
+    // Ångra has already storno-reversed the verifikat server-side. The
+    // closure-local flag covers the per-row Ångra; the shared undoneIdsRef
+    // covers "Ångra alla", which runs outside finishBooking's closure.
    expect(PAGE_SRC).toMatch(/let undone = false/)
    expect(PAGE_SRC).toMatch(/undone = true/)
-    expect(PAGE_SRC).toMatch(/if \(!undone\) \{/)
+    expect(PAGE_SRC).toMatch(/if \(!undone && !undoneIdsRef\.current\.has\(id\)\) \{/)
+    // Both undo paths record into the shared ref, and a fresh booking clears
+    // its row's entry again so a re-booked row still gets its delayed patch.
+    expect(PAGE_SRC).toMatch(/undoneIdsRef\.current\.add\(id\)/)
+    expect(PAGE_SRC).toMatch(/undoneIdsRef\.current\.add\(undoneId\)/)
+    expect(PAGE_SRC).toMatch(/undoneIdsRef\.current\.delete\(id\)/)
  })

  it('clears only the finished row\'s spinner', () => {
@@ -119,21 +119,30 @@ interface ConfirmOptions {

 interface UseDestructiveConfirmReturn {
  dialogProps: DestructiveConfirmDialogProps
-  confirm: (options: ConfirmOptions) => Promise<boolean>
+  confirm: (options: ConfirmOptions, action?: () => void | Promise<void>) => Promise<boolean>
 }

 /**
 * Hook that returns a `confirm()` function as a drop-in replacement for `window.confirm()`.
 * Returns `Promise<boolean>`: true if user confirms, false if they cancel.
 *
+ * Pass the destructive operation itself as the second argument to run it
+ * INSIDE the confirm: the dialog stays open with its pending spinner (and
+ * blocks dismissal) until the action settles, instead of closing on click and
+ * leaving the fetch to run with no visible state anywhere. The action owns its
+ * own error feedback (toast); if it throws, `confirm` resolves `false` so a
+ * caller's success tail is skipped.
+ *
 * Usage:
 * ```
 * const { dialogProps, confirm } = useDestructiveConfirm()
 *
 * async function handleDelete() {
- *   const ok = await confirm({ title: '...', description: '...' })
+ *   const ok = await confirm({ title: '...', description: '...' }, async () => {
+ *     // the DELETE runs while the dialog shows its spinner
+ *   })
 *   if (!ok) return
- *   // proceed with deletion
+ *   // confirmed and the action completed
 * }
 *
 * return <><DestructiveConfirmDialog {...dialogProps} /></>
@@ -146,28 +155,57 @@ export function useDestructiveConfirm(): UseDestructiveConfirmReturn {
    description: '',
  })
  const resolveRef = useRef<((value: boolean) => void) | null>(null)
+  const actionRef = useRef<(() => void | Promise<void>) | null>(null)
+  const runningRef = useRef(false)

-  const confirm = useCallback((opts: ConfirmOptions): Promise<boolean> => {
-    setOptions(opts)
-    setOpen(true)
-    return new Promise<boolean>((resolve) => {
-      resolveRef.current = resolve
-    })
-  }, [])
+  const confirm = useCallback(
+    (opts: ConfirmOptions, action?: () => void | Promise<void>): Promise<boolean> => {
+      setOptions(opts)
+      actionRef.current = action ?? null
+      setOpen(true)
+      return new Promise<boolean>((resolve) => {
+        resolveRef.current = resolve
+      })
+    },
+    [],
+  )

  const handleOpenChange = useCallback((v: boolean) => {
    setOpen(v)
-    if (!v && resolveRef.current) {
-      resolveRef.current(false)
-      resolveRef.current = null
+    if (!v) {
+      actionRef.current = null
+      if (resolveRef.current) {
+        resolveRef.current(false)
+        resolveRef.current = null
+      }
    }
  }, [])

-  const handleConfirm = useCallback(() => {
+  const handleConfirm = useCallback(async () => {
+    // Re-entry guard: a second confirm firing while the action is still in
+    // flight must not resolve the promise early (the dialog disables its
+    // button on isLoading, this covers the same-tick edge).
+    if (runningRef.current) return
+    runningRef.current = true
+    const action = actionRef.current
+    actionRef.current = null
+    let completed = true
+    if (action) {
+      try {
+        // Awaited by the dialog's own handleConfirm, so its isLoading spinner
+        // shows for the duration and the dialog closes only when this settles.
+        await action()
+      } catch {
+        // The action surfaces its own error (toast); resolving false here
+        // keeps the caller's post-confirm tail from running on a failure.
+        completed = false
+      }
+    }
    if (resolveRef.current) {
-      resolveRef.current(true)
+      resolveRef.current(completed)
      resolveRef.current = null
    }
+    runningRef.current = false
  }, [])

  return {