diff --git a/.claude/skills/running-the-stack/SKILL.md b/.claude/skills/running-the-stack/SKILL.md new file mode 100644 index 00000000..dced7920 --- /dev/null +++ b/.claude/skills/running-the-stack/SKILL.md @@ -0,0 +1,42 @@ +--- +name: running-the-stack +description: Use when launching, restarting, or smoke-testing simple_module_python locally — the FastAPI host API (uvicorn) and the Vite dev server for the Inertia/React client — or when a local run fails to start, a port is taken, or login fails. +--- + +# Running simple_module_python + +## Prerequisites (once per checkout / worktree) +- `uv sync --all-packages && npm install && make gen-pages` — a fresh worktree has no `.venv`, no `node_modules` and no generated pages; skipping this fakes unrelated lint/test failures. +- Postgres/Redis are the SHARED stack in `~/Repos/dev-services` (`make docker-up` starts it). Never start your own containers. Default DB is SQLite, so a plain local run needs neither. +- No `.env` is required. If one exists, read it (don't copy values) — `.env` beats process env for `SM_VITE_DEV_URL`. + +## Launch (verified 2026-10-09, SQLite, spare ports 8201/5201) +| Step | Command | +|---|---| +| Check ports free | `ss -ltn \| grep -E ':(8201\|5201) '` (empty = free) | +| DB + migrations | `export SM_DATABASE_URL=sqlite+aiosqlite:///./dev-tmp.db` then `uv run --project host alembic -c host/alembic.ini upgrade heads` | +| Admin login | `uv run smpy users create-admin --email admin@example.com --password --force` | +| Frontend (background) | `SM_VITE_PORT=5201 npm run dev` | +| Backend (background) | `SM_DATABASE_URL=sqlite+aiosqlite:///./dev-tmp.db SM_VITE_DEV_URL=http://localhost:5201 uv run --project host uvicorn host.main:app --port 8201` | +| Everything on default ports | `make dev` — API :8000 + Vite :5050, also runs `docker-up` + `gen-pages` *(unverified this session)* | + +## Ready check +- `curl -fsS http://localhost:8201/health` → 200 (about 20 s after start). +- App: `http://localhost:8201/` · sign in at `/users/login` with the admin you created · admin area `/admin`. +- Anonymous `curl` of an app page 302s to the login page; that is expected. + +## Stop / reset +- Kill every PID listening on your ports: `ss -ltnp | grep -E ':(8201|5201) '` then `kill …`. `uv run … uvicorn` spawns a child, and a `--workers N` server leaves workers bound if you only kill one PID. +- `command rm -f dev-tmp.db* < /dev/null`. + +## Gotchas +- Ports 8000/5050 are often held by OTHER projects on this machine (a foreign Vite will hydrate this HTML with the wrong bundle). Use spare ports; check `/proc//cwd` before killing anything you didn't start. +- Never `pkill -f "uvicorn … --port N"` — the pattern matches your own shell. `make kill` runs `pkill -f vite`, which also kills other projects' Vite servers; prefer killing by port. +- A wrong `E2E_PASSWORD` trips the login rate limiter (5 failures / 300 s) and cascades into 429 timeouts. +- `cp`/`rm` are interactive aliases in this shell: use `command cp -f` / `command rm -f … < /dev/null`. + +## Tests +- Unit: `make test-py` (move any `.env` aside first — a dashboard test asserts Vite on :5050) · `make test-js` · single: `uv run pytest path::name`, `npx vitest run `. +- Postgres: `SM_TEST_DATABASE_URL=postgresql+asyncpg://postgres:postgres@localhost:5432/sm_test uv run pytest -p no:anyio `. +- E2E (server running): `uv run playwright install chromium` once, then `E2E_BASE_URL=http://localhost:8201 E2E_USERNAME=admin@example.com E2E_PASSWORD= uv run pytest -m e2e tests/e2e -v`. See `docs/e2e-testing.md`. +- Gate before a PR: `make lint` (also format-checks Python in `.md` — run `uv run ruff format docs/` first). diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c80f5d4..8fa24c44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,19 @@ All notable changes to this project are documented in this file. The format is b ## [Unreleased] ### Added +- **Host override layer for module copy** (#415). A host ships + `host/locales/overrides/.json` (nested or flat dotted keys) and it is + applied after every module, framework and host catalog, so the server-side + `Translator`, menus and Inertia props all see it. An override only replaces an + existing key: an unknown key is skipped with a warning and reported by + `make doctor`. Admin-only keys stay admin-only. +- **Public tenancy API** (#418): `simple_module_hosting.tenancy` exposes + `tenancy_mode(app)`, `single_tenant_id(app)`, `require_tenant(...)` (binds + the single-tenant id, or the request's tenant else `on_missing`) and + `tenant_vary(request)`; `TenancyMode` lives in `simple_module_core.tenancy`. + Modules no longer need to read the middleware stack. +- `NativeSelect` takes a `wrapperClassName` for sizing the control (#421). + `className` still targets the ``. Width utilities belong on + * `wrapperClassName`: the select is `w-full` of a `w-fit` wrapper, so a width + * on the select alone cannot widen it (#421). + */ +function NativeSelect({ + className, + wrapperClassName, + size = 'default', + ...props +}: Omit, 'size'> & { + size?: 'sm' | 'default'; + wrapperClassName?: string; +}) { + return ( +
+``` + +- [ ] **Step 4: Run** the test again. Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add packages/ui/src/components/ui/native-select.tsx packages/ui/src/components/ui/native-select.test.tsx +git commit -m "feat(ui): NativeSelect wrapperClassName so callers can size the control (#421)" +``` + +--- + +### Task 11: Admin titles, settings h1, admin errors inside the shell (#422) + +**Independent of the others, but adds locale keys, so it regenerates i18n.** + +**Files:** +- Modify: `modules/users/users/pages/Users/Index.tsx` (add ``) +- Modify: every other page assigned `.layout = [AdminLayout]` that lacks ``) +- Modify: `modules/settings/settings/locales/*.json` (add `settings.modules_edit.heading`, or reuse an existing title key if one already names the screen, e.g. `settings.*.title`; check first) +- Modify: `packages/ui/src/components/ErrorScreen.tsx` (add `inline?: boolean`) +- Modify: `host/client_app/pages/Error.tsx` (function layout) +- Create: `host/client_app/pages/ErrorShell.tsx` (keeps `Error.tsx` under the cap) +- Create: `host/client_app/admin-pages-head.test.ts` (guard) +- Modify: `host/client_app/pages/Error.test.tsx` +- Modify: `tests/e2e/test_document_titles.py` (add `/admin/users/`), `tests/e2e/test_error_pages.py` (an admin 404 inside the shell) +- Regenerate: `packages/i18n/src/keys.generated.ts`, `packages/i18n/src/generated-resources.ts` + +**Interfaces:** +- Produces: `ErrorScreen` prop `inline?: boolean`. Default `false` keeps today's full-viewport frame; `true` drops `min-h-screen` and the page background. + +- [ ] **Step 1: Write the failing tests** + +`host/client_app/admin-pages-head.test.ts`: + +```ts +import { readdirSync, readFileSync, statSync } from 'node:fs'; +import { join } from 'node:path'; + +/** Every page rendered in AdminLayout sets a document title (#422: /admin/users/ had none). */ +function pagesUnder(dir: string): string[] { + return readdirSync(dir).flatMap((name) => { + const path = join(dir, name); + if (statSync(path).isDirectory()) return pagesUnder(path); + return name.endsWith('.tsx') && !name.endsWith('.test.tsx') ? [path] : []; + }); +} + +const ROOT = join(__dirname, '..', '..'); +const moduleDirs = readdirSync(join(ROOT, 'modules')).map((m) => join(ROOT, 'modules', m, m, 'pages')); +const files = moduleDirs + .filter((d) => { try { return statSync(d).isDirectory(); } catch { return false; } }) + .flatMap(pagesUnder) + .filter((f) => /\.layout\s*=\s*\[\s*AdminLayout/.test(readFileSync(f, 'utf8'))); + +describe('admin pages', () => { + it('found some', () => expect(files.length).toBeGreaterThan(5)); + it.each(files)('%s renders ', (file) => { + expect(readFileSync(file, 'utf8')).toMatch(/]*title=/); + }); +}); +``` + +`Error.test.tsx`: add cases using its existing `usePage` mock. Make the mocked page `url` and props configurable per test. + +```tsx +it('renders inside AdminLayout for a signed-in admin under /admin', () => { /* url '/admin/users/999999', auth.isAuthenticated true, adminSidebar non-empty → expect the admin nav landmark / AdminLayout marker */ }); +it('renders bare for an anonymous 401 under /admin', () => { /* url '/admin/users/', auth.isAuthenticated false → no AdminLayout */ }); +it('renders bare outside /admin', () => { /* url '/administer' → no AdminLayout (segment-aware) */ }); +it('renders bare when adminSidebar is empty', () => { /* signed-in non-admin 403 */ }); +``` + +Write each case fully. Detect `AdminLayout` the way existing layout tests do (e.g. mock `@simple-module-py/ui` `AdminLayout` as `({children}) =>
{children}
`). + +`ModulesEdit`: a vitest test that renders the page with minimal props (follow any existing `ModulesEdit` or settings page test; if none exists, mock `usePage` the way other page tests do) and asserts `getByRole('heading', { level: 1 })`. + +- [ ] **Step 2: Run to confirm failures** + +Run: `npx vitest run host/client_app/admin-pages-head.test.ts host/client_app/pages/Error.test.tsx` +Expected: the guard fails for `Users/Index.tsx` (and any other offenders); the Error admin cases fail. + +- [ ] **Step 3: Implement** + +`Users/Index.tsx`: `import { Head } from '@inertiajs/react';` and render `` as the first child of the returned fragment. Wrap in `<>…` if the page returns a single `PageShell`. Do the same for every other offender the guard lists, each with its existing title key. + +`ModulesEdit.tsx`: add an `

