SFDC-Assets
diff --git a/‎docs/DESIGN_CRITIQUE.md‎
Lines changed: 186 additions & 0 deletions b/‎docs/DESIGN_CRITIQUE.md‎
Lines changed: 186 additions & 0 deletions
diff --git a/‎docs/REACT_DEVELOPER_GUIDE.md‎
Lines changed: 16 additions & 2 deletions b/‎docs/REACT_DEVELOPER_GUIDE.md‎
Lines changed: 16 additions & 2 deletions
@@ -0,0 +1,186 @@
+# Design Critique — gps-design-systems-react
+
+**Date:** 2026-06-25  
+**Artifact critiqued:** The full monorepo as a product — architecture, component system, developer experience, testing posture, and documentation.  
+**Purpose:** Identify strengths and open gaps. Gaps are marked `[ ]` — they are not failures, just decisions that are currently open or deferred. Some may be intentional; treat each as a question worth answering before v1.0.0 ship.
+
+---
+
+## What was evaluated
+
+- Package inventory and coupling model (`react-uswds`, `react-bridge`, `react-ontario`, `gov-components`)
+- Component API consistency across tiers
+- Wizard / multi-step form pattern
+- Testing posture per package
+- Developer documentation
+- Showcase and reference apps
+- Design system coverage and extensibility model
+
+---
+
+## Strengths (what is working well)
+
+### 1. Coupling model is intentional and enforced
+`react-bridge` has zero design-system imports. ESLint rules enforce the separation at the import boundary. This is the right call — it means the bridge can serve USWDS, Ontario, or any future design system without modification. The constraint is documented, named (PB-1, SB-1), and has a rationale.
+
+### 2. Wizard layer solves the right problem at the right abstraction
+`createWizardStore` + `useWizardStep` decomposes OmniScript into three named layers with clear ownership: Zustand for global memory, RHF+Zod for per-step validation, gateway for the one exit point. The uncontrolled-input decision (no re-render on keystroke) is correct for performance and is documented. The module-scope rule for `createWizardStore` is clearly stated and has an explicit anti-pattern in the docs.
+
+### 3. Security posture is explicit
+DOMPurify is lazy-initialized, frozen, and configured with `FORCE_BODY` (mXSS hardening). `sanitizeHref` handles mixed-case bypass attempts. The gateway route dispatch uses `hasOwnProperty` to guard prototype pollution. These are not incidental — they are documented in `SECURITY.md` with a rationale table. This is a high-trust pattern for a product that will run inside a Salesforce org context.
+
+### 4. i18n is first-class across all packages
+Each package owns an isolated `i18next` instance. The prop-wins-over-translation contract (`prop ?? t('key')`) is consistent. The `.i18n.test.tsx` sibling pattern means locale coverage is tested, not assumed.
+
+### 5. Focus management is precise
+`queueMicrotask` (not `setTimeout`) for post-render focus moves. Explicit `triggerRef` capture for modal restoration. Boolean `isSelectingRef` for ComboBox blur guard. These indicate someone thought carefully about the LWS proxy membrane and where timing assumptions break.
+
+### 6. Documentation is genuinely useful
+`ARCHITECTURE.md` documents the platform constraint register explicitly (modal `aria-hidden`, SVG cross-origin, Skipnav target, frontdoor expiry). `SECURITY.md` has a sanitizer decision table. `AGENT_GUIDE.md` and `REACT_DEVELOPER_GUIDE.md` target different audiences correctly. The Glossary bridges Salesforce and web vocabulary for teams coming from one side or the other.
+
+---
+
+## Gaps
+
+Each gap is a question, not a verdict. The `[ ]` marker means: _this decision is open — address it, defer it consciously, or close it as won't-fix._
+
+---
+
+### Architecture and package model
+
+**GAP-A1 — `gov-components` has no publishable build**  
+`@gov-ido/gov-components` is `private: true` with no tsup build, no `main`/`module`/`types` fields, and can only be consumed from within this monorepo. The 84 components it contains are not accessible to PS teams working in separate repos.  
+`[ ]` Decision: Is `gov-components` intended to ship as a published package, or is in-repo consumption the permanent model?  
+_Principle: Consistency — packages that look like libraries but can't be used as libraries violate the mental model of a package registry._
+
+**GAP-A2 — `react-ontario` and `react-uswds` duplicate utilities**  
+`safeHtml` and `sanitizeHref` are implemented independently in both `react-uswds` and `react-ontario`. If a security fix is needed (e.g., a new `javascript:` bypass pattern), it must be applied in two places.  
+`[ ]` Decision: Should shared security utilities live in `react-bridge` or a new `@sfgps-ds/utils` package?  
+_Principle: Consistency — same thing should behave the same way everywhere._
+
+**GAP-A3 — GraphQL gateway migration is partially deferred**  
+`createGraphQLGateway` exists and is documented, but the env.d.ts block and one-line import rename needed at GA graduation are noted as deferred. The current state creates a latent inconsistency between dev and production gateway behavior.  
+`[ ]` Decision: Is the GA migration blocked on a Salesforce SDK release date, or is there an owner and timeline?
+
+**GAP-A4 — No monorepo build orchestration**  
+Build ordering relies on npm workspace scripts with no Turborepo/Nx dependency graph. `gov-components` has no build step at all. As the number of publishable packages grows, build correctness becomes harder to guarantee.  
+`[ ]` Decision: Is the current npm-workspace approach sufficient for the target number of packages, or is a task runner needed before v1.0.0?
+
+---
+
+### Component API consistency
+
+**GAP-C1 — Six `react-uswds` components have no tests**  
+`Form`, `FormGroup`, `Grid`, `Label`, `Link`, and `ValidationChecklist` have no `__tests__` directory. All six are low-complexity presentational components, but they are also the components most likely to be composed together — a regression in `Form` or `Label` may not surface until an integration test fails.  
+`[ ]` Decision: Are these intentionally deferred (low risk, high cost), or is this a coverage gap to close before v1.0.0?  
+_Principle: Trust — predictable behavior requires predictable coverage._
+
+**GAP-C2 — `react-bridge` has no coverage thresholds**  
+The three component packages enforce 80% coverage (branches / functions / lines / statements). `react-bridge` has no thresholds configured. This is the most critical package — a regression in `useGateway`, `createWizardStore`, or `usePlatformEvent` affects every consuming app.  
+`[ ]` Decision: Should `react-bridge` enforce the same 80% threshold (or higher) as the component packages?
+
+**GAP-C3 — `OntarioSummaryList` is excluded from the public API**  
+The component exists in source, has a test file, and two import tests are skipped pending upstream ODS export. This creates a gap in the Ontario DS coverage that is invisible from the public API surface.  
+`[ ]` Decision: Is there a tracking issue upstream with ODS for exporting `OntarioSummaryList`? Is a workaround (re-export with a warning) acceptable before the upstream fix lands?
+
+**GAP-C4 — `gov-components` has no SPEC.md files**  
+`react-uswds` has 49 of 59 SPEC.md files; `react-ontario` has 33 of 33. `gov-components` has 0 of 83. SPEC.md files are how the Universal Spec Matrix contracts (LWS-1 through I18N-2) are applied per component. Without them, the 84 gov components have no declared baseline for what "correct" looks like.  
+`[ ]` Decision: Is SPEC.md coverage for `gov-components` in scope before v1.0.0, or is the private/in-repo status the reason it's deferred?
+
+**GAP-C5 — `TEMPLATE.test.tsx` in `gov-components` is aspirational infrastructure not yet used**  
+The shared test setup (`setup/test-setup`) with `assertUSWDSClass`, `LWS_RULES`, `assertNoScriptInjection`, etc., exists but is not imported by any actual component test. Component tests are self-contained. The template represents an intent that hasn't been socialized into the test authoring pattern.  
+`[ ]` Decision: Should the shared setup be adopted as the standard for all new `gov-components` tests, or retired as over-engineering for a private package?  
+_Principle: Clarity — a template that doesn't reflect actual practice misleads contributors._
+
+---
+
+### Wizard and multi-step forms
+
+**GAP-W1 — No orchestrator-level error recovery pattern is documented**  
+`useWizardStep` handles per-step validation errors via `ErrorSummary`. But the docs and examples don't show what happens when the final gateway `post` call fails — whether the wizard stays on the last step, shows a full-screen error state, or redirects. For a government service form (e.g., a permit application), submission failure recovery is a critical UX concern.  
+`[ ]` Decision: Should the developer guide and `examples/03-multi-step-wizard/` demonstrate a submission error state as a required pattern, not just a happy path?  
+_Principle: Trust — honest states include error states, not just loading and success._
+
+**GAP-W2 — Save-in-progress / resume is absent from the wizard model**  
+The Zustand store survives back-navigation and step remounts within a session. It does not persist across page refreshes or sessions. For government forms that may take multiple sessions to complete (disability applications, permit renewals), this is a significant gap.  
+`[ ]` Decision: Is session persistence (e.g., localStorage hydration or a draft API endpoint) in scope for v1.0.0, or explicitly out of scope for this layer?  
+_Cognitive principle: Zeigarnik Effect — users return to incomplete tasks; the system should meet them there._
+
+**GAP-W3 — No branching/conditional step model**  
+The wizard pattern assumes a linear step sequence. OmniScript supported conditional navigation (skip step B if answer to step A is X). The current model requires consumers to implement branching logic themselves in the orchestrator.  
+`[ ]` Decision: Is conditional step navigation a documented pattern for consumers to own, or a gap in the bridge abstraction?
+
+---
+
+### Testing posture
+
+**GAP-T1 — E2E coverage for `gov-components` is not verified as comprehensive**  
+`component-lab` has 90 gov-component pages and Playwright runs against them, but there are no SPEC.md files defining what each E2E test must cover. E2E tests may exist for some components and not others, and there is no traceability between test intent and component contract.  
+`[ ]` Decision: Is E2E coverage for `gov-components` sufficient without SPEC.md contracts, or does this create a compliance risk (AODA-1, AODA-2, AODA-3)?
+
+**GAP-T2 — Mobile E2E coverage is limited to two specs**  
+Playwright mobile projects (Pixel 5, iPhone 12) are limited to `a11y.spec.ts` and `banner.spec.ts`. The remaining ~138 E2E specs run desktop-only. Components with touch targets, overflow menus, or responsive layout (Nav, SidePanel, LanguageSelector, Modal) are not validated on mobile.  
+`[ ]` Decision: Is mobile E2E coverage a v1.0.0 requirement or deferred to a post-ship iteration?  
+_Interaction principle: Touch targets must be validated at runtime, not assumed from code._
+
+**GAP-T3 — `react-bridge` has no E2E coverage**  
+Unit tests cover gateway, wizard, config, and pub/sub in isolation. There is no E2E fixture that exercises a full wizard-submit-gateway cycle against a mock server. The `03-multi-step-wizard` reference app may serve this purpose, but it is not wired to the Playwright suite.  
+`[ ]` Decision: Should the reference apps be part of the automated test suite, or are they documentation artifacts only?
+
+---
+
+### Developer experience
+
+**GAP-D1 — No component storybook or interactive component browser**  
+`component-lab` and `ontario-lab` are Vite fixtures with manually authored pages — one page per component. There is no Storybook, Histoire, or equivalent that provides interactive prop controls, variant documentation, or a searchable component index. PS teams evaluating which component to use must read source or docs.  
+`[ ]` Decision: Is an interactive component browser in scope, or is the Experience Cloud showcase the intended browsing surface?  
+_Cognitive principle: Recognition over recall — making variants visible reduces the need for contributors to remember what a component can do._
+
+**GAP-D2 — No automated changelog / release notes generation**  
+Changesets CLI is present for versioning, but there is no documented process for what a changelog entry must contain, who writes it, or whether it is generated from commit messages or authored manually. For an OSS package, changelog quality directly affects consumer trust.  
+`[ ]` Decision: Is changelog authoring a manual step owned by the release engineer, or should it be automated from conventional commit messages?
+
+**GAP-D3 — `ADDING_A_DESIGN_SYSTEM.md` has no reference implementation**  
+The 9-phase checklist for onboarding a new design system is comprehensive. But the first two real integrations (USWDS and Ontario) both exist in the same repo, so there is no external reference for a team adding a third design system from a blank slate.  
+`[ ]` Decision: Should `react-ontario` serve as the canonical worked example, with the checklist cross-referencing it explicitly?
+
+---
+
+### Documentation
+
+**GAP-DOC1 — No user-facing error message catalog**  
+`SECURITY.md` documents what the sanitizer blocks. `ARCHITECTURE.md` documents the platform constraint register. Neither document covers what error messages a user sees in the browser when things go wrong (Zod validation messages, gateway errors, config validation failures). These are usually authored ad-hoc in each component or wizard step.  
+`[ ]` Decision: Should there be a standard error message vocabulary for this system, or is error copy owned entirely by the consuming app?  
+_Principle: Clarity — explicit labels beat clever ones; error messages are labels for failure states._
+
+**GAP-DOC2 — Agentforce integration is documented but not showcased end-to-end**  
+`AgentforceWidget` has a separate entry point (`./agentforce`), a mock adapter, and a token server (`agentforce-token-server/`). There is no complete worked example showing how a PS team would wire the widget to a real Agentforce agent in a scratch org. The token server is a `server.mjs` with no accompanying instructions in the quickstart.  
+`[ ]` Decision: Is an end-to-end Agentforce integration example in scope for v1.0.0?
+
+---
+
+## Summary of open gaps by area
+
+| Area | Gap IDs | Count |
+|---|---|---|
+| Architecture / packaging | A1, A2, A3, A4 | 4 |
+| Component API consistency | C1, C2, C3, C4, C5 | 5 |
+| Wizard / multi-step forms | W1, W2, W3 | 3 |
+| Testing posture | T1, T2, T3 | 3 |
+| Developer experience | D1, D2, D3 | 3 |
+| Documentation | DOC1, DOC2 | 2 |
+| **Total** | | **20** |
+
+---
+
+## Recommended prioritization for v1.0.0
+
+The following gaps most directly affect **trust** (the principle that a system behaves predictably and honestly) and **consumer correctness** (PS teams building on this will reproduce whatever the reference establishes):
+
+1. **GAP-C2** — Add coverage thresholds to `react-bridge`. Highest-value, lowest-effort fix.
+2. **GAP-W1** — Document and implement submission error recovery in the wizard example. Government forms will fail in production; the pattern needs to exist before it's needed.
+3. **GAP-A2** — Consolidate `safeHtml`/`sanitizeHref` to a single source. Security utilities should not drift.
+4. **GAP-C1** — Add baseline tests for the six untested `react-uswds` components. Low complexity, low effort, closes a visible gap before OSS publish.
+5. **GAP-T2** — Expand mobile E2E to at least the overlay/navigation components (Nav, Modal, SidePanel). AODA compliance requires mobile validation.
+
+The remaining gaps (A1, A3, A4, C3, C4, C5, W2, W3, T1, T3, D1, D2, D3, DOC1, DOC2) are legitimate deferred decisions — not bugs. Each should have a named owner and a milestone before the OSS repo goes public.
@@ -846,27 +846,41 @@ The orchestrator tracks the current step and makes the single gateway call on fi
 ```tsx
 function CaseWizard() {
   const [step, setStep] = useState(1);
+  const [submitError, setSubmitError] = useState<string | null>(null);
   const { payload, reset } = useCaseWizard();
   const { mutate: submit, isPending } = useGatewayMutation<CasePayload, { caseNumber: string }>('cases');
 
   function handleFinalSubmit() {
+    // Clear any prior submission error before retrying.
+    setSubmitError(null);
     // One call. Apex receives the complete typed payload.
     // Validation Rules, Flows, and Triggers handle everything else.
     submit(payload, {
-      onSuccess: (result) => { reset(); setStep(1); },
+      onSuccess: () => { reset(); setStep(1); },
+      onError: (err) => {
+        // The wizard store is still populated — the user can fix and resubmit
+        // without re-entering data. Always handle onError: government forms will
+        // fail in production and the user needs a recovery path.
+        setSubmitError((err as Error)?.message ?? 'Submission failed. Please try again.');
+      },
     });
   }
 
   return (
     <>
+      {submitError && (
+        <Alert heading="Submission error" type="error">{submitError}</Alert>
+      )}
       {step === 1 && <StepAgency onNext={() => setStep(2)} />}
       {step === 2 && <StepDraft onNext={() => setStep(3)} onBack={() => setStep(1)} />}
-      {step === 3 && <StepReview onBack={() => setStep(2)} onSubmit={handleFinalSubmit} />}
+      {step === 3 && <StepReview onBack={() => setStep(2)} onSubmit={handleFinalSubmit} isPending={isPending} />}
     </>
   );
 }
 ```
 
+> **Always handle `onError` on the final submit.** The wizard store is still populated when a gateway call fails — the user stays on the review step and can resubmit without re-entering data. Displaying an `<Alert type="error">` above the step gives the user a clear recovery path. Never leave `onError` undefined in a production wizard.
+
 ### Testing a wizard step
 
 Provide the store's payload as `storePayload` and assert that `updatePayload` receives the correct value after a valid submit.