docs: apply spec review round 1 corrections (GUI redesign design)

All corrections audited against the codebase:
- TEST-INFRA-001 verified Resolved; add §13 testing strategy
- §3 specify exact routesFolder + definePage layout meta convention
- §5 boundaries claim corrected; add §14 zone/matrix extension
- §4 drop useWorkspaceStore (dup) → reuse auth/org stores + useShellUiStore
- §12 explicit portal scope (/portal/*); §10 SmartFilter own sub-sprint

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
2026-05-15 23:17:38 +02:00
parent 890bcc88cb
commit 5068ee5db9

View File

@@ -2,13 +2,21 @@
| Field | Value |
|---|---|
| **Status** | Design approved (brainstorming) — pending spec review |
| **Status** | Design approved; spec review round 1 corrections applied — pending re-approval |
| **Date** | 2026-05-15 |
| **Author** | Brainstorming session (Bert + Claude Code) |
| **Supersedes** | The F4aF4d component-migration sub-packages of `dev-docs/RFC-WS-FRONTEND-PRIMEVUE.md` |
| **Next artifact** | Implementation plan (writing-plans), then a new project RFC in `dev-docs/` |
| **Source design system** | `crewli-starter/` (sibling working directory) |
> **Spec review round 1 (2026-05-15) — corrections applied, all audited against the codebase:**
> 1. **TEST-INFRA-001** (blocking): audited → ✅ Resolved; new §13 Testing strategy added (Playwright CT gate kept, re-baselined against crewli-starter, Storybook a11y complementary).
> 2. **Layout-selection mechanism** (blocking): §3 now specifies the exact `definePage({ meta: { layout: 'OrganizerLayoutV2' } })` convention + `routesFolder` vite config (both audited).
> 3. **eslint-plugin-boundaries** (blocking): §5 corrected (was factually wrong); new §14 specifies the `components-v2`/`pages-v2`/`components-foundation` zones + no-back-port asymmetry.
> 4. `useWorkspaceStore` → reuse `useAuthStore`/`useOrganisationStore` for org data + new `useShellUiStore` for sidebar/theme/density only (§4); `provide`/`inject` substitution is now a mandatory per-port rule.
> 5. Portal scope made explicit (§12, in scope, own later sprint, `/portal/*` not `/p/*`).
> 6. Smart Filter promoted to its own sub-sprint (§10).
---
## 1. Context & problem
@@ -78,16 +86,41 @@ apps/app/src/
│ ├── AppShell.vue # EXISTING — untouched
│ └── AppShellV2.vue # NEW — ports crewli-starter shell 1:1
├── stories/ # EXPANDED — see §6
├── stores/ # + useWorkspaceStore
├── stores/ # + useShellUiStore (sidebar/theme/density ONLY —
│ # org/context data reuses existing stores, §4)
├── composables/ # + useRightDrawer
└── types/v2/ # shared v2 types
```
**Routing:** `unplugin-vue-router` generates routes from `pages-v2/`;
they are pinned under `/v2/*` (route prefix/meta). `vite-plugin-vue-meta-layouts`
selects `OrganizerLayoutV2` for the `pages-v2/*` tree. No conditional
logic inside layout files — each page tree picks its own layout. The old
layout stays inert (zero regression risk on un-migrated pages).
**Routing (exact mechanism — audited 2026-05-15).** `vite.config.ts`
currently calls `VueRouter({ getRouteName: … })` with **no `routesFolder`**,
so it defaults to `src/pages` only — `pages-v2/` would generate zero
routes without a config change. Required change (foundation deliverable
1, §9):
```ts
VueRouter({
routesFolder: [
{ src: 'src/pages' }, // unchanged, no prefix
{ src: 'src/pages-v2', path: 'v2/' }, // new → all routes prefixed /v2/
],
getRouteName: , // unchanged
})
```
**Layout selection (exact convention — audited).** The project uses
`MetaLayouts({ target: './src/layouts', defaultLayout: 'default' })` and
`setupLayouts` in `src/plugins/1.router/index.ts`. Pages opt into a
layout via `definePage({ meta: { layout: '<LayoutFileName>' } })`
(verified: `login.vue``'blank'`, `forbidden.vue``'PublicLayout'`).
**Binding convention for v2:** every `pages-v2/**` page declares
`definePage({ meta: { layout: 'OrganizerLayoutV2' } })`, and
`src/layouts/OrganizerLayoutV2.vue` must exist (MetaLayouts `target` is
`./src/layouts`). No conditional logic inside layout files — each page
tree pins its own layout. The old layout stays inert (zero regression
risk on un-migrated pages). A lint/CI check (or a thin
`pages-v2/`-scoped wrapper) enforces the meta-key so a page can't
silently fall back to `default`.
**Cutover convention (per page):** when a v2 page is approved to replace
its v1 counterpart, move `pages-v2/X.vue``pages/X.vue` (overwrite),
@@ -125,13 +158,34 @@ siblings in `layouts/`.)
| RightDrawer | PrimeVue `Drawer` + custom slot scaffold | Plumbing free, content bespoke |
| WorkspaceSwitcher | custom visual + PrimeVue `Popover` | See §7.4 |
**State contract** (replaces crewli-starter `provide`/`inject`):
- `useWorkspaceStore` (Pinia): workspace, sidebar collapsed/expanded,
theme, density.
**State contract** (replaces crewli-starter `provide`/`inject`). Audited
2026-05-15: existing stores are `useAuthStore`, `useOrganisationStore`,
`useImpersonationStore`, `useNotificationStore` — there is **no**
workspace/theme/density store today (theme/density currently via Vuexy
`useSkins.ts`). crewli-starter's "workspace" concept maps to Crewli's
**organisation/context**, which already lives in `useAuthStore` +
`useOrganisationStore`. To avoid duplicating tenant state:
- **Org / context / workspace data:** read from existing
`useAuthStore` (`availableContexts`, current org) +
`useOrganisationStore` (org list, branding). `WorkspaceSwitcher`
consumes these — it does **not** own org data. Switching context
goes through the existing auth/org flow.
- **`useShellUiStore` (NEW — the only new store):** holds *only* genuinely
new v2-shell UI state with no existing home — `sidebarCollapsed`,
`density`, `theme` (v2 light/dark). Nothing tenant-related.
- `useRightDrawer()` composable: `open(component, props)`, `close()`.
- `vue-router`: replaces `provide('navigate', …)`.
- `<html data-theme>` / `<html data-density>` mechanism retained
(composes with Aura `darkModeSelector`); v2 bypasses Vuexy `useSkins.ts`.
(composes with Aura `darkModeSelector`); v2 bypasses Vuexy
`useSkins.ts`. `useShellUiStore` owns the writes to these attributes.
**`provide`/`inject` substitution is a mandatory per-port rule:**
crewli-starter uses `provide`/`inject` pervasively (`workspace`,
`navigate`, `openDrawer`, `theme`, `density`, …). Every ported component
must replace these with the store/composable/router equivalents above —
no `inject()` survives the port. This is an architectural decision in
the RFC, applied to every component without exception.
F3.5's `AppShell` features (notification bell, help, breadcrumb,
org-switcher card) are not lost — the underlying state wires up
@@ -161,8 +215,13 @@ PrimeVue → custom in `components-v2/`.
`defineEmits([...])` → typed `defineEmits<{...}>()`; `inject(...)`
composable/store; component-local types inline, shared types in
`types/v2/`.
- **Boundaries:** existing `eslint-plugin-boundaries` 10-zone matrix
applies to v2 folders identically; violations stay errors.
- **Boundaries (CORRECTED — audited 2026-05-15):** the matrix does **not**
apply to v2 folders automatically. `boundaries/elements` uses explicit
path patterns (`src/components/**`, `src/components/{shared,auth,settings}/**`,
`src/pages/{events,members,…}/**`, etc.); **nothing matches
`src/components-v2/**` or `src/pages-v2/**`** — they fall through to no
element type. New zones + matrix rows are a required foundation
deliverable. Specification in §14.
## 6. Storybook organization
@@ -291,9 +350,12 @@ direction.
## 9. Foundation sprint deliverables
1. Routing + layout scaffold: `OrganizerLayoutV2`, `AppShellV2`, `/v2/*`
mount, `useWorkspaceStore`, `useRightDrawer()`, theme/density wiring,
one empty `pages-v2/dashboard.vue` proving boot.
1. Routing + layout scaffold: **`vite.config.ts` `routesFolder` change**
(§3), `OrganizerLayoutV2`, `AppShellV2`, `/v2/*` mount via the
`definePage` meta-key convention (§3), `useShellUiStore`,
`useRightDrawer()`, theme/density wiring, **`eslint-plugin-boundaries`
zone + matrix extension (§14)**, one empty `pages-v2/dashboard.vue`
proving boot + a passing lint run.
2. Shell pieces ported 1:1 (TS): AppSidebar, AppTopbar, SidebarNav,
WorkspaceSwitcher, RightDrawer, AppDialog.
3. Tier-1 primitives (PrimeVue-based per §8).
@@ -307,14 +369,20 @@ direction.
## 10. Sequencing after foundation
- **Smart Filter sub-sprint:** the Smart Filter subsystem (5 editors +
`useSmartFilters` composable + bar/chip/popover, ~600 lines) is its
**own sub-sprint**, not "in parallel" work bundled into Page-1. It
lands immediately before Page-1 because Page-1 is its first consumer.
- **Page-1 sprint:** events list end-to-end via `ListTemplate`
(exercises SmartFilterBar + lazy DataTable + RightDrawer — highest
pattern-validation value). Smart Filter subsystem secured here.
Finish PrimeVue standard catalog stories in parallel.
(exercises the now-secured SmartFilterBar + lazy DataTable +
RightDrawer — highest pattern-validation value). Finish PrimeVue
standard catalog stories here.
- **Subsequent sprints:** one page tree at a time (dashboard → settings
→ events detail → members → platform → portal). Each: build under
→ events detail → members → platform → **portal**). Each: build under
`pages-v2/`, add only needed components/stories, sign-off, cutover
commit.
commit. Portal is in scope (§12) but is its own later sprint with its
own `PortalLayoutV2` (token-auth, different chrome than the organizer
shell).
- **Domain modules** migrate with their owning page.
- **Final cutover:** rename `pages-v2/``pages/`,
`components-v2/``components/`, drop `*V2` suffixes, delete dead v1
@@ -322,9 +390,11 @@ direction.
## 11. Quality gates (per sprint)
`pnpm test`, `pnpm typecheck`, `pnpm lint` (boundaries matrix on v2
folders), Storybook a11y panel clean on new stories, visual sign-off
against crewli-starter. No `any`. Existing test count must not regress.
`pnpm test`, `pnpm typecheck`, `pnpm lint` (with the §14 boundaries
extension active — v2 folders are zoned, not unmatched), Playwright CT +
visual baselines per §13, Storybook a11y panel clean on new stories,
visual sign-off against crewli-starter. No `any`. Existing test count
must not regress.
## 12. Non-goals
@@ -336,3 +406,67 @@ against crewli-starter. No `any`. Existing test count must not regress.
- Flatpickr / vue-i18n / DatePicker decisions inherit from
RFC-WS-FRONTEND-PRIMEVUE unchanged.
- No PrimeVue Pro / template purchase.
**Portal scope (explicit decision):** the portal surface (route prefix
is **`/portal/*`**, not `/p/*` — RFC-WS-FRONTEND-PRIMEVUE corrected this;
token-auth, `portal.token` middleware) **is in scope** for the v2
redesign — crewli-starter ships portal designs (`PortalPage`,
`PortalVolunteerPage`, `PortalCustomerPage`, `PortalVolunteerFormPage`).
It is **not** foundation work: it is a dedicated later sprint with its
own `PortalLayoutV2` (the token-auth portal chrome differs from the
organizer shell). During parallel mode it lives at `/v2/portal/*`;
cutover folds it to `/portal/*` like every other tree.
## 13. Testing strategy (audited 2026-05-15)
The review correctly flagged that RFC Amendment A-1 made TEST-INFRA-001
a prerequisite to F2. **Audit result: TEST-INFRA-001, TEST-CONTRACT-001,
and TEST-VISUAL-001 are all ✅ Resolved** (`BACKLOG.md` §§ at lines
2307 / 2371 / 2424; closed in branch `chore/test-infra-001`, commits
`b8d18e6` / `2dfb1e8` / `f6509d9`). The Playwright Component Testing +
visual-regression foundation **exists in the repo today**
(`playwright-ct.config.ts`, `pnpm test:component`, `pnpm test:visual`).
The precondition is therefore satisfied — but RFC R-11's rationale
("jsdom does not detect visual regression in a multi-component
migration") is **more** relevant here, not less, because v2 builds
components from scratch rather than translating them. Explicit decision:
| Tier | Tool | v2 application |
|---|---|---|
| Logic / props / emits | Vitest + jsdom | Unchanged. Component-local logic, store/composable units. |
| Component render + a11y | Playwright CT (`test:component`) | **Required** for every bespoke component (§7) and every template (§8 Tier-3). Reuses the TEST-INFRA-001 foundation. |
| Visual regression | Playwright CT `@visual` (`test:visual`) | **Required** baselines for bespoke components + templates + the shell. Workflow: the v2 component is visually signed off against crewli-starter (human parity check), *then* its rendered output is captured as the committed baseline screenshot; thereafter CI flags any drift from that approved baseline. (Not an automated cross-repo diff against crewli-starter, and *not* the legacy Vuexy prototype HTML that TEST-VISUAL-001 baselined.) Sub-package closure requires green baselines. |
| Accessibility (authoring aid) | Storybook `addon-a11y` | **Complementary, not a replacement** for Playwright a11y. Storybook is the authoring/QA surface; CI correctness gate stays Playwright CT + `vitest-axe`. |
Storybook + manual sign-off does **not** replace Playwright CT — it
augments it. This is the deliberate, bidirectional-valid choice the
review asked for, made explicitly: keep the Playwright gate, add
Storybook as the authoring/review surface, re-baseline visuals against
crewli-starter.
## 14. eslint-plugin-boundaries extension (audited 2026-05-15)
My original §5 claim ("matrix applies identically") was **factually
wrong** and is corrected here. `.eslintrc.cjs` `boundaries/elements`
uses explicit, order-sensitive path patterns; none match
`src/components-v2/**` or `src/pages-v2/**`. Required foundation change
(deliverable 1):
1. **New element zones** (added *before* the generic `components`
catch-all, since first-match wins):
- `{ type: 'components-v2', pattern: 'src/components-v2/**' }`
- `{ type: 'pages-v2', pattern: 'src/pages-v2/**' }`
- A narrow whitelist zone for the two intentional cross-imports:
`{ type: 'components-foundation', pattern: 'src/components/{forms/**,Icon.vue}' }`
(FormField + Icon — audited to live in the generic `components`
zone today, not `components-shared`).
2. **New matrix rows** (`boundaries/element-types`):
- `{ from: 'components-v2', allow: ['types','utils','lib','composables','composables-forms','stores','components-v2','components-foundation'] }`
- `{ from: 'pages-v2', allow: ['types','utils','lib','composables','composables-forms','stores','navigation','components-v2','components-foundation','layouts','plugins'] }`
3. **Asymmetry rule:** `components-v2` / `pages-v2` may import
`components-foundation` (FormField, Icon) — the *only* permitted
v1→v2 bridge. v1 zones get **no** `allow` entry for `components-v2`
(no back-porting, enforced structurally, matching §12).
Exact ESLint object syntax is an implementation-plan task; this section
fixes the *contract* (zones, allowed edges, the no-back-port asymmetry).