fix(ux): actions update lists in place - no more takeovers, jumps and dead air (#1629)

* fix(ux): update lists in place on actions instead of takeover spinners and jumps

Founder report: the app feels glitchy when clicking around, especially
when deleting a row or booking something. The repo-wide anti-pattern
behind it: single-row actions trigger whole-list skeleton/spinner
takeovers (layout collapse, scroll jump, full stagger-enter replay),
deletes give zero feedback then hard-jump, and the /transactions exit
"animation" was filter-only and never animated.

Per surface:

- Never take over a rendered list for a background refresh. The
  skeleton/spinner swap is now reserved for an empty (or foreign) list
  on /transactions (fetchTransactions), /pending (fetchOperations,
  covering both listed Granskning findings, one file), kundfakturor
  (fetchInvoices), leverantörsfakturor (fetchInvoices, plus
  try/catch/finally so a failed fetch can no longer stick the skeleton
  or masquerade as an empty register) and the verifikat list
  (JournalEntryList now takes a refreshToken prop and refetches in
  place; /bookkeeping no longer key-remounts it into a spinner, so
  expansion/selection/pagination/scroll survive a created verifikat).
  Quiet inline Loader2 cues near the list headers on /transactions and
  /pending signal a background reconcile.

- /transactions row exit: exiting rows (booked/ignored/deleted) stay
  rendered through the existing 350ms window with a real exit
  transition (.row-exit: fast fade, then the space closes by
  transitioning cell paddings/line metrics and a numeric max-height on
  the fixed-height cell spans) and pointer-events off. Instant removal
  under prefers-reduced-motion. Applied to the inbox cards, the
  skattekonto card and the history rows.

- /transactions delete: routes through processingId (row spinner) and
  the exitingIds path, and decrements totalUncategorizedCount when the
  deleted row was pending (the realtime echo is not guaranteed for
  DELETE on a filtered subscription).

- FyPicker double-fetch: the initial fetch now waits for FyPicker's
  onReady (fires after its persisted-scope restore), so mount does one
  correctly scoped fetch instead of racing an unscoped fetch against
  the restore refetch (list -> skeleton -> list on every visit). Period
  changes refetch background-only behind the client-filtered list.

- Pagination survives realtime echoes: background refreshes re-fetch
  range(0, pagedCountRef) instead of resetting to the first 200 rows,
  so "Visa fler" pages no longer collapse after any action.

Gates: full vitest suite green (14764 passed), tsc output byte-identical
to the origin/main baseline, eslint 0 errors on touched files,
check:guards green, package-lock untouched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(ui): apply review round on action-feedback smoothness

- /pending: sequence-guard fetchOperations so a stale previous-tab
  response can't overwrite the current tab's rows, counts, or loading cues
- /pending: check res.ok on the pending fetch and both history fetches
  before applying payloads; failures keep current rows and surface the
  existing error toast
- /transactions: reset fiscal scope (fyReady/fyPeriodId/fyPeriod) during
  render on company switch so FyPicker re-runs its persisted restore and
  stale bounds never scope a fetch for the wrong company
- /transactions: drop a deleted row's id from selectedIds so the bulk bar
  can't act on a deleted row
- row exit: add the inert attribute on exiting row wrappers alongside
  pointer-events so keyboard focus and activation are blocked too
- JournalEntryList: preserve selection on refreshToken background
  refreshes (reconciled against the refreshed page); user-initiated
  reloads still clear it

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>
This commit is contained in:
Jakob Wennberg
2026-08-16 18:44:02 +02:00
committed by GitHub
co-authored by Claude Fable 5 Jakob Wennberg
parent 4dbd19aeb0
commit c897a906df
14 changed files with 385 additions and 43 deletions
+4 -1
View File
@@ -265,8 +265,11 @@ export default function BookkeepingPage() {
}
/>
{/* refreshToken, NOT key: a created verifikat refreshes the list in
place (dim + refetch) instead of remounting it into a spinner and
losing expansion/selection/scroll. */}
<JournalEntryList
key={refreshKey}
refreshToken={refreshKey}
pristineSlot={
<div className="animate-fade-in space-y-4">
<StartCard
+5 -1
View File
@@ -280,7 +280,11 @@ export default function InvoicesPage() {
async function fetchInvoices() {
if (!company) return
setIsLoading(true)
// Skeleton takeover only while nothing is on screen: refetches after an
// action (bulk Bokför) reconcile BEHIND the rendered table. Collapsing
// hundreds of rows to 3 skeleton stubs and replaying the stagger-enter
// entrance for a row-scoped action was the "booking feels glitchy" jump.
if (invoices.length === 0) setIsLoading(true)
const [invoicesResult, settingsResult] = await Promise.allSettled([
fetchAllRows<Invoice>(
({ from, to }) =>
+54 -5
View File
@@ -1,6 +1,6 @@
'use client'
import { useState, useEffect, useCallback, useMemo } from 'react'
import { useState, useEffect, useCallback, useMemo, useRef } from 'react'
import { useRouter } from 'next/navigation'
import { useTranslations } from 'next-intl'
import { Badge } from '@/components/ui/badge'
@@ -324,32 +324,73 @@ export default function PendingOperationsPage() {
if (conv) setConversationFilter(conv)
}, [])
// Which tab the rows in `operations` belong to, and whether anything is on
// screen: lets refetches decide between the first-load takeover and a
// background reconcile without making row state a useCallback dependency
// (which would re-subscribe the realtime channel on every data change).
const loadedTabRef = useRef<ViewTab | null>(null)
const hasRowsRef = useRef(false)
useEffect(() => {
hasRowsRef.current = operations.length > 0
}, [operations])
// Background reconcile in flight: drives the quiet toolbar cue.
const [isRefreshing, setIsRefreshing] = useState(false)
// Monotonic request sequence: a fetch kicked off for a previous tab (or an
// older realtime echo) can resolve after the current one. Only the latest
// request may touch rows, counts, refs, toasts, or loading cues; stale
// responses bail out and leave the newer request's state alone.
const fetchSequenceRef = useRef(0)
const fetchOperations = useCallback(async () => {
setIsLoading(true)
const sequence = ++fetchSequenceRef.current
const isCurrent = () => sequence === fetchSequenceRef.current
// The list-for-spinner swap is reserved for a first load or a tab whose
// rows aren't on screen yet. Approving/rejecting a row (and its realtime
// echo) used to run list → spinner → list → spinner → list: a whole-page
// layout collapse plus a full stagger-enter replay, twice, for a
// single-row change.
const takeover = !hasRowsRef.current || loadedTabRef.current !== activeTab
if (takeover) setIsLoading(true)
else setIsRefreshing(true)
try {
if (activeTab === 'pending') {
const res = await fetch('/api/pending-operations?status=pending')
// A JSON error body parses fine but carries no data: without this
// check a failed refresh would blank the list and zero the badge
// instead of keeping current rows and surfacing the error toast.
if (!res.ok) throw new Error(`HTTP ${res.status}`)
const json = await res.json()
if (!isCurrent()) return
setOperations(json.data ?? [])
setPendingCount(json.count ?? json.data?.length ?? 0)
loadedTabRef.current = 'pending'
} else {
// The API is single-status per fetch: Historik merges godkända and
// avvisade, newest resolution first.
const [committed, rejected] = await Promise.all([
fetch('/api/pending-operations?status=committed').then((r) => r.json()),
fetch('/api/pending-operations?status=rejected').then((r) => r.json()),
const [committedRes, rejectedRes] = await Promise.all([
fetch('/api/pending-operations?status=committed'),
fetch('/api/pending-operations?status=rejected'),
])
if (!committedRes.ok || !rejectedRes.ok) {
throw new Error(`HTTP ${committedRes.status}/${rejectedRes.status}`)
}
const [committed, rejected] = await Promise.all([committedRes.json(), rejectedRes.json()])
if (!isCurrent()) return
const merged = ([...(committed.data ?? []), ...(rejected.data ?? [])] as PendingOperation[]).sort(
(a, b) => (b.resolved_at ?? b.created_at).localeCompare(a.resolved_at ?? a.created_at),
)
setOperations(merged)
const pc = committed.counts?.pending ?? rejected.counts?.pending
if (typeof pc === 'number') setPendingCount(pc)
loadedTabRef.current = 'history'
}
} catch {
if (!isCurrent()) return
toast({ title: 'Kunde inte ladda operationer', variant: 'destructive' })
}
if (!isCurrent()) return
setIsLoading(false)
setIsRefreshing(false)
}, [activeTab, toast])
useEffect(() => {
@@ -743,6 +784,14 @@ export default function PendingOperationsPage() {
count: tab === 'pending' ? pendingCount ?? 0 : undefined,
}))}
/>
{/* Quiet cue that a background reconcile is running (post-action or
realtime): the list itself never swaps to a spinner for it. */}
{isRefreshing && !isLoading && (
<Loader2
className="h-3.5 w-3.5 animate-spin text-muted-foreground"
aria-label={t('refreshing')}
/>
)}
<div className="ml-auto">
<ContextPicker
value={sourceFilter}
+24 -5
View File
@@ -117,11 +117,27 @@ export default function SupplierInvoicesPage() {
const openNewInvoice = () => router.push('/supplier-invoices?new=1', { scroll: false })
async function fetchInvoices() {
setIsLoading(true)
const res = await fetch('/api/supplier-invoices?status=all')
const { data } = await res.json()
setInvoices(data || [])
setIsLoading(false)
// Skeleton takeover only while nothing is on screen: refetches after an
// action (register, betalfil, approve fallback) reconcile BEHIND the
// rendered table instead of collapsing it to 4 skeleton stubs and
// replaying the entrance animation.
if (invoices.length === 0) setIsLoading(true)
try {
const res = await fetch('/api/supplier-invoices?status=all')
if (!res.ok) throw new Error(`HTTP ${res.status}`)
const { data } = await res.json()
setInvoices(data || [])
} catch {
// Without this, a failed fetch either stuck the skeleton forever or
// silently rendered the empty state as if the invoices were gone.
toast({
title: t('load_failed_title'),
description: t('load_failed_description'),
variant: 'destructive',
})
} finally {
setIsLoading(false)
}
}
// Which invoices already sit in an active (not cancelled) betalfil: feeds
@@ -144,6 +160,9 @@ export default function SupplierInvoicesPage() {
useEffect(() => {
fetchInvoices()
fetchActiveBatchMembership()
// Mount-only fetch (same pattern as /invoices): fetchInvoices reads state
// only to decide skeleton vs background refresh.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [])
// Mirrors the old standalone page's post-create navigation: inbox
+145 -17
View File
@@ -577,10 +577,18 @@ export default function TransactionsPage() {
}
}, [companyId, supabase])
// Computed lists
// Computed lists. Exiting rows (just booked/ignored/deleted) stay IN this
// list on purpose: they render for the 350ms exit window with the .row-exit
// collapse animation instead of popping out the same frame, and the delayed
// state patch (finishBooking et al.) is what actually drops them. Logic
// surfaces that must not act on a leaving row (batch select) exclude
// exitingIds themselves.
const uncategorizedTransactions = useMemo(
() => transactions
.filter((t) => t.is_business === null && !t.is_ignored && !exitingIds.has(t.id))
// The exitingIds clause pins a leaving row even when a realtime-echo
// refetch replaces its state with the booked shape mid-window: the
// animation still finishes instead of being cut to a jump.
.filter((t) => (t.is_business === null && !t.is_ignored) || exitingIds.has(t.id))
.sort((a, b) => {
const aHasMatch = a.potential_invoice || a.potential_supplier_invoice ? 1 : 0
const bHasMatch = b.potential_invoice || b.potential_supplier_invoice ? 1 : 0
@@ -656,8 +664,9 @@ export default function TransactionsPage() {
if (sourceFilter === 'all' || sourceFilter === 'skatteverket') {
// Inbox only shows SKV rows that need action (no verifikat yet).
for (const r of skvRows) {
if (r.journal_entry_id) continue
if (exitingIds.has(r.id)) continue
// Exiting rows stay rendered for the exit animation, even if a
// refetch already gave them a journal_entry_id mid-window (see above).
if (r.journal_entry_id && !exitingIds.has(r.id)) continue
// SKV rows live client-side only, so the period filter applies here.
if (!isWithinBounds(r.transaktionsdatum, periodBounds)) continue
if (
@@ -788,8 +797,13 @@ export default function TransactionsPage() {
// Rows the bulkbar's "Markera alla" can select: the visible bank rows
// (they feed the /api/transactions/* batch handlers) ...
const selectableInboxIds = useMemo(
() => inboxItems.filter((item) => item.source === 'bank').map((item) => item.data.id),
[inboxItems],
() =>
inboxItems
// Rows mid-exit still render (for the animation) but must not be
// selectable: they are already booked/deleted server-side.
.filter((item) => item.source === 'bank' && !exitingIds.has(item.data.id))
.map((item) => item.data.id),
[exitingIds, inboxItems],
)
// ... plus the visible skattekonto rows whose booking is deterministic
@@ -798,9 +812,14 @@ export default function TransactionsPage() {
const selectableSkvIds = useMemo(
() =>
inboxItems
.filter((item) => item.source === 'skatteverket' && isSkvBulkEligible(item.data))
.filter(
(item) =>
item.source === 'skatteverket' &&
isSkvBulkEligible(item.data) &&
!exitingIds.has(item.data.id),
)
.map((item) => item.data.id),
[inboxItems],
[exitingIds, inboxItems],
)
const skvSelectedRows = useMemo(
@@ -907,9 +926,20 @@ export default function TransactionsPage() {
}
}, [])
const fetchTransactions = useCallback(async (showLoading = false, includeSkvRows = false) => {
const fetchTransactions = useCallback(async (
showLoading = false,
includeSkvRows = false,
// Background refreshes (realtime echo) re-fetch the window the user has
// already paged through instead of resetting to the first PAGE_SIZE rows:
// an action after "Visa fler" must not collapse the loaded pages and
// yank the scroll position.
preserveWindow = false,
) => {
if (!companyId) return
const generation = ++fetchGenerationRef.current
const windowSize = preserveWindow
? Math.max(pagedCountRef.current, PAGE_SIZE)
: PAGE_SIZE
if (showLoading) setIsLoading(true)
if (includeSkvRows) void loadSkvRows()
try {
@@ -934,7 +964,7 @@ export default function TransactionsPage() {
windowQuery
.order('date', { ascending: false })
.order('id', { ascending: true })
.limit(PAGE_SIZE),
.range(0, windowSize - 1),
supabase
.from('transactions')
.select('*', { count: 'exact', head: true })
@@ -1004,8 +1034,8 @@ export default function TransactionsPage() {
setTransactions(transactionsWithInvoices)
setTotalUncategorizedCount(uncatCount ?? 0)
pagedCountRef.current = rows.length
setPagedThroughDate(rows.length >= PAGE_SIZE ? rows[rows.length - 1].date : null)
setHasMore(rows.length >= PAGE_SIZE)
setPagedThroughDate(rows.length >= windowSize ? rows[rows.length - 1].date : null)
setHasMore(rows.length >= windowSize)
} finally {
// Only the newest request may touch the skeleton: a stale one must not
@@ -1027,7 +1057,7 @@ export default function TransactionsPage() {
try {
do {
refreshTransactionsQueuedRef.current = false
await fetchTransactions(false, false)
await fetchTransactions(false, false, true)
} while (refreshTransactionsQueuedRef.current)
} finally {
refreshTransactionsInFlightRef.current = false
@@ -1235,10 +1265,65 @@ export default function TransactionsPage() {
}
}
// Fetch the initial page. Entity type is already available from CompanyContext.
// The initial fetch waits for FyPicker to finish restoring the persisted
// period scope (onReady fires after its restore onChange). Fetching at
// mount used to race the restore: an unscoped fetch painted the list, the
// restore re-created fetchTransactions and re-ran this effect with a second
// skeleton takeover, so every visit flashed list → skeleton → list.
const [fyReady, setFyReady] = useState(false)
const handleFyReady = useCallback(() => setFyReady(true), [])
// Whether anything is on screen, readable from the effect without making
// row state a dependency (which would refetch on every row patch).
const hasRowsRef = useRef(false)
useEffect(() => {
void fetchTransactions(true, true)
}, [fetchTransactions])
hasRowsRef.current = transactions.length > 0 || skvRows.length > 0
}, [transactions, skvRows])
const lastFetchCompanyRef = useRef<string | null>(null)
// Subtle "refreshing" cue next to the period chip while a scope change
// refetches behind the rendered list. Sequence-guarded so overlapping
// scope changes can't clear the cue early.
const [isScopeRefreshing, setIsScopeRefreshing] = useState(false)
const scopeRefreshSeqRef = useRef(0)
// Fiscal scope must not survive a company switch: periodBounds from the
// previous company would scope the first fetch for the new one, and a
// non-null fyPeriodId makes FyPicker skip its persisted-scope restore
// (it only restores when value === null). Resetting during render (React's
// adjust-state-on-prop-change pattern) rather than in an effect guarantees
// FyPicker's restore effect observes the cleared value: its effect captures
// `value` at commit time, and child effects run before a parent effect
// could reset it. fyReady = false holds the fetch effect until the new
// company's restore completes via onReady.
const [scopeCompanyId, setScopeCompanyId] = useState<string | null>(companyId)
if (scopeCompanyId !== companyId) {
setScopeCompanyId(companyId)
setFyReady(false)
setFyPeriodId(null)
setFyPeriod(null)
// A scope refresh in flight belongs to the old company: invalidate its
// finally-guard and drop the cue.
scopeRefreshSeqRef.current++
setIsScopeRefreshing(false)
}
// Fetch the page whenever the scope changes (mount, company switch, period
// change). The skeleton takeover is reserved for an empty or foreign list:
// a period change refetches BEHIND the rendered rows, whose client-side
// isWithinBounds filters already show the new scope correctly.
useEffect(() => {
if (!fyReady) return
const companySwitched = lastFetchCompanyRef.current !== companyId
lastFetchCompanyRef.current = companyId
if (companySwitched || !hasRowsRef.current) {
void fetchTransactions(true, true)
return
}
const seq = ++scopeRefreshSeqRef.current
setIsScopeRefreshing(true)
void fetchTransactions(false, true).finally(() => {
if (scopeRefreshSeqRef.current === seq) setIsScopeRefreshing(false)
})
}, [companyId, fetchTransactions, fyReady])
useEffect(() => {
if (!companyId) return
@@ -2349,9 +2434,13 @@ export default function TransactionsPage() {
if (!ok) return
try {
// Row-level pending state while the DELETE runs (the card renders a
// spinner for processingId): confirm-to-completion used to be dead air.
setProcessingId(id)
const response = await fetch(`/api/transactions/${id}`, { method: 'DELETE' })
if (!response.ok) {
const result = await response.json()
setProcessingId((prev) => (prev === id ? null : prev))
toast({
title: 'Kunde inte ta bort',
description: getErrorMessage(result, { context: 'transaction' }),
@@ -2359,9 +2448,36 @@ export default function TransactionsPage() {
})
return
}
setTransactions((prev) => prev.filter((t) => t.id !== id))
// Same exit path as booking/ignore: the row animates out over the
// 350ms window instead of vanishing with a hard jump, then the delayed
// filter actually removes it.
setExitingIds((prev) => new Set(prev).add(id))
// Drop the row from the batch selection immediately: leaving it there
// keeps the bulk bar counting (and acting on) a row that no longer
// exists once the timer removes it.
setSelectedIds((prev) => {
if (!prev.has(id)) return prev
const next = new Set(prev)
next.delete(id)
return next
})
// The realtime echo is not guaranteed for DELETE on a filtered
// subscription, so the pending badge must decrement locally.
if (transaction.is_business === null && !transaction.is_ignored) {
setTotalUncategorizedCount((prev) => Math.max(0, (prev ?? 1) - 1))
}
toast({ title: t('deleted_title'), description: t('deleted_description') })
setTimeout(() => {
setTransactions((prev) => prev.filter((t) => t.id !== id))
setExitingIds((prev) => {
const next = new Set(prev)
next.delete(id)
return next
})
setProcessingId((prev) => (prev === id ? null : prev))
}, 350)
} catch {
setProcessingId((prev) => (prev === id ? null : prev))
toast({
title: 'Kunde inte ta bort',
description: t('delete_failed_description'),
@@ -3213,9 +3329,18 @@ export default function TransactionsPage() {
and on a brutet rakenskapsar they were not momsdeklaration
quarters, so they misled more than they scoped. */}
<div className="ml-auto flex flex-wrap items-center justify-end gap-2">
{/* Quiet cue that a scope change is reconciling behind the rendered
list (the list itself never swaps to a skeleton for it). */}
{isScopeRefreshing && (
<Loader2
className="h-3.5 w-3.5 animate-spin text-muted-foreground"
aria-label={t('refreshing')}
/>
)}
<FyPicker
value={fyPeriodId}
onChange={handlePeriodChange}
onReady={handleFyReady}
storageKeyPrefix={PERIOD_FILTER_STORAGE_PREFIX}
/>
{sourceItems.length > 1 && (
@@ -3376,6 +3501,7 @@ export default function TransactionsPage() {
key={`bank-${item.data.id}`}
transaction={item.data}
skvCounterpartDate={bankToSkvHints.get(item.data.id)}
isExiting={exitingIds.has(item.data.id)}
processingId={processingId}
isSelected={selectedIds.has(item.data.id)}
isExpanded={expandedTxId === item.data.id}
@@ -3404,6 +3530,7 @@ export default function TransactionsPage() {
row={item.data}
matchSuggestion={item.data.match_suggestion}
bookingSuggestion={item.data.booking_suggestion}
isExiting={exitingIds.has(item.data.id)}
processing={skvBulkSubmitting}
selectable={isSkvBulkEligible(item.data)}
isSelected={skvSelectedIds.has(item.data.id)}
@@ -3443,6 +3570,7 @@ export default function TransactionsPage() {
<TransactionHistoryList
transactions={historyTransactions}
skvRows={skvRowsInScope}
exitingIds={exitingIds}
searchTerm={searchTerm}
sourceFilter={sourceFilter}
jeUnderlagStatus={jeUnderlagStatus}
+51
View File
@@ -616,6 +616,57 @@ summary,
.stagger-enter > *:nth-child(9) { animation-delay: 320ms; }
.stagger-enter > *:nth-child(n+10) { animation-delay: 360ms; }
/* Row exit (booking/ignore/delete in dry-table lists). The page keeps the
row rendered for a 350ms window (see finishBooking on /transactions) and
marks it .row-exit; the row fades fast, then the space closes by
transitioning the cell paddings/line metrics to zero. Table rows cannot
animate height directly, so fixed-height cell content (action pill,
badges) carries .row-collapsible: a numeric max-height at rest, so the
transition to 0 interpolates instead of snapping. The stagger-enter
animation on the same row has long finished; `both` fill keeps it inert.
All timings stay inside the 350ms removal window.
pointer-events only guards the pointer: the exiting row also carries the
`inert` attribute (set by the row components) so keyboard focus and
activation are blocked during the window too. */
.row-exit {
pointer-events: none;
}
.row-exit > td {
opacity: 0;
padding-top: 0;
padding-bottom: 0;
line-height: 0;
font-size: 0;
border-bottom-width: 0;
transition:
opacity 160ms var(--ease-out),
padding-top 240ms var(--ease-emphasized) 60ms,
padding-bottom 240ms var(--ease-emphasized) 60ms,
line-height 240ms var(--ease-emphasized) 60ms,
font-size 240ms var(--ease-emphasized) 60ms,
border-bottom-width 240ms var(--ease-emphasized) 60ms;
}
.row-collapsible {
/* Numeric rest value so the exit transition has something to interpolate
from; comfortably above the tallest row content (h-7 action pill). */
max-height: 2.5rem;
}
.row-exit .row-collapsible {
max-height: 0;
overflow: hidden;
transform: translateX(0.5rem);
transition:
max-height 240ms var(--ease-emphasized) 60ms,
transform 240ms var(--ease-out);
}
/* Reduced motion: the row is removed instantly; the delayed state cleanup
in the page is invisible. */
@media (prefers-reduced-motion: reduce) {
.row-exit {
display: none;
}
}
/* Utility animations */
.animate-fade-in {
animation: fadeIn var(--duration-base) var(--ease-out);