Skip to content

Commit badb492

Browse files
authored
Refactor form dialogs with DialogForm and add Button isPending prop (#870)
### Summary & Motivation `DialogContent` is a bounded flex column that relies on `DialogHeader`, `DialogBody`, and `DialogFooter` being its direct flex children — `DialogBody` is the scroll region (`flex min-h-0 flex-1 overflow-y-auto`). A plain `<Form>` interposed between `DialogContent` and `DialogBody` had no flex classes of its own, so the scroll chain broke: multi-field dialogs overflowed past the max-height and pushed the footer out of view. The InviteUserDialog example in the rules worked only because it had a single field. - Add a `DialogForm` helper in `application/shared-webapp/ui/components/Dialog.tsx` that pre-applies `flex min-h-0 flex-1 flex-col`, wires `FormValidationContext`, and defaults `noValidate` from `validationBehavior="aria"`. It replaces `<Form>` inside dialogs; the wrapper still owns `DialogContent` and `DialogHeader`. - Migrate InviteUserDialog, ChangeUserRoleDialog, EditBillingInfoDialog, ShareRecipeDialog, and the three RecipeEditor wizard steps to `DialogForm`. - Update `.claude/rules/frontend/frontend.md` and `.claude/rules/frontend/form-with-validation.md` with the scroll-chain rationale, an anti-pattern note ("`<Form>` inside a dialog breaks scroll"), and a multi-field example. To make submitting feedback visible, add an `isPending` prop to `Button` that auto-disables and prepends a `<Spinner />`. Call sites change `disabled={mutation.isPending}` to `isPending={mutation.isPending}` and get the spinner for free. Cancel and Close buttons keep `disabled={mutation.isPending}` because they do not trigger mutations. - Extend `application/shared-webapp/ui/components/Button.tsx` with the `isPending` prop. - Apply it to mutation CTAs across the account self-contained system (login, signup, welcome setup, profile, account settings, verification code resend, user invite/role-change/delete, billing subscribe/upgrade/downgrade/reactivate/cancel/retry/payment-method/billing-info, plan cards, invitation accept/decline, component preview dialogs). - Extract `PlanAction` from `PlanCard` into its own file because the added props pushed `PlanCard` past the per-function and per-file line caps. - Document the pattern in `form-with-validation.md`, the dialog example in `frontend.md`, and the Button divergence row in the component README. ### Checklist - [x] I have added tests, or done manual regression tests - [x] I have updated the documentation, if necessary
2 parents 44becae + 0d59484 commit badb492

32 files changed

Lines changed: 297 additions & 188 deletions

.claude/rules/frontend/form-with-validation.md

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,35 +7,41 @@ description: Rules for forms with validation using ShadCN 2.0 components
77

88
## Form wiring
99

10-
- `api.useMutation` (or TanStack `useMutation` for multi-call flows) + `mutationSubmitter` on `<Form>`.
11-
- Server validation via `<Form validationErrors={mutation.error?.errors} validationBehavior="aria">`. Fields read their own errors from `FormValidationContext` by `name`.
12-
- Submit button: `disabled={mutation.isPending}` only. No client-side `isValid` gate — server validates.
13-
- If the backend can't return field-level errors (e.g. non-nullable `DateOnly` fails JSON deserialization on `""`), fix the backend: make it nullable + `NotNull` FluentValidation rule.
10+
- In dialogs use `<DialogForm>` (from `@repo/ui/components/Dialog`); outside dialogs use `<Form>`. A plain `<Form>` between `DialogContent` and `DialogBody` breaks the scroll chain — `DialogForm` pre-applies `flex min-h-0 flex-1 flex-col` so `DialogBody`'s `flex-1 min-h-0 overflow-y-auto` still works.
11+
- `api.useMutation` (or TanStack `useMutation` for multi-call flows) + `mutationSubmitter` on the form.
12+
- Server validation: `validationErrors={mutation.error?.errors}` on the form. Fields read their own errors from `FormValidationContext` by `name`. `DialogForm` defaults `validationBehavior` to `"aria"`; `Form` requires you to set it.
13+
- Mutation CTAs: `isPending={mutation.isPending}` on `<Button>` (auto-disables, prepends `<Spinner />`). Cancel/Close keep `disabled={...}` — they are not CTAs, no spinner.
14+
- No client-side `isValid` gate — the server validates, and gating client-side hides the real errors from the user.
15+
- If the backend can't return field-level errors (e.g. non-nullable `DateOnly` fails JSON deserialization on `""`), fix the backend: make it nullable + add a `NotNull` FluentValidation rule.
1416

1517
## Dialog wrapper/body split
1618

1719
Every form dialog has two components in the same file:
1820

1921
- **Wrapper** (`XxxDialog`): receives `isOpen` / `onOpenChange`. Contains only `DirtyDialog` + `DialogContent` + `DialogHeader`. No state, no mutation, no dirty tracking.
20-
- **Body** (`XxxDialogBody`): child of `<DialogContent>`. Owns all state, mutation, `Form`, `DialogBody`, `DialogFooter`. Signals dirtiness via `useDialogSetDirty()` in field `onChange` handlers.
22+
- **Body** (`XxxDialogBody`): child of `<DialogContent>`. Owns state, mutation, the form, `DialogBody`, and `DialogFooter`. Signals dirtiness via `useDialogSetDirty()` (from `@repo/ui/components/DirtyDialogContext`) in field `onChange` handlers.
2123

22-
**Why this split:** `DialogContent` unmounts its children on close, so the body is recreated on every open. Form state, mutation errors, dirty flag — all reset automatically because the components holding them no longer exist. No `handleCloseComplete`, no `mutation.reset()`, no `setIsFormDirty(false)` anywhere.
24+
`DialogContent` unmounts children on close, so the body is recreated on every open. Form state, mutation errors, and the dirty flag reset automatically — never write `handleCloseComplete`, `mutation.reset()`, or `setIsFormDirty(false)`.
2325

24-
## DirtyDialog API
26+
Wizard state (`step`, intermediate values) also lives in the body — unmount resets the wizard to step 0 on reopen.
2527

26-
- Props: `open`, `onOpenChange`, `trackingTitle`, optional label overrides. That's it.
27-
- Body calls `useDialogSetDirty()(true)` on any field change. The wrapper tracks the flag internally and clears it when `open` flips false.
28-
- Cancel button: `<DialogClose render={<Button type="reset" ... />}>``type="reset"` bypasses the unsaved warning.
29-
- Close on success: call `onClose` passed from the wrapper. Do not reset anything manually.
28+
## DirtyDialog
29+
30+
- Props: `open`, `onOpenChange`, `trackingTitle` (+ optional label overrides). All state lives in the body.
31+
- Cancel button: `<DialogClose render={<Button type="reset" ... />}>`. `type="reset"` bypasses the unsaved-changes warning.
32+
- Close on success: call `onClose` passed from the wrapper. Do not reset anything manually — unmount does it.
3033

3134
## Anti-patterns
3235

36+
- `<Form>` inside a dialog — breaks the `DialogBody` scroll chain. Use `<DialogForm>`.
3337
- State (`useState`, `useMutation`, refs) in the wrapper — persists across close/reopen. Always in the body.
3438
- `isValid`-gated submit button — hides server errors from the user.
35-
- `handleCloseComplete` / `mutation.reset()` / `setIsFormDirty(false)` — symptoms of state in the wrong place.
39+
- `handleCloseComplete`, `mutation.reset()`, `setIsFormDirty(false)` — symptoms of state living in the wrong component.
3640
- `<FormErrorMessage>` — deprecated. Use `validationErrors`.
3741

38-
Note: .NET endpoints generate an `*.Api.json` on backend build; `openapi-typescript` turns it into `api.generated.d.ts`. Backend contract changes need both builds.
42+
## Multi-call submits
43+
44+
Compose `api.useMutation` calls inside a TanStack `useMutation({ mutationFn: async (d) => { ... } })`, then pass its `error?.errors` into `validationErrors`.
3945

4046
## Example
4147

@@ -61,27 +67,21 @@ function InviteUserDialogBody({ onClose }: { onClose: () => void }) {
6167
});
6268

