Skip to content

Commit e5ee9a8

Browse files
bjcoombsbjcoombs
authored
feat: Align accounts pages to canonical PageShell/PageHeader pattern (#1615)
* feat: Align accounts pages to canonical PageShell/PageHeader pattern Replace ad-hoc div wrappers with PageShell, PageHeader, ErrorState, and DetailSkeleton shared components on the accounts list and detail pages. Update account-detail test to match new detail-skeleton testid. * fix: Address review feedback on accounts detail page - Wrap DetailSkeleton loading states in PageShell + Breadcrumbs to prevent layout shift - Fix ErrorState retry button visibility: hide during refetch instead of disabling - Add flex-wrap to AccountActions container to prevent overflow on narrow screens --------- Co-authored-by: Ben Coombs <bjcoombs@users.noreply.github.com>
1 parent 173b288 commit e5ee9a8

3 files changed

Lines changed: 136 additions & 151 deletions

File tree

‎frontend/src/features/accounts/pages/[accountId].tsx‎

Lines changed: 105 additions & 124 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { Button } from '@/components/ui/button'
66
import { StatusBadge } from '@/shared/status-badge'
77
import { TimeDisplay } from '@/shared/time-display'
88
import { MoneyDisplay } from '@/shared/money-display'
9-
import { AuditTrail, EntityLink, Breadcrumbs } from '@/shared'
9+
import { AuditTrail, EntityLink, Breadcrumbs, PageShell, PageHeader, ErrorState, DetailSkeleton } from '@/shared'
1010
import { useAccountResolver } from '@/shared/use-account-resolver'
1111
import { DepositDialog } from './deposit-dialog'
1212
import { WithdrawDialog } from './withdraw-dialog'
@@ -17,26 +17,6 @@ import { CreateValuationFeatureDialog } from '@/features/reference-data/componen
1717
import type { AccountStatus as AccountStatusType } from './types'
1818
import { useAccountDetail, useAccountPostings, useAccountLiens } from '../hooks'
1919

20-
// ---------------------------------------------------------------------------
21-
// Skeleton
22-
// ---------------------------------------------------------------------------
23-
24-
function AccountDetailSkeleton() {
25-
return (
26-
<div data-testid="account-detail-skeleton" className="animate-pulse space-y-6 p-6">
27-
<div className="flex items-center gap-3">
28-
<div className="h-4 w-24 rounded bg-muted" />
29-
</div>
30-
<div className="h-8 w-64 rounded bg-muted" />
31-
<div className="grid grid-cols-2 gap-4 md:grid-cols-4">
32-
{Array.from({ length: 4 }).map((_, i) => (
33-
<div key={i} className="h-20 rounded bg-muted" />
34-
))}
35-
</div>
36-
<div className="h-64 rounded bg-muted" />
37-
</div>
38-
)
39-
}
4020

4121
// ---------------------------------------------------------------------------
4222
// Not found
@@ -87,7 +67,7 @@ function AccountActions({ status, accountId, currency }: AccountActionsProps) {
8767

8868
return (
8969
<>
90-
<div className="flex gap-2">
70+
<div className="flex flex-wrap justify-end gap-2">
9171
{status === 'ACTIVE' && (
9272
<>
9373
<Button variant="outline" size="sm" onClick={() => setDepositOpen(true)}>
@@ -358,13 +338,23 @@ export function AccountDetailPage() {
358338
)
359339

360340
if (isLoading) {
361-
return <AccountDetailSkeleton />
341+
return (
342+
<PageShell>
343+
<Breadcrumbs items={[{ label: 'Accounts', href: '/accounts' }]} />
344+
<DetailSkeleton />
345+
</PageShell>
346+
)
362347
}
363348

364349
// Current-account returned 404 — check if it's an internal account
365350
if (account === null) {
366351
if (isResolving) {
367-
return <AccountDetailSkeleton />
352+
return (
353+
<PageShell>
354+
<Breadcrumbs items={[{ label: 'Accounts', href: '/accounts' }]} />
355+
<DetailSkeleton />
356+
</PageShell>
357+
)
368358
}
369359
if (resolved?.type === 'internal') {
370360
return <Navigate to={`/internal-accounts/${encodeURIComponent(resolved.accountId)}`} replace />
@@ -374,57 +364,50 @@ export function AccountDetailPage() {
374364

375365
if (isError || account === undefined) {
376366
return (
377-
<div data-testid="account-error" className="p-6">
378-
<Breadcrumbs items={[{ label: 'Accounts', href: '/accounts' }, { label: accountId ?? 'Error' }]} />
379-
<div className="mt-8 text-center">
380-
<h2 className="text-xl font-semibold">Failed to load account</h2>
381-
<p className="mt-2 text-sm text-muted-foreground">
382-
There was a problem loading this account. Please try again.
383-
</p>
384-
<Button
385-
variant="outline"
386-
size="sm"
387-
className="mt-4"
388-
disabled={isFetching}
389-
onClick={() => void refetch()}
390-
>
391-
{isFetching ? 'Retrying…' : 'Retry'}
392-
</Button>
393-
</div>
367+
<div data-testid="account-error">
368+
<PageShell>
369+
<Breadcrumbs items={[{ label: 'Accounts', href: '/accounts' }, { label: accountId ?? 'Error' }]} />
370+
<ErrorState
371+
title="Failed to load account"
372+
message="There was a problem loading this account. Please try again."
373+
onRetry={isFetching ? undefined : () => void refetch()}
374+
retryLabel="Retry"
375+
/>
376+
</PageShell>
394377
</div>
395378
)
396379
}
397380

398381
return (
399-
<div className="p-6">
400-
{/* Breadcrumb navigation */}
382+
<PageShell>
401383
<Breadcrumbs
402384
items={[
403385
{ label: 'Accounts', href: '/accounts' },
404386
{ label: account.accountId },
405387
]}
406388
/>
407389

408-
{/* Page header */}
409-
<div className="mt-4 flex flex-wrap items-start justify-between gap-4">
410-
<div>
411-
<div className="flex items-center gap-3">
412-
<h1 className="text-2xl font-semibold">{account.accountId}</h1>
413-
<StatusBadge status={account.status} />
414-
</div>
390+
<div>
391+
<PageHeader
392+
title={account.accountId}
393+
actions={
394+
<AccountActions
395+
status={account.status}
396+
accountId={account.accountId}
397+
currency={account.instrumentCode}
398+
/>
399+
}
400+
/>
401+
<div className="mt-1 flex items-center gap-3">
402+
<StatusBadge status={account.status} />
415403
{account.externalReference && (
416-
<p className="mt-1 font-mono text-sm text-muted-foreground">{account.externalReference}</p>
404+
<p className="font-mono text-sm text-muted-foreground">{account.externalReference}</p>
417405
)}
418406
</div>
419-
<AccountActions
420-
status={account.status}
421-
accountId={account.accountId}
422-
currency={account.instrumentCode}
423-
/>
424407
</div>
425408

426409
{/* Summary fields */}
427-
<Card className="mt-6">
410+
<Card>
428411
<CardContent>
429412
<dl className="grid grid-cols-2 gap-4 pt-2 md:grid-cols-4">
430413
<DetailField label="Instrument">{account.instrumentCode}</DetailField>
@@ -444,73 +427,71 @@ export function AccountDetailPage() {
444427
</Card>
445428

446429
{/* Tabs */}
447-
<div className="mt-6">
448-
<Tabs defaultValue="overview">
449-
<TabsList>
450-
<TabsTrigger value="overview">Overview</TabsTrigger>
451-
<TabsTrigger value="transactions">Transactions</TabsTrigger>
452-
<TabsTrigger value="liens">Liens</TabsTrigger>
453-
<TabsTrigger value="audit">Audit Trail</TabsTrigger>
454-
</TabsList>
455-
456-
<TabsContent value="overview" className="mt-4">
457-
<Card>
458-
<CardHeader>
459-
<CardTitle>Account Details</CardTitle>
460-
</CardHeader>
461-
<CardContent>
462-
<dl className="grid grid-cols-1 gap-4 sm:grid-cols-2">
463-
<DetailField label="Account ID">{account.accountId}</DetailField>
464-
<DetailField label="External Reference">{account.externalReference || '—'}</DetailField>
465-
<DetailField label="Status">
466-
<StatusBadge status={account.status} />
467-
</DetailField>
468-
<DetailField label="Instrument">{account.instrumentCode}</DetailField>
469-
<DetailField label="Available Balance">
470-
{account.availableBalance ?? '—'}
430+
<Tabs defaultValue="overview">
431+
<TabsList>
432+
<TabsTrigger value="overview">Overview</TabsTrigger>
433+
<TabsTrigger value="transactions">Transactions</TabsTrigger>
434+
<TabsTrigger value="liens">Liens</TabsTrigger>
435+
<TabsTrigger value="audit">Audit Trail</TabsTrigger>
436+
</TabsList>
437+
438+
<TabsContent value="overview" className="mt-4">
439+
<Card>
440+
<CardHeader>
441+
<CardTitle>Account Details</CardTitle>
442+
</CardHeader>
443+
<CardContent>
444+
<dl className="grid grid-cols-1 gap-4 sm:grid-cols-2">
445+
<DetailField label="Account ID">{account.accountId}</DetailField>
446+
<DetailField label="External Reference">{account.externalReference || '—'}</DetailField>
447+
<DetailField label="Status">
448+
<StatusBadge status={account.status} />
449+
</DetailField>
450+
<DetailField label="Instrument">{account.instrumentCode}</DetailField>
451+
<DetailField label="Available Balance">
452+
{account.availableBalance ?? '—'}
453+
</DetailField>
454+
<DetailField label="Reserved Balance">
455+
{account.reservedBalance ?? '—'}
456+
</DetailField>
457+
{account.name && (
458+
<DetailField label="Name">{account.name}</DetailField>
459+
)}
460+
{account.partyId && (
461+
<DetailField label="Party ID">
462+
<EntityLink type="party" id={account.partyId} />
471463
</DetailField>
472-
<DetailField label="Reserved Balance">
473-
{account.reservedBalance ?? '—'}
474-
</DetailField>
475-
{account.name && (
476-
<DetailField label="Name">{account.name}</DetailField>
477-
)}
478-
{account.partyId && (
479-
<DetailField label="Party ID">
480-
<EntityLink type="party" id={account.partyId} />
481-
</DetailField>
482-
)}
483-
<DetailField label="Created">
484-
<TimeDisplay timestamp={account.createdAt} format="both" />
485-
</DetailField>
486-
<DetailField label="Last Updated">
487-
<TimeDisplay timestamp={account.updatedAt} format="both" />
488-
</DetailField>
489-
</dl>
490-
</CardContent>
491-
</Card>
492-
</TabsContent>
493-
494-
<TabsContent value="transactions" className="mt-4">
495-
<AccountTransactions accountId={account.accountId} instrumentCode={account.instrumentCode} />
496-
</TabsContent>
497-
498-
<TabsContent value="liens" className="mt-4">
499-
<AccountLiens accountId={account.accountId} instrumentCode={account.instrumentCode} />
500-
</TabsContent>
501-
502-
<TabsContent value="audit" className="mt-4">
503-
<Card>
504-
<CardHeader>
505-
<CardTitle>Audit Trail</CardTitle>
506-
</CardHeader>
507-
<CardContent>
508-
<AuditTrail entityType="CurrentAccount" entityId={account.accountId} />
509-
</CardContent>
510-
</Card>
511-
</TabsContent>
512-
</Tabs>
513-
</div>
514-
</div>
464+
)}
465+
<DetailField label="Created">
466+
<TimeDisplay timestamp={account.createdAt} format="both" />
467+
</DetailField>
468+
<DetailField label="Last Updated">
469+
<TimeDisplay timestamp={account.updatedAt} format="both" />
470+
</DetailField>
471+
</dl>
472+
</CardContent>
473+
</Card>
474+
</TabsContent>
475+
476+
<TabsContent value="transactions" className="mt-4">
477+
<AccountTransactions accountId={account.accountId} instrumentCode={account.instrumentCode} />
478+
</TabsContent>
479+
480+
<TabsContent value="liens" className="mt-4">
481+
<AccountLiens accountId={account.accountId} instrumentCode={account.instrumentCode} />
482+
</TabsContent>
483+
484+
<TabsContent value="audit" className="mt-4">
485+
<Card>
486+
<CardHeader>
487+
<CardTitle>Audit Trail</CardTitle>
488+
</CardHeader>
489+
<CardContent>
490+
<AuditTrail entityType="CurrentAccount" entityId={account.accountId} />
491+
</CardContent>
492+
</Card>
493+
</TabsContent>
494+
</Tabs>
495+
</PageShell>
515496
)
516497
}

‎frontend/src/features/accounts/pages/account-detail.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ describe('AccountDetailPage - loading and error states', () => {
5555

5656
renderDetailPage()
5757

58-
expect(screen.getByTestId('account-detail-skeleton')).toBeInTheDocument()
58+
expect(screen.getByTestId('detail-skeleton')).toBeInTheDocument()
5959
})
6060

6161
it('shows error state for failed account fetch', async () => {

‎frontend/src/features/accounts/pages/index.tsx‎

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ import type { ColumnDef } from '@tanstack/react-table'
44
import { DataTable } from '@/shared/data-table'
55
import { StatusBadge } from '@/shared/status-badge'
66
import { TimeDisplay } from '@/shared/time-display'
7-
import { EntityLink } from '@/shared'
7+
import { EntityLink, PageShell, PageHeader } from '@/shared'
8+
import { Card } from '@/components/ui/card'
89
import { Button } from '@/components/ui/button'
910
import { AccountStatus } from '@/api/gen/meridian/current_account/v1/current_account_pb'
1011
import { CreateAccountDialog } from './create-account-dialog'
@@ -55,37 +56,40 @@ export function AccountsPage() {
5556
]
5657

5758
return (
58-
<div className="p-6">
59-
<div className="mb-6 flex items-center justify-between">
60-
<h1 className="text-2xl font-semibold">Accounts</h1>
61-
<Button onClick={() => setCreateOpen(true)}>Create Account</Button>
62-
</div>
63-
64-
<DataTable
65-
queryKey={queryKey}
66-
queryFn={queryFn}
67-
columns={columns}
68-
filters={[
69-
{
70-
field: 'status',
71-
label: 'Status',
72-
type: 'select',
73-
options: STATUS_OPTIONS,
74-
},
75-
{
76-
field: 'externalReference',
77-
label: 'External Ref',
78-
type: 'text',
79-
},
80-
]}
81-
onRowClick={(row) => navigate(`/accounts/${row.accountId}`)}
59+
<PageShell>
60+
<PageHeader
61+
title="Accounts"
62+
description="Manage current accounts and their balances."
63+
actions={<Button onClick={() => setCreateOpen(true)}>Create Account</Button>}
8264
/>
8365

66+
<Card>
67+
<DataTable
68+
queryKey={queryKey}
69+
queryFn={queryFn}
70+
columns={columns}
71+
filters={[
72+
{
73+
field: 'status',
74+
label: 'Status',
75+
type: 'select',
76+
options: STATUS_OPTIONS,
77+
},
78+
{
79+
field: 'externalReference',
80+
label: 'External Ref',
81+
type: 'text',
82+
},
83+
]}
84+
onRowClick={(row) => navigate(`/accounts/${row.accountId}`)}
85+
/>
86+
</Card>
87+
8488
<CreateAccountDialog
8589
open={createOpen}
8690
onOpenChange={setCreateOpen}
8791
onCreated={(accountId) => navigate(`/accounts/${accountId}`)}
8892
/>
89-
</div>
93+
</PageShell>
9094
)
9195
}

0 commit comments

Comments
 (0)