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

All corrections audited against the codebase:
- §7.4 useWorkspaceStore ghost removed (computed over auth/org stores)
- §12 portal /portal/* verified in repo; observability is meta-based,
  /api/v1/p/* is separate backend layer — no cross-doc conflict
- §3 getRouteName v2- name-prefix convention (route-name collision)
- §4 theme parallel-mode AD + useRightDrawer in useShellUiStore
- §8/§9 DraggableBlock is foundation, not Tier-4
- §3 single ESLint enforcement for definePage meta-key
- §8 StatusTag severity map; §14 brace-glob fallback; §13 CT/Storybook

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
2026-05-16 00:33:51 +02:00
parent 5068ee5db9
commit 4302ed389d

View File

@@ -2,7 +2,7 @@
| Field | Value |
|---|---|
| **Status** | Design approved; spec review round 1 corrections applied — pending re-approval |
| **Status** | Design approved; spec review rounds 12 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` |
@@ -16,6 +16,18 @@
> 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).
>
> **Spec review round 2 (2026-05-16) — corrections applied, all audited against the codebase:**
> 1. **`useWorkspaceStore` ghost** (blocking): §7.4 contradiction removed — WorkspaceSwitcher data now explicitly derived via computed over `useAuthStore.organisations`/`currentOrganisation` + `useOrganisationStore` branding; no store.
> 2. **Portal `/p/*` vs `/portal/*`** (blocking): audited — frontend SPA *already* uses `/portal/*` (`src/pages/portal/...`); observability binds on `route.meta.context` not path (`contextBinding.ts:51`); `/api/v1/p/*` is a separate untouched backend layer. §12 rewritten; **no cross-doc commit needed** (feared conflict dissolved by audit).
> 3. **Route name collision** (blocking): §3 specifies the `getRouteName` `v2-` name-prefix convention for the second `routesFolder` (prevents `events` name clash).
> 4. Theme/density parallel-mode: explicit AD (§4) — v1/v2 *not* synchronised during parallel-mode (accepted, temporary; bridge explicitly rejected).
> 5. `useRightDrawer()` state: decided (§4) — lives in `useShellUiStore` (CT-testable), composable is a thin facade.
> 6. `DraggableBlock`: disambiguated (§8/§9) — it **is** a foundation deliverable; Tier-4 defers only the Timetable/Cue pages.
> 7. `definePage` enforcement: single mechanism chosen (§3) — custom ESLint rule on `pages-v2/**`, no "or".
> 8. StatusTag severity map: documented table + single-source-of-truth rule (§8), seeded from audited `src/types/` enums.
> 9. `components-foundation` brace-glob: verification note + two-pattern fallback (§14).
> 10. Storybook ↔ Playwright CT interplay: decided (§13) — independent surfaces, standalone CT specs, no `@storybook/test-runner` (matches TEST-INFRA-001 infra).
---
@@ -104,7 +116,20 @@ VueRouter({
{ src: 'src/pages' }, // unchanged, no prefix
{ src: 'src/pages-v2', path: 'v2/' }, // new → all routes prefixed /v2/
],
getRouteName: , // unchanged
// CHANGED (issue 3 — route NAME collision): the existing getRouteName
// derives the name from the file path *relative to its routesFolder*,
// so pages/events/index.vue and pages-v2/events/index.vue would BOTH
// generate name `events` → silent runtime collision (router.push({
// name: 'events' }) becomes ambiguous; one wins). The v2 routesFolder
// node carries `path: 'v2/'`; getRouteName must detect that origin and
// prefix the name with `v2-`. Binding convention:
// pages-v2/events/index.vue → name `v2-events`
// pages-v2/events/[id].vue → name `v2-events-id`
// Every v2 `<RouterLink :to="{ name }">` / `router.push({ name })`
// uses the `v2-` prefix. At final cutover the prefix is stripped in
// the same commit as the folder rename (mechanical find/replace).
getRouteName: routeNode => { /* existing kebab logic + v2- prefix when
routeNode originates from src/pages-v2 */ },
})
```
@@ -118,9 +143,16 @@ layout via `definePage({ meta: { layout: '<LayoutFileName>' } })`
`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`.
risk on un-migrated pages).
**Enforcement (issue 7 — single chosen mechanism, no "or"):** a custom
ESLint rule scoped to `src/pages-v2/**/*.vue` that fails the build
unless the file contains a `definePage({ meta: { layout:
'OrganizerLayoutV2' } })` call with exactly that layout value (portal v2
pages: `'PortalLayoutV2'`). ~15-line AST rule (or `ast-grep`),
foundation deliverable 1. Chosen over a runtime wrapper because a
missing meta-key is otherwise a silent wrong-shell bug (no error, just
the `default` layout) — an ESLint error fails CI loudly at author time.
**Cutover convention (per page):** when a v2 page is approved to replace
its v1 counterpart, move `pages-v2/X.vue``pages/X.vue` (overwrite),
@@ -173,13 +205,33 @@ workspace/theme/density store today (theme/density currently via Vuexy
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()`.
`density`, `theme` (v2 light/dark), **and the RightDrawer state**
(`activeComponent`, `props`, `isOpen`). Nothing tenant-related.
- **`useRightDrawer()` (issue 5 — state location decided):** a thin
composable **facade over `useShellUiStore`** (not a module-level
`ref()` singleton). `open(component, props)` / `close()` mutate the
store. Rationale: Playwright CT can drive the drawer by setting store
state directly (via `@pinia/testing`) without rendering the shell — a
module-level ref would force importing the composable and leak state
across tests. Decision is binding, not deferred.
- `vue-router`: replaces `provide('navigate', …)`.
- `<html data-theme>` / `<html data-density>` mechanism retained
(composes with Aura `darkModeSelector`); v2 bypasses Vuexy
`useSkins.ts`. `useShellUiStore` owns the writes to these attributes.
**AD — theme/density during parallel-mode (issue 4, decided not
deferred):** v1 (Vuexy `useSkins.ts`) and v2 (`useShellUiStore`) have
**separate sources of truth** and are **not synchronised** while both
ship. Crossing a v1↔v2 boundary mid-session may not carry a
just-changed theme. This is an **accepted, documented limitation**: it
is temporary (gone at final cutover), the cross-boundary-after-toggle
path is rare, and a bridge would couple new code to the Vuexy module
being deleted. Explicitly rejected alternative: a
`useShellUiStore.$subscribe` → Vuexy skin-key writer (rejected — adds
coupling to dead-end code for a transient edge case). If user testing
later shows this is jarring, the one-way bridge is the pre-identified
fallback, but it is **not** built by default.
**`provide`/`inject` substitution is a mandatory per-port rule:**
crewli-starter uses `provide`/`inject` pervasively (`workspace`,
`navigate`, `openDrawer`, `theme`, `density`, …). Every ported component
@@ -302,9 +354,20 @@ chevron) stays 1:1 custom CSS. Dropdown plumbing (open/close,
click-outside, positioning) replaced by PrimeVue `Popover` (drops manual
`mousedown` listener). Panel content (Workspaces header + Manage link,
list with gradient logos + current checkmark, footer New workspace /
Invite) stays custom markup. Data shape
`{ initials, name, sub, gradient: [from, to] }` backed by
`useWorkspaceStore`.
Invite) stays custom markup.
**Data source (corrected — consistent with §4, no `useWorkspaceStore`):**
the switcher reads existing stores only:
- org list ← `useAuthStore().organisations`
- current org ← `useAuthStore().currentOrganisation` (existing computed)
- context switching ← existing auth/org flow (not a new action)
The crewli-starter shape `{ initials, name, sub, gradient }` is **derived
via computed properties over that store data**, not stored:
`name` = org name; `initials` = computed from `name`; `gradient` =
derived from `useOrganisationStore` branding (`primaryColor` per
RFC-WS-FRONTEND-PRIMEVUE AD-2) → a two-stop gradient; `sub` = org
metadata line. No new store, no duplicated tenant state.
**Pattern established:** overlay/focus/escape/click-outside behaviour
comes free from PrimeVue `Dialog`/`Drawer`/`Popover`; only bespoke
@@ -323,6 +386,27 @@ comes free from PrimeVue `Dialog`/`Drawer`/`Popover`; only bespoke
| PageHead | thin Tailwind flex (title/sub/`#actions`) | none (pure layout) |
| EnergyDots / EnergyPicker | 5-dot meter — no PrimeVue primitive (`Rating` is stars, wrong visual) | minimal, justified |
**StatusTag severity map (issue 8 — documented, single source of
truth).** Audited backend-mirrored enums in `src/types/` today:
`ShiftAssignmentStatus`, `ArtistEngagementStatus`, `PaymentStatus`,
`PersonStatus`, `MatchStatus`. Status→PrimeVue-`severity` is **project
knowledge, not a per-page decision** — it lives in one map
`components-v2/shared/statusSeverity.ts`, consumed by `StatusTag`:
| Status value(s) | `severity` | Semantics |
|---|---|---|
| `approved`, `confirmed`, `completed`, `active` | `success` | terminal-good / active |
| `pending`, `pending_approval`, `invited` | `warn` | awaiting action |
| `rejected`, `cancelled`, `declined` | `danger` | terminal-bad |
| `draft` | `secondary` | not yet live |
| `inactive`, `expired` | `secondary` (muted) | dormant |
| (unmapped fallback) | `info` | + dev-warn so gaps surface |
Rule: **every** backend status enum mirrored into `src/types/` gets a
row here in the same PR that adds the enum (extends the existing
"mirror backend PHP enums" project rule). `StatusTag` never inlines a
severity; it always resolves through this map.
**Tier-2 — Smart Filter subsystem (secure generic version now):**
`SmartFilterBar` + `FilterChip` + `FilterPopover` + `AddFilterMenu` +
5 editors (Text / NumberRange / MultiSelect / EnumGrid / Tag) +
@@ -337,11 +421,20 @@ variant — deferred with the music module.
`StateBlock` (the CLAUDE.md mandatory loading/error/empty three-state in
one place). Pages-v2 compose a template instead of hand-rolling layout.
**Tier-4 — Deferred (migrate with owning page):** Timetable suite
**Tier-4 — Deferred (migrate with owning page):** the Timetable suite
(`TimetableGrid` 947, `TimetableTab` 687, `TimetableModals`,
`ParkingColumn`, `TimetablePopover`), Music/Cue suite
`ParkingColumn`, `TimetablePopover`) and Music/Cue suite
(`CueTimelineEditor` 889, `SectionBuilderPro` 2084, `SongDrawerBody`,
`SongFilesPanel`). `DraggableBlock` is the one primitive extracted early.
`SongFilesPanel`) — these large feature *modules and their pages* are
deferred.
**`DraggableBlock` is explicitly NOT deferred (issue 6 resolved).** It is
a **foundation-sprint deliverable** (built in §9 deliverable 3, listed
in `components-v2/shared/`, Storybook stories required early per the
original brief). Tier-4 defers the Timetable/Cue *pages*, not this
shared primitive. It is non-trivial (pointer drag, 3 density modes,
scoped CSS) and intentionally built first precisely because two future
features depend on it being stable and visually locked.
**Reconciliation:** v2 uses crewli-starter-derived versions; existing
`AppKpiCard` / `AppSearchHeader` / `AppLoadingIndicator` / `ErrorHeader`
@@ -358,7 +451,9 @@ direction.
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).
3. Tier-1 primitives (PrimeVue-based per §8) **plus `DraggableBlock`**
(custom, §7.1 — foundation despite Tier-4 deferring the
Timetable/Cue pages; see §8).
4. Template layer: List / Form / Detail / Dashboard / StateBlock.
5. Storybook: global theme/density toolbar; story tree; stories for
every Tier-1 primitive + 4 shell components + each template +
@@ -385,8 +480,10 @@ direction.
shell).
- **Domain modules** migrate with their owning page.
- **Final cutover:** rename `pages-v2/``pages/`,
`components-v2/``components/`, drop `*V2` suffixes, delete dead v1
shell + Vuetify, one closure commit.
`components-v2/``components/`, drop `*V2` suffixes, **strip the `v2-`
route-name prefix and revert the `routesFolder`/`getRouteName` change
(§3)**, collapse the boundaries zones (§14), delete dead v1 shell +
Vuetify, one closure commit.
## 11. Quality gates (per sprint)
@@ -407,15 +504,37 @@ must not regress.
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.
**Portal scope (explicit decision — audited 2026-05-15, issue 2
resolved).** The review flagged a feared cross-doc `/p/*` vs `/portal/*`
conflict touching observability + API. **Audit dissolves it — there is
no conflict, because two different layers were being conflated:**
- **Frontend SPA route prefix = `/portal/*`** (verified in the repo, not
just claimed by a doc): `src/pages/portal/{advance,evenementen,
profiel,registreren,shifts,…}`; router tests assert
`/portal/evenementen`, `/portal/advance/:token`, `/portal/profiel`.
This is the layer the v2 redesign touches.
- **Backend API path = `/api/v1/p/...`** (e.g.
`/api/v1/p/artist/{token}` in RFC-TIMETABLE). This is a **different
layer**; it already coexists with the `/portal/*` SPA routes today
and is **out of scope** (this redesign is frontend-only, §12).
- **Frontend observability is NOT path-prefix bound.**
`src/observability/contextBinding.ts:51` keys on
`route.meta.public === true && route.meta.context === 'portal'`
**route meta, not the URL path**. Moving/prefixing SPA routes does not
affect Sentry actor-scope tagging as long as v2 portal routes carry
the same `meta.context: 'portal'` (binding requirement, added to the
portal sub-sprint). RFC-WS-7-OBSERVABILITY's `/p/*` prose describes
the backend; no observability-config change is triggered by this
redesign, and **no cross-doc commit is required.**
**Decision:** portal is **in scope** (crewli-starter ships
`PortalPage`/`PortalVolunteerPage`/`PortalCustomerPage`/
`PortalVolunteerFormPage`), as a dedicated **later sprint** (not
foundation) with its own `PortalLayoutV2` (token-auth chrome differs
from the organizer shell). During parallel mode it lives at
`/v2/portal/*` **with `meta.context: 'portal'` preserved**; cutover
folds it to `/portal/*`. The backend `/api/v1/p/*` layer is untouched.
## 13. Testing strategy (audited 2026-05-15)
@@ -444,6 +563,21 @@ review asked for, made explicitly: keep the Playwright gate, add
Storybook as the authoring/review surface, re-baseline visuals against
crewli-starter.
**Storybook ↔ Playwright CT interplay (issue 10 — audited, matches
TEST-INFRA-001 infra).** Verified: `playwright-ct.config.ts` uses
`testDir: './tests/playwright-ct'`, `testMatch: /.*\.spec\.ts$/`, and
the existing CT suite does **not** mount Storybook stories (no
`.stories` reference in the CT config or specs). **Decision (consistent
with the established infra):** the two surfaces are **independent and
share only the component under test** — CT tests are standalone
`tests/playwright-ct/**/*.spec.ts` that `mount()` the component
directly; Storybook stories are separate authoring/QA artifacts. v2
follows this exactly: do **not** wire Playwright to render Storybook
stories (no `@storybook/test-runner`); a component has both a
`.stories.ts` (authoring) and a `tests/playwright-ct/.../*.spec.ts`
(gate), each owning its own fixtures. Rationale: matches TEST-INFRA-001,
avoids coupling the CI gate to Storybook's build.
## 14. eslint-plugin-boundaries extension (audited 2026-05-15)
My original §5 claim ("matrix applies identically") was **factually
@@ -460,6 +594,16 @@ uses explicit, order-sensitive path patterns; none match
`{ type: 'components-foundation', pattern: 'src/components/{forms/**,Icon.vue}' }`
(FormField + Icon — audited to live in the generic `components`
zone today, not `components-shared`).
**Issue 9 — brace-expansion caveat:** `eslint-plugin-boundaries`
resolves `pattern` via micromatch; single-level brace expansion is
supported in current versions, but deliverable 1 must **verify it
against the project's installed `eslint-plugin-boundaries` version**
(currently `6.0.2`) with a one-line test. **Safe fallback if brace
expansion misbehaves:** declare the zone as two explicit element
entries instead — `{ type: 'components-foundation', pattern:
'src/components/forms/**' }` and `{ type: 'components-foundation',
pattern: 'src/components/Icon.vue' }` (same `type`, two patterns is
valid and unambiguous).
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'] }`