Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 122 additions & 0 deletions docs/swr-loading-migration.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
# Moving data loading to SWR

## Where we are

The frontend fetches data by hand. Each screen has a `useEffect` that calls an
API function, stores the result in `useState`, and tracks its own `loading`
flag. The guards look like this:

```ts
useEffect(() => {
if (authenticated && token && activeWorkspaceId) {
// fetch, set state, set loading false
}
}, [authenticated, token, activeWorkspaceId]);
```

It works, but every screen repeats the same wiring, and "refresh on workspace
switch" is done manually. There's no shared cache, so two screens asking for the
same thing fetch it twice.

The loading UI is already standard: one `CenteredProgress` component (a centered
spinner, `role="status"`, screen-reader label, no visible text). Tables take a
`loading` prop. None of that changes here — SWR just decides when `loading` is
true.

## Where we're going

Use [SWR](https://swr.vercel.app/) to do the fetching. We picked SWR over
TanStack Query because it's smaller and its conditional-key feature lines up
with how we gate on auth and workspace.

The key idea: a request key that's `null` means "not ready, don't fetch." So the
auth/workspace guard becomes part of the key instead of an `if`:

```ts
const key = token && workspaceId ? ["forms", workspaceId] : null;
const { data, isLoading } = useSWR(key, () => getSobaForms(token, workspaceId));
```

When `workspaceId` changes, the key changes, and SWR refetches on its own. That
removes the manual refetch effects.

## Plan

Do it one screen at a time. Each phase is shippable on its own.

### Phase B — set up SWR, migrate one screen

1. Add the `swr` dependency.
2. Wrap the app in `<SWRConfig>` (in `AppProviders`) with the default options we
want (e.g. don't revalidate on window focus unless we decide to).
3. Write a small `useAuthedSWR` hook that reads the token and `activeWorkspaceId`
from the store, builds the key (or `null`), and calls the API function. It
returns SWR's `data`, `isLoading`, `isValidating`, `error`, and `mutate`.
4. Migrate `FormList` to it. Delete the `useEffect`, the `useState`, and the
guard. Feed `isLoading` to the table's `loading` prop.
5. Update the `FormList` test to render inside an `<SWRConfig>` that resets the
cache (see Testing below).

After this, FormList is the reference. The rest copy it.

### Phase C — migrate the other reads

One per change, same shape as FormList:

- `SubmissionList`
- `SubmissionView`
- `Header` (workspaces, current user)
- the designer's schema/version loads

Each migration deletes a `useEffect` and its guard. Workspace-switch refetch
effects go away because the key handles it.

### Phase D — writes and Redux cleanup

- Move the create/save/publish calls to `useSWRMutation`, and call `mutate` to
refresh the affected list after a write.
- Once a slice is only holding fetched data, delete it. `currentUserSlice` and
the list part of `workspaceSlice` are the candidates.
- Keep in Redux what's actually client state: the Keycloak token and auth flags,
the selected `activeWorkspaceId`, and notifications.

### Phase E — optional

Add Next's `loading.tsx` files for route transitions, reusing `CenteredProgress`.
Only worth it if the route-change flash bothers us.

## How loading maps

| State | What the user sees |
| --------------------------------------------------------- | ------------------------------------------------- |
| `isLoading` (first load, no data yet) | `CenteredProgress` / table `loading` |
| `isValidating` (background refresh, data already showing) | leave the data up; optionally a small inline hint |
| `error` | the existing inline alert / empty state |

Don't show the full spinner on a background refresh — that's the point of
splitting `isLoading` from `isValidating`.

## Testing

SWR caches across renders, so tests need a fresh cache each time or they leak
into each other. Wrap the component under test:

```tsx
import { SWRConfig } from "swr";

render(
<SWRConfig value={{ provider: () => new Map(), dedupingInterval: 0 }}>
<FormList />
</SWRConfig>,
);
```

The fetch functions are already mocked the same way they are today, so the
assertions don't change much — you're just driving them through SWR.

## Notes

- These are client-side reads, same as now. SWR runs in the browser; there's no
server-rendering change.
- Don't touch the Form.io engine. This is about how SOBA loads its own data.
- Token stays in Redux. The hook reads it for the key; SWR never owns auth.
7 changes: 6 additions & 1 deletion frontend/app/[lang]/designer/[...formId]/page.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { getDictionary, hasLocale, Locale } from '../../dictionaries';
import FormDesignerLoader from '@/src/features/designer/ui/FormDesignerLoader';
import { DsPageHeading } from '@/app/ui/DsPageHeading';
import { notFound } from 'next/navigation';
import { loadFeaturesMeta } from '@/src/shared/config/featuresMeta';
import { createIsFeatureAllowed, FEATURE_CODES } from '@/src/shared/featureFlags/flags';
Expand Down Expand Up @@ -28,10 +29,14 @@ export default async function Page({ params }: PageProps) {
notFound();
}

const { formId } = await params;
const { lang, formId } = await params;
const dict = await getDictionary((hasLocale(lang) ? lang : 'en') as Locale);

return (
<section className="p-4" aria-labelledby="designer-heading">
<DsPageHeading id="designer-heading" className="visually-hidden">
{dict.general.formDesigner}
</DsPageHeading>
<FormDesignerLoader id={formId} />
</section>
);
Expand Down
8 changes: 7 additions & 1 deletion frontend/app/[lang]/designer/page.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { getDictionary, hasLocale, Locale } from '../dictionaries';
import FormDesignerLoader from '@/src/features/designer/ui/FormDesignerLoader';
import { DsPageHeading } from '@/app/ui/DsPageHeading';
import { notFound } from 'next/navigation';
import { loadFeaturesMeta } from '@/src/shared/config/featuresMeta';
import { createIsFeatureAllowed, FEATURE_CODES } from '@/src/shared/featureFlags/flags';
Expand All @@ -20,16 +21,21 @@ export async function generateMetadata({ params }: PageProps) {
};
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
export default async function Page({ params }: PageProps) {
const featuresMeta = await loadFeaturesMeta();
const isFeatureAllowed = createIsFeatureAllowed(featuresMeta);
if (!isFeatureAllowed(FEATURE_CODES.DESIGN_MODE)) {
notFound();
}

const { lang } = await params;
const dict = await getDictionary((hasLocale(lang) ? lang : 'en') as Locale);

return (
<section className="p-4" aria-labelledby="designer-heading">
<DsPageHeading id="designer-heading" className="visually-hidden">
{dict.general.formDesigner}
</DsPageHeading>
<FormDesignerLoader id={[]} />
</section>
);
Expand Down
12 changes: 10 additions & 2 deletions frontend/app/[lang]/feedback/page.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { getDictionary, resolveLocale } from '../dictionaries';
import { DsPageHeading } from '@/app/ui/DsPageHeading';

type PageProps = {
params: Promise<{ lang: string }>;
Expand All @@ -8,7 +9,7 @@ export async function generateMetadata({ params }: PageProps) {
const locale = resolveLocale(param.lang);
const dict = await getDictionary(locale);
return {
title: `${dict.general.title}`,
title: `${dict.general.feedback} | ${dict.general.title}`,
description: dict.general.description,
};
}
Expand All @@ -18,5 +19,12 @@ export default async function Page({ params }: PageProps) {
const locale = resolveLocale(param.lang);
const dict = await getDictionary(locale);

return <div>{dict.general.feedback}</div>;
return (
<section className="p-4" aria-labelledby="feedback-heading">
<DsPageHeading id="feedback-heading">{dict.general.feedback}</DsPageHeading>
<p className="mt-3 text-muted" data-testid="feedback-coming-soon">
{dict.general.comingSoon}
</p>
</section>
);
}
4 changes: 2 additions & 2 deletions frontend/app/[lang]/forms/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ export default async function Page({ params }: PageProps) {
const locale = resolveLocale(param.lang);
return (
<AuthRedirect to={`/${locale}`} ifLogged={false}>
<div>
<section aria-labelledby="forms-heading">
<FormList
designModeEnabled={isFeatureAllowed(FEATURE_CODES.DESIGN_MODE)}
submitModeEnabled={isFeatureAllowed(FEATURE_CODES.SUBMIT_MODE)}
/>
</div>
</section>
</AuthRedirect>
);
}
12 changes: 10 additions & 2 deletions frontend/app/[lang]/help/page.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { getDictionary, resolveLocale } from '../dictionaries';
import { DsPageHeading } from '@/app/ui/DsPageHeading';

type PageProps = {
params: Promise<{ lang: string }>;
Expand All @@ -8,7 +9,7 @@ export async function generateMetadata({ params }: PageProps) {
const locale = resolveLocale(param.lang);
const dict = await getDictionary(locale);
return {
title: `${dict.general.title}`,
title: `${dict.general.help} | ${dict.general.title}`,
description: dict.general.description,
};
}
Expand All @@ -18,5 +19,12 @@ export default async function Page({ params }: PageProps) {
const locale = resolveLocale(param.lang);
const dict = await getDictionary(locale);

return <div>{dict.general.help}</div>;
return (
<section className="p-4" aria-labelledby="help-heading">
<DsPageHeading id="help-heading">{dict.general.help}</DsPageHeading>
<p className="mt-3 text-muted" data-testid="help-coming-soon">
{dict.general.comingSoon}
</p>
</section>
);
}
33 changes: 0 additions & 33 deletions frontend/app/[lang]/meta/page.tsx

This file was deleted.

7 changes: 5 additions & 2 deletions frontend/app/[lang]/submission/[submissionId]/page.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { SubmissionView } from '@/src/features/submit-mode/ui/SubmissionView';
import { DsPageHeading } from '@/app/ui/DsPageHeading';
import { getDictionary, hasLocale, Locale } from '../../dictionaries';
import { notFound } from 'next/navigation';
import { loadFeaturesMeta } from '@/src/shared/config/featuresMeta';
Expand All @@ -15,7 +16,7 @@ export async function generateMetadata({ params }: PageProps) {
}
const dict = await getDictionary(param.lang as Locale);
return {
title: `Submission | ${dict.general.title}`,
title: `${dict.submission.pageTitle} | ${dict.general.title}`,
description: dict.general.description,
};
}
Expand All @@ -27,9 +28,11 @@ export default async function Page({ params }: PageProps) {
notFound();
}

await params;
const { lang } = await params;
const dict = await getDictionary((hasLocale(lang) ? lang : 'en') as Locale);
return (
<section className="p-4" aria-labelledby="submission-view-heading">
<DsPageHeading id="submission-view-heading">{dict.submission.pageTitle}</DsPageHeading>
<SubmissionView />
</section>
);
Expand Down
7 changes: 7 additions & 0 deletions frontend/app/ui/Header.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@
border-color: var(--app-border) !important;
}

.userDrop:focus-visible {
/* Keyboard focus needs a clearly visible ring; the surface-pinning rules
above otherwise leave focus indistinguishable from the resting state. */
outline: 2px solid var(--app-focus-ring) !important;
outline-offset: 2px;
}

.limitText {
max-width: 100px;
/* overflow: hidden; */
Expand Down
2 changes: 1 addition & 1 deletion frontend/app/ui/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ function Header({ headerNavItems }: HeaderProps) {
{displayName ? (
<Dropdown>
<Dropdown.Toggle className={styles.userDrop} data-testid="user-dropdown" id="dropdown-user">
<FaUser className="align-text-top" />
<FaUser className="align-text-top" aria-hidden="true" />
<span className={styles.limitText + ' ms-2 me-2'}>{displayName}</span>
</Dropdown.Toggle>
<Dropdown.Menu>
Expand Down
41 changes: 41 additions & 0 deletions frontend/app/ui/base/CenteredProgress.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
'use client';

import { ProgressCircle } from '@bcgov/design-system-react-components';

type CenteredProgressProps = {
/**
* Screen-reader-only label for the spinner. There is no visible loading text
* by design, so this is the only thing assistive tech announces — keep it
* meaningful (e.g. the localized "Loading…").
*/
label?: string;
/** Optional minimum height so the spinner can center within a tall empty area. */
minHeight?: string;
'data-testid'?: string;
};

/**
* The single loading indicator for the app: a horizontally/vertically centered
* ProgressCircle with no visible text. Use everywhere a screen, page data area,
* or table body is pending so loading looks and behaves the same throughout.
*
* Accessibility: the wrapper is a `role="status"` live region (implicit
* `aria-live="polite"`) so screen readers announce the spinner when it appears,
* and the ProgressCircle carries an `aria-label` as its accessible name.
*/
export function CenteredProgress({
label = 'Loading',
minHeight,
'data-testid': testId = 'loading-indicator',
}: CenteredProgressProps) {
return (
<div
role="status"
className="d-flex justify-content-center align-items-center p-5"
style={minHeight ? { minHeight } : undefined}
data-testid={testId}
>
<ProgressCircle isIndeterminate aria-label={label} />
</div>
);
}
Loading
Loading