` that names the screen, styled like `PageShell`'s heading (`PageShell.tsx:80`), placed at the top of the main pane. Use a translation key; add `settings.modules_edit.heading` to every `modules/settings/settings/locales/.json` if no suitable key exists. + +`ErrorScreen.tsx`: + +```tsx + /** Inside an app shell: no full-viewport frame or page background (#422). */ + inline?: boolean; +... +
+``` + +`ErrorShell.tsx`: + +```tsx +import { usePage } from '@inertiajs/react'; +import { AdminLayout } from '@simple-module-py/ui'; +import type { SharedProps } from '@simple-module-py/ui/types'; +import { createContext, type ReactNode, useContext } from 'react'; + +/** `/admin` or anything under `/admin/` — not `/administer`. */ +export function isAdminPath(url: string): boolean { + const path = url.split(/[?#]/, 1)[0]; + return path === '/admin' || path.startsWith('/admin/'); +} + +const InShell = createContext(false); +export const useInAdminShell = () => useContext(InShell); + +/** + * Keeps an admin inside the admin shell when a page under /admin fails (#422). + * Only for a signed-in viewer whose admin menu reached the page: an anonymous + * 401/419 has no admin to keep in context, and a 500 raised before the shared + * props were built has no menu to render. + */ +export function ErrorShell({ children }: { children: ReactNode }) { + const page = usePage(); + const props = page.props as unknown as Partial & { adminSidebar?: unknown[] }; + const inShell = + isAdminPath(page.url) && + Boolean(props.auth?.isAuthenticated) && + Array.isArray(props.adminSidebar) && + props.adminSidebar.length > 0; + if (!inShell) return <>{children}; + return ( + + {children} + + ); +} +``` + +Check where `adminSidebar` actually lives in the shared props: `menus.adminSidebar` or top-level. Grep `adminSidebar` in `packages/ui/src` and `host/client_app/pages/Admin.tsx`, and use the same path. + +`Error.tsx`: `ErrorPage.layout = (page: ReactNode) => {page};` and pass `inline={useInAdminShell()}` to `ErrorScreen`. + +Regenerate i18n: `uv run --project host python scripts/gen_i18n.py`. + +- [ ] **Step 4: Run tests** + +Run: `npx vitest run host/client_app packages/ui modules/settings modules/users` +Expected: PASS. + +Then extend the e2e files: +- `tests/e2e/test_document_titles.py`: add `("/admin/users/", "Users")` to `_TITLED_PAGES`. +- `tests/e2e/test_error_pages.py`: add a test that, signed in as admin, visits `/admin/users/00000000-0000-0000-0000-000000000000` (or the module's real id format), expects the 404 copy, and expects the admin sidebar navigation to be present. Follow the file's existing login helper. + +These run in the integration task against a live server. + +- [ ] **Step 5: Commit** + +```bash +git add modules/users/users/pages modules/settings/settings packages/ui/src/components/ErrorScreen.tsx host/client_app packages/i18n/src tests/e2e +git commit -m "fix(admin): page titles, settings h1, admin errors inside the admin shell (#422)" +``` + +--- + +### Task 12: Integration — full gates, changelog, live checks + +**Sequential, last. Done by the lead (or a sonnet agent), not in parallel with anything.** + +- [ ] **Step 1:** `uv run --project host python scripts/gen_i18n.py`. Confirm the generated keys are committed and every installed namespace is present. +- [ ] **Step 2:** `make lint`. Expected exit 0. Fix anything (e.g. `ruff format docs/` for Python snippets in the new docs). +- [ ] **Step 3:** Run the full Python suite with the worktree's `.env` moved aside: `uv run pytest -q`. Expected: 0 failures. +- [ ] **Step 4:** Postgres: + + `SM_TEST_DATABASE_URL=postgresql+asyncpg://postgres:postgres@localhost:5432/sm_test uv run pytest -p no:anyio framework/db/tests framework/hosting/tests/test_tenancy_api.py modules/tenants/tests modules/background_tasks/tests modules/users/tests -q` + + Then `SM_MIGRATIONS_PG_URL=postgresql+asyncpg://postgres:postgres@localhost:5432/sm_migrations make migrations-roundtrip-pg` (create the DB first). Expected: green. +- [ ] **Step 5:** `npm test`. Expected: green. +- [ ] **Step 6:** Start the app on spare ports and run the e2e files touched in Task 11: `tests/e2e/test_document_titles.py tests/e2e/test_error_pages.py`. Follow `docs/e2e-testing.md` and the memory notes on ports and `E2E_PASSWORD`. If no `running-the-stack` skill exists, run `/runbook` once the app is up, and commit the skill. +- [ ] **Step 7:** `CHANGELOG.md` `[Unreleased]`: one entry per issue under Added (#415, #418, #421), Fixed (#413 pinned, #414, #416, #417, #419, #420, #422, #423, #424), and Changed (Vary: Cookie/Authorization). Commit as `docs: changelog for #413–#424`. + +--- + +## Self-review notes + +- **Spec coverage:** every row of the spec's scope table maps to a task (413→5, 414/420→9, 415→7, 416→6, 417→4, 418→1+2+3, 419→8, 421→10, 422→11, 423/424→1). +- **Deviations recorded:** + - #413 needs no model change: it is already fixed on `main` by #406, unreleased. + - #416 falls back to recording when no fetch metadata is present, rather than requiring `Accept: text/html`, so existing clients and tests keep working. +- **Type consistency:** `tenant_vary` is `tuple[str, ...]` in both Task 1 and Task 2. `TenancyMode` is defined once (core), re-exported by hosting. `ErrorScreen.inline` is used only by `Error.tsx`. diff --git a/docs/superpowers/specs/2026-10-09-issue-batch-413-424-design.md b/docs/superpowers/specs/2026-10-09-issue-batch-413-424-design.md new file mode 100644 index 00000000..1b751679 --- /dev/null +++ b/docs/superpowers/specs/2026-10-09-issue-batch-413-424-design.md @@ -0,0 +1,296 @@ +# Issue batch #413–#424 — design + +**Date:** 2026-10-09 · **Status:** approved · **Branch:** `feat/issue-batch-413-424` · **One combined PR** + +Twelve open issues filed on 2026-10-08, fixed together. #317 (re-verify the 28 +hi-fi screens) is out: it needs the external design file, which neither the repo +nor this session can read. + +## Scope + +| # | Area | One-line fix | +|---|---|---| +| 413 | background_tasks | Admin page 500s on Postgres: four timestamp columns declared naive in the model | +| 414 | ui | Sign-in aside footer token below WCAG AA | +| 415 | i18n | Host override layer for module copy | +| 416 | auth | Only top-level navigations record the post-login target | +| 417 | db | Tenant filter turns an outer join into an inner join | +| 418 | hosting | Public tenancy API: mode, single-tenant id, `require_tenant`, complete `Vary` | +| 419 | hosting | gen-pages `@source` reaches subdirectories of wheel modules | +| 420 | ui | Brand foreground ink follows the brand colour; sidebar active row uses it | +| 421 | ui | `NativeSelect` gets a `wrapperClassName` | +| 422 | admin | `/admin/users/` title, `/admin/settings/` h1, admin errors inside the admin shell | +| 423 | hosting | `TenantMiddleware` merges every `Vary` line and never drops `*` | +| 424 | hosting | `tenant_source` is `None` whenever no tenant is bound | + +Out of scope: #317; the optional route-walking test helper floated in #418; the +branding-settings alternative for #415. + +## Design + +### #413 — naive timestamps in `TaskExecution` + +`queued_at`, `started_at`, `finished_at` and `heartbeat_at` +(`modules/background_tasks/background_tasks/models.py`) are bare +`datetime | None`, so SQLModel maps them to `timestamp without time zone`. +Every writer and comparer already uses aware UTC (`now_utc()`, +`datetime.now(UTC)`), and asyncpg refuses an aware parameter against a naive +column. + +The schema side is already done: migration `e5f2a8c1d7b3_timestamps_timezone_aware` +retypes these four columns, and `users_refresh_token`'s, to `timestamptz` on +Postgres. Only the model is wrong. Fix: `Field(default=None, sa_type=DateTime(timezone=True))` +on the four fields. **No new migration.** + +The same pass covers the model of every column that migration retyped +(`users_refresh_token.created_at/expires_at/revoked_at`), so model and schema +agree everywhere `alembic check` looks. The other bare-`datetime` fields the +audit turned up (`user_role.assigned_at`, `permissions`, `audit_log`) are +checked against their migrated column types. If a column is still naive in the +schema, it stays as it is: changing it needs a migration and has no reported bug +behind it. + +### #414 — aside footer contrast + +`--color-dark-text-subtle` goes from `oklch(0.45 0.01 250)` (2.7:1 on the aside) +to `oklch(0.62 0.01 250)`. That clears 4.5:1 and stays a shade lighter than +`--color-dark-text-muted` (0.6), so the two tokens remain distinct. The footer is +its only consumer. + +### #415 — host override layer + +A host ships `host/locales/overrides/.json`, keyed by full dotted path, for +example `{"users": {"login": {"aside_heading": "…"}}}` or the flat +`{"users.login.aside_heading": "…"}`. The overrides are applied after every module, +framework and host source. + +- `I18nRegistry.add_overrides(dir)` applies them at the end of `load()`, before + locale layering and snapshots, so the server-side `Translator`, menus and + Inertia shared props all see them. +- **An override may only replace an existing key.** An unknown key is skipped + with a warning and reported by `make doctor` as a new locale diagnostic. A + typo can't silently invent a key, and `keys.generated.ts` doesn't churn. +- **Audience is preserved.** An override of a key that is only in the admin + catalog stays admin-only. The public snapshot is updated only for keys it + already holds, so the #248 split cannot leak. +- `build_i18n_registry` registers `host/locales/overrides` when it exists. + Documented in the i18n docs next to the `host` namespace. + +### #416 — post-login target + +`AuthMiddleware` keeps redirecting every unauthenticated, non-public request +exactly as today. It writes `SESSION_NEXT_KEY`, and passes a `next` to the +provider, only for a top-level navigation: + +1. If `Sec-Fetch-Dest` is present, the request must be `document`. +2. Else, if `Sec-Fetch-Mode` is present, it must be `navigate`. +3. Else, `Accept` must include `text/html`. +4. **And always** an Inertia visit (`X-Inertia: true`), so a session that + expires mid-navigation still returns the user to the page they clicked. + +A favicon, script, image or `fetch()` no longer overwrites the target. The +favicon half of the issue is already fixed on `main` (`branding_head()` falls +back to a data-URI icon). Only the scaffold template is re-checked. + +### #417 — tenant filter and outer joins + +A table referenced only inside a function (`func.count(Child.id)`) loses its ORM +annotation. `_plain_tables` then treats it as a Core table and puts +`child.tenant_id = :t` in `WHERE`, which defeats the outer join. + +The obvious fix is to drop every outer-join target from the `WHERE` candidates, +and it is a tenant leak in one shape. When the target is a raw +`Model.__table__`, nothing puts its predicate in the `ON` clause: +`with_loader_criteria` only covers ORM entities. With the `WHERE` predicate gone, +another tenant's child rows would match the `ON` and inflate the count. So: + +- For each outer-join entry in `stmt._setup_joins` whose **target is an ORM + entity** (its loader criteria land in `ON`), exclude that entity's table from + `_plain_tables`. Full outer joins are treated the same way. +- **A raw-table outer-join target keeps today's `WHERE` predicate.** That + over-filters but is never a leak. Moving it into `ON` would need the statement's + join rewritten, which is outside this fix. +- The soft-delete path (`_soft_delete_criteria`) uses the same helper and gets the + same fix. + +Tests, in `framework/db/tests/test_query_filters.py`, with a new +tenant-scoped parent/child pair: + +- The issue's four shapes keep the unused parent with count 0. +- A second tenant's child rows never appear in any count. This is the leak guard. +- An inner join through `_setup_joins` still filters the child. +- A raw-table outer join stays filtered. +- The soft-delete variant. + +This task goes to an `opus` implementer plus a separate security review. + +### #418 — public tenancy API + +New public module `simple_module_hosting.tenancy`. Modules import from it instead +of reading the middleware stack. + +| API | Behaviour | +|---|---| +| `TenancyMode` | `StrEnum` `SINGLE` / `MULTI`, in `simple_module_core.tenancy` so core and modules can name it without importing hosting. | +| `tenancy_mode(app)` | Read from `app.state.sm.tenancy`, a new defaulted field on `Services` that the app builder sets where it decides on `TenantMiddleware`: `MULTI` iff `multi_tenant`. | +| `single_tenant_id(app)` | `app.state.sm.db.default_tenant_id`: the host's `default_tenant`, else `DEFAULT_TENANT_ID`. | +| `require_tenant(*, on_missing=403, detail="tenant_required")` | Async yield-dependency factory. `SINGLE` binds `single_tenant_id(app)`; `MULTI` takes `request.state.tenant_id`, or raises `on_missing`, which is a status code or a `Callable[[Request], Exception]` (a public surface passes its own 404). It enters `tenant_context(tenant)` for the rest of the request and yields the tenant id. Documented to be listed before `get_db`, and verified by a test that a write commits under the bound tenant. | +| `tenant_vary(request)` | The headers the resolved tenant depended on. `TenantMiddleware` now records `request.state.tenant_vary`. Meant for a route that builds its own cache headers before the middleware sees the response, e.g. a 304. | + +`Vary` completeness, as chosen at the question batch: + +- The tenants resolver adds `Cookie` when the source is `"session"`. +- The no-resolver `"claim"` path adds `Cookie` and `Authorization`, since the claim + came from whichever credential authenticated the request. + +Docs: the multi-tenancy guide gains a "For module authors" section with the four +calls and the dependency-order rule. + +### #419 — `@source` for wheel-module subdirectories + +uv writes `.venv/.gitignore` = `*`, and Tailwind's scanner honours ignore files +below an `@source` base, so only the base directory's own files are scanned. + +`render_modules_css()` (`framework/hosting/simple_module_hosting/assets.py`) keeps +the base line for an out-of-repo module and adds one `@source "/**/*.{ts,tsx}"` +for every descendant directory that directly holds a `.ts`/`.tsx` file: + +- skipping `__pycache__` and `node_modules`; +- de-duplicated and sorted, so the output is deterministic; +- tolerant of a directory that doesn't exist, as today. + +In-repo modules still get no lines. + +### #420 — brand foreground ink + +`deriveBrandRamp` (`packages/ui/src/lib/color.ts`) also sets +`--primary-foreground` and `--sidebar-primary-foreground`. It picks by **WCAG +contrast ratio**, not a fixed lightness threshold: white if white reaches 4.5:1 on +the brand colour, otherwise a dark ink (`oklch(0.2 0.02 250)`); if neither +reaches 4.5:1, whichever is higher. + +- That gives dark ink on `#62B8E2` and `#9AD3EF`, and white on `#2E6DB0` and + `#16276E`, matching the issue's measurements. +- `BrandingHead` already applies and clears every key the ramp returns, so it + needs no change. +- `sidebar-theme.ts` `activeClass` becomes `bg-primary text-primary-foreground`. +- `BrandingMark`'s gradient badge is checked for the same light-brand problem and + fixed the same way if it has it. + +### #421 — `NativeSelect` width + +New `wrapperClassName` prop, merged with `cn()` onto +`[data-slot=native-select-wrapper]`. `className` keeps targeting the ` onChange(e.target.value)} + className="h-9 w-9 shrink-0 cursor-pointer rounded-[9px] border bg-transparent" + /> + onChange(e.target.value)} + onBlur={() => { + const normal = normalizeHex(value); + if (normal && normal !== value) onChange(normal); + }} + className="min-w-0 flex-1 font-mono" + /> +
+ {valid ? null : ( + + )} +

+ ); +} diff --git a/modules/branding/branding/components/PreviewSurfaces.tsx b/modules/branding/branding/components/PreviewSurfaces.tsx index 538979c8..d353685f 100644 --- a/modules/branding/branding/components/PreviewSurfaces.tsx +++ b/modules/branding/branding/components/PreviewSurfaces.tsx @@ -1,5 +1,5 @@ import { keys, useT } from '@simple-module-py/i18n'; -import { type PreviewBrand, previewFooterLinks } from './BrandingPreview'; +import { type PreviewBrand, previewFooterLinks, previewInk } from './BrandingPreview'; function Logo({ brand }: { brand: PreviewBrand }) { return brand.logoUrl ? ( @@ -38,8 +38,8 @@ export function SignInPreview({ brand }: { brand: PreviewBrand }) {
{t(keys.branding.manage.preview_signin_action)}
@@ -63,11 +63,9 @@ export function EmailPreview({ brand }: { brand: PreviewBrand }) {
- - {brand.appName} - + {brand.appName}
@@ -76,8 +74,8 @@ export function EmailPreview({ brand }: { brand: PreviewBrand }) {
{t(keys.branding.manage.preview_email_action)}
diff --git a/modules/branding/branding/components/hex.ts b/modules/branding/branding/components/hex.ts new file mode 100644 index 00000000..33a10315 --- /dev/null +++ b/modules/branding/branding/components/hex.ts @@ -0,0 +1,19 @@ +const HEX = /^#?([0-9a-f]{3}|[0-9a-f]{6})$/i; + +/** + * Normalise what an admin typed into `#rrggbb`, or `null` when it is not a + * 3- or 6-digit hex colour. The leading `#` is optional; case is folded. + * The server only accepts `#rrggbb`, so 3-digit shorthand is expanded here. + */ +export function normalizeHex(raw: string): string | null { + const match = HEX.exec(raw.trim()); + if (!match) return null; + let digits = match[1].toLowerCase(); + if (digits.length === 3) digits = [...digits].map((d) => d + d).join(''); + return `#${digits}`; +} + +/** Empty means "use the theme default", which is a valid stored value. */ +export function isValidColor(raw: string): boolean { + return raw.trim() === '' || normalizeHex(raw) !== null; +} diff --git a/modules/branding/branding/locales/en.json b/modules/branding/branding/locales/en.json index de85cdc6..9762d25d 100644 --- a/modules/branding/branding/locales/en.json +++ b/modules/branding/branding/locales/en.json @@ -9,6 +9,7 @@ "publishing": "Publishing…", "app_name_label": "App name", "primary_color_label": "Primary colour", + "primary_color_invalid": "Enter a hex colour such as #0f766e or #0fe.", "design_pack_label": "Design pack", "design_pack_inline": "Design pack: {name}", "design_pack_none": "None (base tokens)", diff --git a/modules/branding/branding/pages/Manage.tsx b/modules/branding/branding/pages/Manage.tsx index 56f1aad0..26768f56 100644 --- a/modules/branding/branding/pages/Manage.tsx +++ b/modules/branding/branding/pages/Manage.tsx @@ -10,9 +10,11 @@ import type { SharedProps } from '@simple-module-py/ui/types'; import { useMemo, useState } from 'react'; import { toast } from 'sonner'; import { BannerField, type BannerSeverity } from '../components/BannerField'; +import { ColorField } from '../components/ColorField'; import { DesignPackField, type DesignPackOption } from '../components/DesignPackField'; import { type BrandingForm, countBrandingChanges } from '../components/dirty'; import { FooterField } from '../components/FooterField'; +import { isValidColor, normalizeHex } from '../components/hex'; import { ImageDropzones, type ImageKind } from '../components/ImageDropzones'; import { PresetField, type PresetOption } from '../components/PresetField'; import { PreviewTabs } from '../components/PreviewTabs'; @@ -65,6 +67,8 @@ function Manage() { const [busy, setBusy] = useState(false); const changes = countBrandingChanges(form, baseline); const locked = !canManage || busy; + const colorValid = isValidColor(form.color); + const accent = normalizeHex(form.color) ?? DEFAULT_SWATCH; const set = (patch: Partial) => setForm((current) => ({ ...current, ...patch })); @@ -90,7 +94,7 @@ function Manage() { headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ app_name: form.appName, - primary_color: form.color, + primary_color: normalizeHex(form.color) ?? '', design_pack: form.designPack, banner_message: form.bannerMessage, banner_severity: form.bannerSeverity, @@ -144,7 +148,7 @@ function Manage() {
-
- -
- set({ color: e.target.value })} - className="h-9 w-9 shrink-0 cursor-pointer rounded-[9px] border bg-transparent" - /> - set({ color: e.target.value })} - className="min-w-0 flex-1 font-mono" - /> -
-
+ set({ color })} + />
{/* The *effective* colour, not the stored one: with nothing set @@ -204,7 +188,7 @@ function Manage() { chip is active claims the opposite. */} set({ color: swatch })} disabled={locked} /> @@ -251,7 +235,7 @@ function Manage() { ; +} + +const BRAND: PreviewBrand = { + appName: 'Acme', + accent: '#f5f5f5', + logoUrl: null, + logoDarkUrl: null, + bannerMessage: '', + footerText: '', + footerLinks: [], + menuLabels: [], +}; + +describe('normalizeHex', () => { + test('accepts 3/6 digits, optional hash, any case', () => { + expect(normalizeHex('F5F5F5')).toBe('#f5f5f5'); + expect(normalizeHex('#ABC')).toBe('#aabbcc'); + expect(normalizeHex(' #0f766e ')).toBe('#0f766e'); + }); + + test('rejects invalid values', () => { + for (const bad of ['#zzz', '12345', 'red', '#12345g', '#1234']) { + expect(normalizeHex(bad)).toBeNull(); + expect(isValidColor(bad)).toBe(false); + } + expect(isValidColor('')).toBe(true); + }); +}); + +describe('ColorField', () => { + test('flags an invalid value and keeps the swatch valid', () => { + const { container } = render(); + fireEvent.change(screen.getByRole('textbox'), { target: { value: '#zzz' } }); + expect(screen.getByRole('alert')).toHaveTextContent('Enter a hex colour'); + expect(container.querySelector('input[type="color"]')).toHaveValue('#0f766e'); + }); + + test('normalises a bare hex value on blur', () => { + render(); + const input = screen.getByRole('textbox'); + fireEvent.change(input, { target: { value: 'F5F5F5' } }); + expect(screen.queryByRole('alert')).toBeNull(); + fireEvent.blur(input); + expect(input).toHaveValue('#f5f5f5'); + }); +}); + +describe('preview ink', () => { + test('light brand colour gets dark ink, dark one gets white', () => { + expect(previewInk('#f5f5f5')).not.toBe(previewInk('#0f766e')); + expect(previewInk('#0f766e')).toBe('oklch(1 0 0)'); + }); + + test('sign-in button uses the derived ink, not hard-coded white', () => { + render(); + const button = screen.getByText('Sign in now'); + expect(button.className).not.toContain('text-white'); + expect(button.style.color).toBe(previewInk('#f5f5f5')); + }); +}); diff --git a/modules/settings/settings/pages/ModulesEdit.tsx b/modules/settings/settings/pages/ModulesEdit.tsx index 996d0c41..23cdefc0 100644 --- a/modules/settings/settings/pages/ModulesEdit.tsx +++ b/modules/settings/settings/pages/ModulesEdit.tsx @@ -110,6 +110,11 @@ function ModulesEdit({ modules, testable = {} }: Props) { {/* `pb-10` rather than a fade: the fields list ends on a full row with breathing room under it, not half a row cut off by the pane edge. */}
+ {/* One h1 per screen (#422); the selected module's own heading in + ModuleForm is an h2 beneath it. */} +

+ {t(keys.settings.modules.title)} +

{current?.manage_url ? ( {/* No second editor for these fields — the module's own page is diff --git a/modules/settings/settings/pages/components/ModuleForm.tsx b/modules/settings/settings/pages/components/ModuleForm.tsx index 6478b8c8..87b838e3 100644 --- a/modules/settings/settings/pages/components/ModuleForm.tsx +++ b/modules/settings/settings/pages/components/ModuleForm.tsx @@ -117,10 +117,10 @@ export function ModuleForm({ module: m, checks = [] }: Props) { card for the fields alone. */}
-

+

{m.package}{' '} {t(keys.settings.modules.heading_suffix)} -

+

{t(keys.settings.modules.description)}

@@ -148,7 +148,7 @@ export function ModuleForm({ module: m, checks = [] }: Props) {
{groups.map(([group, fields]) => (
- {group &&

{group}

} + {group &&

{group}

} {fields.map((f) => ( ({ + Head: () => null, + Link: ({ children, ...rest }: { children?: unknown }) => {children as never}, + router: { reload: vi.fn() }, +})); + +vi.mock('@simple-module-py/ui/layouts/AdminLayout', () => ({ AdminLayout: () => null })); + +import ModulesEdit from '../settings/pages/ModulesEdit'; + +describe('ModulesEdit', () => { + test('has one level-1 heading naming the screen, even with no modules', () => { + render(); + + const h1 = screen.getAllByRole('heading', { level: 1 }); + expect(h1).toHaveLength(1); + expect(h1[0]).toHaveTextContent('Module Settings'); + // Visible, like the other admin pages' headings (#422), not screen-reader-only. + expect(h1[0]).not.toHaveClass('sr-only'); + }); + + test('keeps the single h1 when a module is managed elsewhere', () => { + render( + , + ); + + expect(screen.getAllByRole('heading', { level: 1 })).toHaveLength(1); + }); +}); diff --git a/modules/tenants/tenants/resolver.py b/modules/tenants/tenants/resolver.py index 9803e746..86341c32 100644 --- a/modules/tenants/tenants/resolver.py +++ b/modules/tenants/tenants/resolver.py @@ -142,9 +142,14 @@ async def _resolve(request: Request) -> tuple[str | None, str | None, tuple[str, request.state.tenant_suspended = False request.state.suspended_tenant_name = None user = getattr(request.state, "user", None) + # A signed-in answer depended on the credential (membership is checked on + # every branch), so a shared cache must key on it too; an anonymous one did + # not, and anonymous public pages stay cacheable (#418). + credential: tuple[str, ...] = ("Cookie",) if user is not None else () slug = subdomain_slug(request) if slug is not None: - return await _resolve_subdomain(request, user, slug), "subdomain", ("Host",) + tenant_id = await _resolve_subdomain(request, user, slug) + return tenant_id, "subdomain", ("Host", *credential) # No slug in the host is still an answer that depended on the host. vary: tuple[str, ...] = ("Host",) if subdomains_enabled(request) else () if user is None: @@ -155,6 +160,7 @@ async def _resolve(request: Request) -> tuple[str | None, str | None, tuple[str, header_name = _header_name(request) if header_name: vary = (*vary, header_name) + vary = (*vary, *credential) requested = request.headers.get(header_name) if header_name else None if requested is not None: # An explicit per-request choice (API clients). Never fall back to @@ -170,6 +176,8 @@ async def _resolve(request: Request) -> tuple[str | None, str | None, tuple[str, return None, None, vary return _enter(request, user, active), "header", vary + # From here the answer also depends on the stored preference in the + # session cookie, which ``vary`` already names. session = request.scope.get("session") preferred = session.get(SESSION_ACTIVE_TENANT) if session is not None else None active = pick_active(memberships, preferred) diff --git a/modules/tenants/tests/test_resolver_source.py b/modules/tenants/tests/test_resolver_source.py index 9500a7e3..87ccda4b 100644 --- a/modules/tenants/tests/test_resolver_source.py +++ b/modules/tenants/tests/test_resolver_source.py @@ -5,6 +5,7 @@ import httpx import pytest from fastapi import Request +from simple_module_test import forge_session_cookie from tenants.host_resolver import forget_hosts @@ -45,6 +46,7 @@ async def test_header_source_and_vary(probe, user_client): resp = await a.get("/probe", headers={"X-Tenant-ID": t["id"]}) assert resp.json()["source"] == "header" assert "x-tenant-id" in _vary(resp) + assert "cookie" in _vary(resp) # membership was checked for the signed-in user async def test_unresolved_header_has_no_source_but_varies(probe, user_client): @@ -53,6 +55,7 @@ async def test_unresolved_header_has_no_source_but_varies(probe, user_client): resp = await a.get("/probe", headers={"X-Tenant-ID": "nope"}) assert resp.json()["source"] is None assert "x-tenant-id" in _vary(resp) + assert "cookie" in _vary(resp) async def test_subdomain_source_and_vary_host(probe, user_client): @@ -67,11 +70,43 @@ async def test_subdomain_source_and_vary_host(probe, user_client): resp = await anon.get("/probe") assert resp.json() == {"tenant": t["id"], "source": "subdomain"} assert "host" in _vary(resp) + assert "cookie" not in _vary(resp) # anonymous: public pages stay cacheable + + +async def test_subdomain_signed_in_member_varies_on_cookie(probe, user_client): + probe.state.tenants.settings.subdomain_base = "example.com" + forget_hosts() + async with user_client("a@x.io") as (a, user_id): + t = await _create(a, "Acme") + cookie = forge_session_cookie(probe.state.sm.settings.secret_key, {"user_id": user_id}) + member = httpx.AsyncClient( + transport=httpx.ASGITransport(app=probe), + base_url=f"http://{t['slug']}.example.com", + cookies={"session": cookie}, + ) + async with member: + resp = await member.get("/probe") + assert resp.json() == {"tenant": t["id"], "source": "subdomain"} + tokens = _vary(resp) + assert "host" in tokens + assert "cookie" in tokens + assert len(tokens) == len(set(tokens)) async def test_vary_has_no_duplicates(probe, user_client): async with user_client("a@x.io") as (a, _): await _create(a, "One") - resp = await a.get("/probe", headers={"X-Tenant-ID": "nope"}) + for headers in ({"X-Tenant-ID": "nope"}, {}): + tokens = _vary(await a.get("/probe", headers=headers)) + assert len(tokens) == len(set(tokens)), tokens + + +async def test_session_source_varies_on_cookie(probe, user_client): + """The tenant came from the session cookie, so a shared cache must key on it (#418).""" + async with user_client("a@x.io") as (a, _): + await _create(a, "One") + resp = await a.get("/probe") + assert resp.json()["source"] == "session" tokens = _vary(resp) + assert "cookie" in tokens assert len(tokens) == len(set(tokens)) diff --git a/modules/users/users/pages/Users/AddPeople.tsx b/modules/users/users/pages/Users/AddPeople.tsx index 398b968b..774931ec 100644 --- a/modules/users/users/pages/Users/AddPeople.tsx +++ b/modules/users/users/pages/Users/AddPeople.tsx @@ -1,4 +1,4 @@ -import { Link, router, usePage } from '@inertiajs/react'; +import { Head, Link, router, usePage } from '@inertiajs/react'; import { keys, useT } from '@simple-module-py/i18n'; import { InlineBanner } from '@simple-module-py/ui/components/InlineBanner'; import { PageShell } from '@simple-module-py/ui/components/PageShell'; @@ -180,110 +180,117 @@ function AddPeople() { : t(keys.users.add_people.submit_create); return ( - - { - setMode(next); - setError(null); - }} - aria-label={t(keys.users.add_people.tablist_label)} - options={[ - { value: 'invite', label: t(keys.users.add_people.mode_invite) }, - { value: 'create', label: t(keys.users.add_people.mode_create) }, - ]} - className="mb-4" - /> + <> + + + { + setMode(next); + setError(null); + }} + aria-label={t(keys.users.add_people.tablist_label)} + options={[ + { value: 'invite', label: t(keys.users.add_people.mode_invite) }, + { value: 'create', label: t(keys.users.add_people.mode_create) }, + ]} + className="mb-4" + /> - {!mailerDelivers && ( - // One sentence, as the deck has it — a title/body pair reads as two - // separate facts when it is really one: which mailer, and what that - // means for the links. The "Configure SMTP" link ends the sentence - // rather than sitting right-aligned away from what it refers to. - - {t(keys.users.add_people.mailer_banner_prefix)} {mailerName}{' '} - {t(keys.users.add_people.mailer_banner_suffix)}{' '} - {/* The 44px tap area is grown with a pseudo-element: an inline + {!mailerDelivers && ( + // One sentence, as the deck has it — a title/body pair reads as two + // separate facts when it is really one: which mailer, and what that + // means for the links. The "Configure SMTP" link ends the sentence + // rather than sitting right-aligned away from what it refers to. + + {t(keys.users.add_people.mailer_banner_prefix)} {mailerName}{' '} + {t(keys.users.add_people.mailer_banner_suffix)}{' '} + {/* The 44px tap area is grown with a pseudo-element: an inline link inside a sentence cannot take `min-h-11` without stretching the line box around it. */} - - {t(keys.users.add_people.configure_smtp)} - - - } - /> - )} + + {t(keys.users.add_people.configure_smtp)} + + + } + /> + )} - {/* "Last batch" belongs to the invite flow; creating one account with a + {/* "Last batch" belongs to the invite flow; creating one account with a password you set has no batch to report. */} -
- - -
- {mode === 'invite' ? ( - - ) : ( - <> - + + + + {mode === 'invite' ? ( + - - - )} + ) : ( + <> + + + + )} - {error &&

{error}

} + {error &&

{error}

} -
- - -
- - - +
+ + +
+ +
+ - {mode === 'invite' && ( - - )} -
- + {mode === 'invite' && ( + + )} +
+ + ); } diff --git a/modules/users/users/pages/Users/Edit.tsx b/modules/users/users/pages/Users/Edit.tsx index 4b826ef0..29503218 100644 --- a/modules/users/users/pages/Users/Edit.tsx +++ b/modules/users/users/pages/Users/Edit.tsx @@ -1,4 +1,4 @@ -import { usePage } from '@inertiajs/react'; +import { Head, usePage } from '@inertiajs/react'; import { keys, useT } from '@simple-module-py/i18n'; import { PageShell } from '@simple-module-py/ui/components/PageShell'; import { useRelativeTime } from '@simple-module-py/ui/hooks/use-relative-time'; @@ -57,68 +57,71 @@ function Edit() { const actions = useUserActions(user.id, user.is_active, user.is_verified); return ( - - {initials(user.full_name, user.email)} - - } - badge={ - - } - description={t(keys.users.edit.subtitle, { - email: user.email, - joined: joinedMonth(user.created_at), - lastLogin: user.last_login_at ? ago(user.last_login_at) : t(keys.users.edit.never), - })} - actions={ - - } - > -
- form.setForm((prev) => ({ ...prev, email }))} - onFullNameChange={(fullName) => form.setForm((prev) => ({ ...prev, fullName }))} - roles={roles} - selectedRoles={form.form.roles} - onToggleRole={form.toggleRole} - userId={user.id} - hasPermissionsModule={has_permissions_module} - error={form.error} - /> + <> + + + {initials(user.full_name, user.email)} + + } + badge={ + + } + description={t(keys.users.edit.subtitle, { + email: user.email, + joined: joinedMonth(user.created_at), + lastLogin: user.last_login_at ? ago(user.last_login_at) : t(keys.users.edit.never), + })} + actions={ + + } + > +
+ form.setForm((prev) => ({ ...prev, email }))} + onFullNameChange={(fullName) => form.setForm((prev) => ({ ...prev, fullName }))} + roles={roles} + selectedRoles={form.form.roles} + onToggleRole={form.toggleRole} + userId={user.id} + hasPermissionsModule={has_permissions_module} + error={form.error} + /> - + - {/* Absent, not empty, when the deployment records nothing. */} - {recentActivity !== null && ( - - )} + {/* Absent, not empty, when the deployment records nothing. */} + {recentActivity !== null && ( + + )} - -
- + +
+ + ); } diff --git a/modules/users/users/pages/Users/Index.tsx b/modules/users/users/pages/Users/Index.tsx index 71147b70..ea364421 100644 --- a/modules/users/users/pages/Users/Index.tsx +++ b/modules/users/users/pages/Users/Index.tsx @@ -1,4 +1,4 @@ -import { Link, router, usePage } from '@inertiajs/react'; +import { Head, Link, router, usePage } from '@inertiajs/react'; import { keys, useT } from '@simple-module-py/i18n'; import { PageShell } from '@simple-module-py/ui/components/PageShell'; import { SegmentedControl } from '@simple-module-py/ui/components/SegmentedControl'; @@ -109,77 +109,80 @@ function Index() { }, [navigate]); return ( - - - - {t(keys.users.index.add_people)} - - - } - > - - -
- + + + + + {t(keys.users.index.add_people)} + + + } + > + - {view === 'users' && ( - <> -
- - setSearch(e.target.value)} - className="pl-9 max-lg:min-h-11" + +
+ + {view === 'users' && ( + <> +
+ + setSearch(e.target.value)} + className="pl-9 max-lg:min-h-11" + /> +
+ r.name)} + onChange={(next) => navigate(next)} /> -
- + )} +
+ + {view === 'users' ? ( + <> + {pagination.total === 1 && !isFiltered && } + r.name)} - onChange={(next) => navigate(next)} + page={pagination.page} + perPage={pagination.per_page} + total={pagination.total} + filtered={isFiltered} + onSort={toggleSort} + onPage={(page) => navigate({ page })} + onClearFilters={clearFilters} /> + ) : ( + )} -
- - {view === 'users' ? ( - <> - {pagination.total === 1 && !isFiltered && } - navigate({ page })} - onClearFilters={clearFilters} - /> - - ) : ( - - )} - + + ); } diff --git a/packages/i18n/src/generated-resources.ts b/packages/i18n/src/generated-resources.ts index b4f44ca8..60a46de2 100644 --- a/packages/i18n/src/generated-resources.ts +++ b/packages/i18n/src/generated-resources.ts @@ -222,6 +222,7 @@ export default { 'branding.manage.preview_tab_signin': '', 'branding.manage.preview_tabs_label': '', 'branding.manage.preview_title': '', + 'branding.manage.primary_color_invalid': '', 'branding.manage.primary_color_label': '', 'branding.manage.publish': '', 'branding.manage.publish_note': '', diff --git a/packages/i18n/src/keys.generated.ts b/packages/i18n/src/keys.generated.ts index f470994a..c06942c0 100644 --- a/packages/i18n/src/keys.generated.ts +++ b/packages/i18n/src/keys.generated.ts @@ -279,6 +279,7 @@ export const keys = { preview_tab_signin: 'branding.manage.preview_tab_signin', preview_tabs_label: 'branding.manage.preview_tabs_label', preview_title: 'branding.manage.preview_title', + primary_color_invalid: 'branding.manage.primary_color_invalid', primary_color_label: 'branding.manage.primary_color_label', publish: 'branding.manage.publish', publish_note: 'branding.manage.publish_note', diff --git a/packages/ui/src/components/BrandingHead.test.tsx b/packages/ui/src/components/BrandingHead.test.tsx index 1330bbbc..603b5f97 100644 --- a/packages/ui/src/components/BrandingHead.test.tsx +++ b/packages/ui/src/components/BrandingHead.test.tsx @@ -16,6 +16,8 @@ beforeEach(() => { document.documentElement.style.removeProperty('--primary'); document.documentElement.style.removeProperty('--sidebar-primary'); document.documentElement.style.removeProperty('--color-primary-600'); + document.documentElement.style.removeProperty('--primary-foreground'); + document.documentElement.style.removeProperty('--sidebar-primary-foreground'); }); afterEach(() => cleanup()); @@ -47,6 +49,17 @@ describe('BrandingHead', () => { expect(document.documentElement.style.getPropertyValue('--color-primary-600')).toBe(''); }); + test('picks a dark foreground ink for a light brand and clears it on unmount (#420)', () => { + state.branding = { appName: 'Acme', primaryColor: '#62B8E2', logoUrl: null, faviconUrl: null }; + const { unmount } = render(); + const style = document.documentElement.style; + expect(style.getPropertyValue('--primary-foreground')).toBe('oklch(0.2 0.02 250)'); + expect(style.getPropertyValue('--sidebar-primary-foreground')).toBe('oklch(0.2 0.02 250)'); + unmount(); + expect(style.getPropertyValue('--primary-foreground')).toBe(''); + expect(style.getPropertyValue('--sidebar-primary-foreground')).toBe(''); + }); + test('keeps the server-rendered theme-color meta in sync, restoring on unmount', () => { const meta = document.createElement('meta'); meta.setAttribute('name', 'theme-color'); diff --git a/packages/ui/src/components/BrandingHead.tsx b/packages/ui/src/components/BrandingHead.tsx index 3f512b51..f7c422ca 100644 --- a/packages/ui/src/components/BrandingHead.tsx +++ b/packages/ui/src/components/BrandingHead.tsx @@ -9,7 +9,9 @@ import type { SharedProps } from '../types'; * * - the favicon `` when a custom favicon is set, * - the primary brand colour — derived into the full `--color-primary-*` ramp - * (plus base `--primary` / `--sidebar-primary`) and written as inline CSS + * (plus base `--primary` / `--sidebar-primary`, and the matching + * `--primary-foreground` / `--sidebar-primary-foreground` ink chosen by WCAG + * contrast) and written as inline CSS * variables on `:root`. Inline wins over the stylesheet's `:root`/`.dark` * rules, so every Tailwind `primary` utility — solid buttons *and* the * `primary-600/700/800` gradient tints used by the brand badge — follows the diff --git a/packages/ui/src/components/BrandingMark.tsx b/packages/ui/src/components/BrandingMark.tsx index b418501a..838b2511 100644 --- a/packages/ui/src/components/BrandingMark.tsx +++ b/packages/ui/src/components/BrandingMark.tsx @@ -65,6 +65,7 @@ export function BrandingMark({ /> ) : (
+ {/* Stays white: the gradient uses fixed-lightness ramp steps, so the brand-hex ink does not apply. */} {initial}
)} diff --git a/packages/ui/src/components/ErrorScreen.tsx b/packages/ui/src/components/ErrorScreen.tsx index 9bf8b750..33b3c889 100644 --- a/packages/ui/src/components/ErrorScreen.tsx +++ b/packages/ui/src/components/ErrorScreen.tsx @@ -9,6 +9,8 @@ interface Props { children: ReactNode; /** Visual accent for the numeral (403=warning, 404=primary, 5xx=destructive). */ accent?: 'primary' | 'warning' | 'destructive'; + /** Inside an app shell: no full-viewport frame or page background (#422). */ + inline?: boolean; } /** @@ -30,9 +32,16 @@ export function ErrorScreen({ details, children, accent = 'primary', + inline = false, }: Props) { return ( -
+

{ + it('puts wrapperClassName on the wrapper and className on the select (#421)', () => { + const { container } = render( + + {/* i18n-exempt: test fixture */} + A + , + ); + const wrapper = container.querySelector('[data-slot="native-select-wrapper"]'); + const select = container.querySelector('select'); + expect(wrapper?.className).toContain('w-full'); + expect(select?.className).toContain('text-xs'); + expect(select?.className).not.toContain('w-fit'); + }); + + it('keeps the wrapper w-fit by default', () => { + const { container } = render(); + expect(container.querySelector('[data-slot="native-select-wrapper"]')?.className).toContain( + 'w-fit', + ); + }); +}); diff --git a/packages/ui/src/components/ui/native-select.tsx b/packages/ui/src/components/ui/native-select.tsx index a7fed4a6..91922981 100644 --- a/packages/ui/src/components/ui/native-select.tsx +++ b/packages/ui/src/components/ui/native-select.tsx @@ -2,14 +2,26 @@ import { cn } from '@simple-module-py/ui/lib/utils'; import { ChevronDownIcon } from 'lucide-react'; import type * as React from 'react'; +/** + * `className` styles the ` = { // Solid pill, per the deck: a tinted row with a left rule read as a hover // state next to the near-black surface, and lost the current page at a // glance on a phone. - activeClass: 'bg-primary text-white', + activeClass: 'bg-primary text-primary-foreground', inactiveClass: 'text-app-sidebar-text hover:bg-app-sidebar-hover hover:text-white', mutedTextClass: 'text-app-sidebar-text-muted', }; diff --git a/packages/ui/src/lib/color.test.ts b/packages/ui/src/lib/color.test.ts index b190bf06..c4931880 100644 --- a/packages/ui/src/lib/color.test.ts +++ b/packages/ui/src/lib/color.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from 'vitest'; -import { BASE_PRIMARY_RAMP, deriveBrandRamp, hexToOklch } from './color'; +import { BASE_PRIMARY_RAMP, contrastRatio, deriveBrandRamp, hexToOklch } from './color'; describe('hexToOklch', () => { test('white is light and (near) achromatic', () => { @@ -60,3 +60,28 @@ describe('deriveBrandRamp', () => { expect(deriveBrandRamp('not-a-color')).toBeNull(); }); }); + +const DARK_INK = 'oklch(0.2 0.02 250)'; +const WHITE = 'oklch(1 0 0)'; + +describe('brand foreground ink (#420)', () => { + test.each([ + ['#62B8E2', DARK_INK], + ['#9AD3EF', DARK_INK], + ['#2E6DB0', WHITE], + ['#16276E', WHITE], + ])('%s gets %s', (hex, ink) => { + const ramp = deriveBrandRamp(hex); + expect(ramp?.['--primary-foreground']).toBe(ink); + expect(ramp?.['--sidebar-primary-foreground']).toBe(ink); + }); + + test('white is rejected on light brands because it fails WCAG AA', () => { + expect(contrastRatio('#62B8E2', '#ffffff')).toBeLessThan(4.5); + }); + + test('contrastRatio is 21 for black on white and 1 for unparseable input', () => { + expect(contrastRatio('#000000', '#ffffff')).toBeCloseTo(21, 5); + expect(contrastRatio('nope', '#ffffff')).toBe(1); + }); +}); diff --git a/packages/ui/src/lib/color.ts b/packages/ui/src/lib/color.ts index 09b93140..c8fe497c 100644 --- a/packages/ui/src/lib/color.ts +++ b/packages/ui/src/lib/color.ts @@ -91,11 +91,44 @@ function oklchString({ l, c, h }: Oklch): string { return `oklch(${round(l, 4)} ${round(c, 4)} ${round(h, 2)})`; } +/** WCAG 2.x relative luminance of an `#rrggbb` colour (null if unparseable). */ +function relativeLuminance(hex: string): number | null { + const lin = parseHex(hex); + if (!lin) return null; + const [r, g, b] = lin; + return 0.2126 * r + 0.7152 * g + 0.0722 * b; +} + +/** WCAG contrast ratio between two `#rrggbb` colours (1-21; 1 when either is unparseable). */ +export function contrastRatio(a: string, b: string): number { + const la = relativeLuminance(a); + const lb = relativeLuminance(b); + if (la === null || lb === null) return 1; + const [hi, lo] = la > lb ? [la, lb] : [lb, la]; + return (hi + 0.05) / (lo + 0.05); +} + +const WHITE_INK = 'oklch(1 0 0)'; +const DARK_INK = 'oklch(0.2 0.02 250)'; +// DARK_INK converted to sRGB (OKLCH -> OKLab -> linear sRGB -> gamma); used only +// for the contrast ratio, never emitted as a colour. +const DARK_INK_HEX = '#0f171f'; +const AA_NORMAL_TEXT = 4.5; + +/** Text colour for a brand-coloured surface: white when it passes AA, else the higher-contrast ink. */ +function foregroundFor(hex: string): string { + const white = contrastRatio(hex, '#ffffff'); + if (white >= AA_NORMAL_TEXT) return WHITE_INK; + return contrastRatio(hex, DARK_INK_HEX) > white ? DARK_INK : WHITE_INK; +} + /** * Derive the CSS custom properties that re-theme the primary ramp to `hex`. * * Returns a map of `--color-primary-` → `oklch(…)` (plus the base - * `--primary` / `--sidebar-primary` set to the brand colour itself). Returns + * `--primary` / `--sidebar-primary` set to the brand colour itself, and + * `--primary-foreground` / `--sidebar-primary-foreground` set to white or a dark + * ink, whichever reads better on it). Returns * null for an unparseable colour so callers can leave the default theme intact. */ export function deriveBrandRamp(hex: string): Record | null { @@ -111,5 +144,10 @@ export function deriveBrandRamp(hex: string): Record | null { // precisely what the admin chose, while the ramp drives gradients/tints. vars['--primary'] = hex; vars['--sidebar-primary'] = hex; + // White text on a light brand colour fails AA (2.2:1 on #62B8E2, #420), so + // the ink follows the colour rather than staying the stylesheet's white. + const ink = foregroundFor(hex); + vars['--primary-foreground'] = ink; + vars['--sidebar-primary-foreground'] = ink; return vars; } diff --git a/packages/ui/src/styles/globals.css b/packages/ui/src/styles/globals.css index 163ddde3..8c3051e9 100644 --- a/packages/ui/src/styles/globals.css +++ b/packages/ui/src/styles/globals.css @@ -27,7 +27,8 @@ --color-landing-bg: oklch(0.13 0.02 250); --color-dark-text: oklch(0.9 0.005 250); --color-dark-text-muted: oklch(0.6 0.01 250); - --color-dark-text-subtle: oklch(0.45 0.01 250); + /* 5.5:1 on the auth aside (#03080f), above AA's 4.5 (#414). */ + --color-dark-text-subtle: oklch(0.62 0.01 250); --color-dark-border: oklch(0.35 0.01 250); --color-dark-border-hover: oklch(0.5 0.01 250); diff --git a/tests/e2e/test_document_titles.py b/tests/e2e/test_document_titles.py index 134a32fa..68df19af 100644 --- a/tests/e2e/test_document_titles.py +++ b/tests/e2e/test_document_titles.py @@ -25,6 +25,7 @@ _TITLED_PAGES = [ ("/dashboard/", "Dashboard"), ("/admin", "Administration"), + ("/admin/users/", "Users"), ] diff --git a/tests/e2e/test_error_pages.py b/tests/e2e/test_error_pages.py index e87761d1..05e9e24b 100644 --- a/tests/e2e/test_error_pages.py +++ b/tests/e2e/test_error_pages.py @@ -66,3 +66,16 @@ def test_fetch_callers_get_json_detail(page: Page, e2e_username: str, e2e_passwo ) assert bare.status == 404 assert "detail" in bare.json() + + +def test_admin_not_found_renders_inside_the_admin_shell( + page: Page, e2e_username: str, e2e_password: str +) -> None: + """A missing admin record keeps the admin in the admin layout (#422).""" + _login(page, e2e_username, e2e_password) + response = page.goto("/admin/users/00000000-0000-0000-0000-000000000000") + assert response is not None + assert response.status == 404 + expect(page.get_by_role("heading", level=1)).to_be_visible(timeout=10_000) + # The admin panel badge links to /admin; the bare public error page has none. + expect(page.locator("a[href='/admin']").first).to_be_attached()