6369
return (
64-
<Form
65-
onSubmit={mutationSubmitter(inviteMutation)}
66-
validationErrors={inviteMutation.error?.errors}
67-
validationBehavior="aria"
68-
>
70+
<DialogForm onSubmit={mutationSubmitter(inviteMutation)} validationErrors={inviteMutation.error?.errors}>
6971
<DialogBody>
70-
<TextField autoFocus name="email" label={t`Email`} onChange={() => setDirty(true)} />
72+
<TextField autoFocus required name="email" label={t`Email`} onChange={() => setDirty(true)} />
7173
</DialogBody>
7274
<DialogFooter>
7375
<DialogClose render={<Button type="reset" variant="secondary" disabled={inviteMutation.isPending} />}>
7476
<Trans>Cancel</Trans>
7577
</DialogClose>
76-
<Button type="submit" disabled={inviteMutation.isPending}>
78+
<Button type="submit" isPending={inviteMutation.isPending}>
7779
{inviteMutation.isPending ? <Trans>Sending...</Trans> : <Trans>Send invite</Trans>}
7880
</Button>
7981
</DialogFooter>
80-
</Form>
82+
</DialogForm>
8183
);
8284
}
8385
```
8486

85-
Multi-step dialogs: wizard state (`step`, intermediate values) also lives in the body — unmount resets the wizard to step 0 on reopen.
86-
87-
Multi-call submits: compose `api.useMutation` calls inside a TanStack `useMutation({ mutationFn: async (d) => { ... } })`, pass its `error?.errors` into `<Form validationErrors>`.
87+
Note: .NET endpoints generate `*.Api.json` on backend build; `openapi-typescript` turns it into `api.generated.d.ts`. Backend contract changes need both builds.

.claude/rules/frontend/frontend.md

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ Use browser MCP tools to test at `[APP_URL]`. Use `UNLOCK` as OTP verification c
3838
4. **API Integration**:
3939
- API client auto-generated from OpenAPI spec
4040
- Located in `shared/lib/api/client.ts`
41+
- A SPA only calls its own self-contained system's endpoints via that system's generated OpenAPI client. Prefixes: `main``/api/*`, `account``/api/account/*`, `back-office``/api/back-office/*`, etc. Cross-system needs go backend-to-backend via `/internal-api/...`, exposed to the SPA through a facade endpoint in its own system
4142
- Never make direct fetch calls
4243
- Server state lives in TanStack Query only
4344
- Use `queryClient.invalidateQueries()` to refresh data after mutations
@@ -130,6 +131,7 @@ Use browser MCP tools to test at `[APP_URL]`. Use `UNLOCK` as OTP verification c
130131
- Split every dialog into two components in the same file: wrapper owns only `isOpen` + `DirtyDialog`/`DialogContent`/`DialogHeader` shell, body lives inside `<DialogContent>` and owns all state/mutation/form. Body unmounts on close, so state auto-resets on reopen — no `handleCloseComplete`, no `mutation.reset()`
131132
- Body signals dirtiness with `useDialogSetDirty()` from `@repo/ui/components/DirtyDialogContext`. `DirtyDialog` tracks the flag internally and clears it on close
132133
- `DialogBody` between `DialogHeader` and `DialogFooter` (scroll container)
134+
- In dialogs use `DialogForm` (not `Form`); a `<Form>` between `DialogContent` and `DialogBody` breaks the scroll chain
133135
- Cancel button: `<DialogClose render={<Button type="reset" ... />}>``type="reset"` skips the unsaved-changes warning
134136
- Close on success: call `onClose` from the wrapper. Do not reset anything by hand — unmount does it
135137

@@ -182,23 +184,22 @@ function InviteUserDialogBody({ onClose }: { onClose: () => void }) { // ✅ Bod
182184
});
183185

184186
return (
185-
<Form
187+
<DialogForm // ✅ DialogForm (not Form) preserves the DialogBody scroll chain
186188
onSubmit={mutationSubmitter(inviteMutation)}
187189
validationErrors={inviteMutation.error?.errors}
188-
validationBehavior="aria"
189190
>
190191
<DialogBody> // ✅ Always wrap content in DialogBody
191192
<TextField autoFocus required name="email" label={t`Email`} onChange={() => setDirty(true)} />
192193
</DialogBody>
193194
<DialogFooter>
194-
<DialogClose render={<Button type="reset" variant="secondary" disabled={inviteMutation.isPending} />}> // ✅ type="reset" bypasses warning
195+
<DialogClose render={<Button type="reset" variant="secondary" disabled={inviteMutation.isPending} />}> // ✅ type="reset" bypasses warning; disabled (not isPending) — no spinner on Cancel
195196
<Trans>Cancel</Trans>
196197
</DialogClose>
197-
<Button type="submit" disabled={inviteMutation.isPending}> // ✅ disabled for pending
198+
<Button type="submit" isPending={inviteMutation.isPending}> // ✅ isPending auto-disables and prepends <Spinner />
198199
{inviteMutation.isPending ? <Trans>Sending...</Trans> : <Trans>Send invite</Trans>}
199200
</Button>
200201
</DialogFooter>
201-
</Form>
202+
</DialogForm>
202203
);
203204
}
204205

.claude/rules/frontend/translations.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ Guidelines for implementing translations and internationalization in the fronten
1414
3. Always use plain English text in translation markers
1515
4. Don't hardcode text without proper translation wrappers
1616
5. Use "Sentence case" for everything (buttons, menus, headings, etc.)
17-
6. **Lingui macros work in `shared-webapp/ui` components**. The SWC plugin runs in source-build mode across all workspace packages, and each SCS's `createLinguiConfig()` includes `shared-webapp/ui/**/*.{ts,tsx}` in its extraction scope. This means every shared UI component auto-generates catalog entries in every SCS (main, account, back-office) independently. Prefer macros over threading translatable text through props -- only accept a text prop when a consumer genuinely needs to override the default (e.g., context-specific wording). Note: shared strings must be translated in every SCS's `*.po` files -- translation workflow must keep all three SCSs in sync
17+
6. **Lingui macros work in `shared-webapp/ui` components**. The SWC plugin runs in source-build mode across all workspace packages, and each self-contained system's `createLinguiConfig()` includes `shared-webapp/ui/**/*.{ts,tsx}` in its extraction scope. This means every shared UI component auto-generates catalog entries in every self-contained system (main, account, back-office) independently. Prefer macros over threading translatable text through props -- only accept a text prop when a consumer genuinely needs to override the default (e.g., context-specific wording). Note: shared strings must be translated in every self-contained system's `*.po` files -- translation workflow must keep all three self-contained systems in sync
1818
7. Translation workflow:
1919
- Translation files are in `shared/translations/locale/` (e.g., `da-DK.po`, `en-US.po`)
2020
- After adding/changing `<Trans>` or t\`\` markers, the `*.po` files are auto-generated/updated by the build system

application/account/WebApp/federated-modules/common/AcceptInvitationDialog.tsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,17 @@ export function AcceptInvitationDialog({
7474
<Button
7575
variant="destructive"
7676
onClick={handleDeclineInvitation}
77-
disabled={isLoading || declineInvitationMutation.isPending}
77+
isPending={declineInvitationMutation.isPending}
78+
disabled={isLoading}
7879
>
7980
{declineInvitationMutation.isPending ? <Trans>Declining...</Trans> : <Trans>Decline</Trans>}
8081
</Button>
81-
<Button variant="default" onClick={onAccept} disabled={isLoading || declineInvitationMutation.isPending}>
82+
<Button
83+
variant="default"
84+
onClick={onAccept}
85+
isPending={isLoading}
86+
disabled={declineInvitationMutation.isPending}
87+
>
8288
{isLoading ? <Trans>Accepting...</Trans> : <Trans>Accept invitation</Trans>}
8389
</Button>
8490
</DialogFooter>

application/account/WebApp/routes/-components/OtpVerificationForm.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ export function OtpVerificationForm({
9191
</p>
9292
)}
9393
</div>
94-
<Button type="submit" className="mt-4 w-full text-center" disabled={isSubmitDisabled}>
94+
<Button type="submit" className="mt-4 w-full text-center" isPending={isSubmitting} disabled={isSubmitDisabled}>
9595
{isSubmitting ? <Trans>Verifying...</Trans> : <Trans>Verify</Trans>}
9696
</Button>
9797
</div>

application/account/WebApp/routes/-components/VerificationFooter.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ export function VerificationFooter({
4242
validationErrors={resendMutation.error?.errors}
4343
className="inline"
4444
>
45-
<Button type="submit" variant="link" disabled={resendMutation.isPending} className="h-auto p-0 text-sm">
45+
<Button type="submit" variant="link" isPending={resendMutation.isPending} className="h-auto p-0 text-sm">
4646
<Trans>Request a new code</Trans>
4747
</Button>
4848
</Form>

application/account/WebApp/routes/account/billing/-components/CancelDowngradeDialog.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ export function CancelDowngradeDialog({
6262
</AlertDialogHeader>
6363
<AlertDialogFooter>
6464
<AlertDialogCancel disabled={isPending}>{t`Keep downgrade`}</AlertDialogCancel>
65-
<AlertDialogAction onClick={onConfirm} disabled={isPending}>
65+
<AlertDialogAction onClick={onConfirm} isPending={isPending}>
6666
{isPending ? t`Processing...` : t`Cancel downgrade`}
6767
</AlertDialogAction>
6868
</AlertDialogFooter>

application/account/WebApp/routes/account/billing/-components/CancelSubscriptionDialog.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,8 @@ export function CancelSubscriptionDialog({
141141
onConfirm(reason, feedback.trim() || null);
142142
}
143143
}}
144-
disabled={isPending || reason === null}
144+
isPending={isPending}
145+
disabled={reason === null}
145146
>
146147
{isPending ? t`Cancelling...` : t`Cancel subscription`}
147148
</AlertDialogAction>

application/account/WebApp/routes/account/billing/-components/DowngradeConfirmationDialog.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ export function DowngradeConfirmationDialog({
5959
</AlertDialogHeader>
6060
<AlertDialogFooter>
6161
<AlertDialogCancel disabled={isPending}>{t`Cancel`}</AlertDialogCancel>
62-
<AlertDialogAction onClick={onConfirm} disabled={isPending}>
62+
<AlertDialogAction onClick={onConfirm} isPending={isPending}>
6363
{isPending ? t`Processing...` : t`Confirm downgrade`}
6464
</AlertDialogAction>
6565
</AlertDialogFooter>

application/account/WebApp/routes/account/billing/-components/EditBillingInfoDialog.tsx

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@ import {
99
DialogContent,
1010
DialogDescription,
1111
DialogFooter,
12+
DialogForm,
1213
DialogHeader,
1314
DialogTitle
1415
} from "@repo/ui/components/Dialog";
1516
import { DirtyDialog } from "@repo/ui/components/DirtyDialog";
1617
import { useDialogSetDirty } from "@repo/ui/components/DirtyDialogContext";
17-
import { Form } from "@repo/ui/components/Form";
1818
import { mutationSubmitter } from "@repo/ui/forms/mutationSubmitter";
1919
import { useQueryClient } from "@tanstack/react-query";
2020
import { useState } from "react";
@@ -113,11 +113,7 @@ function EditBillingInfoDialogBody({
113113
const markDirty = () => setDirty(true);
114114

115115
return (
116-
<Form
117-
onSubmit={mutationSubmitter(mutation)}
118-
validationErrors={mutation.error?.errors}
119-
className="flex min-h-0 flex-1 flex-col"
120-
>
116+
<DialogForm onSubmit={mutationSubmitter(mutation)} validationErrors={mutation.error?.errors}>
121117
<DialogBody>
122118
<BillingInfoFormFields
123119
billingInfo={billingInfo}
@@ -136,10 +132,10 @@ function EditBillingInfoDialogBody({
136132
<DialogClose render={<Button type="reset" variant="secondary" disabled={mutation.isPending} />}>
137133
<Trans>Cancel</Trans>
138134
</DialogClose>
139-
<Button type="submit" disabled={mutation.isPending}>
135+
<Button type="submit" isPending={mutation.isPending}>
140136
{mutation.isPending ? (pendingLabel ?? t`Saving...`) : (submitLabel ?? t`Save`)}
141137
</Button>
142138
</DialogFooter>
143-
</Form>
139+
</DialogForm>
144140
);
145141
}

0 commit comments

Comments
 (0)