diff --git a/dev-docs/superpowers/specs/2026-05-15-crewli-starter-gui-redesign-design.md b/dev-docs/superpowers/specs/2026-05-15-crewli-starter-gui-redesign-design.md index e4d0bfca..2e25cca9 100644 --- a/dev-docs/superpowers/specs/2026-05-15-crewli-starter-gui-redesign-design.md +++ b/dev-docs/superpowers/specs/2026-05-15-crewli-starter-gui-redesign-design.md @@ -2,7 +2,7 @@ | Field | Value | |---|---| -| **Status** | Design approved; spec review round 1 corrections applied — pending re-approval | +| **Status** | Design approved; spec review rounds 1–2 corrections applied — pending re-approval | | **Date** | 2026-05-15 | | **Author** | Brainstorming session (Bert + Claude Code) | | **Supersedes** | The F4a–F4d 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 `` / `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: '' } })` `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', …)`. - `` / `` 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'] }`