Fix deployed-UI feedback: dashboard 500, checkbox grids, manage-access cards #3

Merged

crueber merged 3 commits from redesign/tailwind-v4 into main

2026-08-29 20:45:46 +00:00

crueber commented

2026-08-29 20:35:14 +00:00

Owner

Follow-up to #2 (deployed feedback). Three commits on top of what was merged.

1. Root page returned 500 for every logged-in user (`65ab273`)

The redesigned dashboard template executes the shared flash partial, which evaluates .Flash — but the / handler rendered dashboardData{User, Apps} with no such field, so every GET / failed with a template error. Reproduced locally before fixing.

Fix: the handler moved from an untestable run.go closure into user.Handler.GetDashboard, following the package's existing consumer-interface pattern:

new user.AppLister (backed by *app.Service in run.go)
new userContextReader func type (wired to session.UserFromContext; avoids the session→user import cycle)
dashboardData gains Flash, populated from ?flash= like the other pages

Regression tests (internal/user/handler_test.go): empty state; apps + flash partial end-to-end (fails if the Flash field disappears again); lister error → 500. All user.NewHandler call sites updated.

2. Checkbox grids + manage-access cleanup (`65ab273` + `4c9fe38`)

New .check-list grid component (1-col mobile / 2-col ≥sm): applied to role-edit and role-add group checkboxes, and the app-assignments Roles/Groups fieldsets — name + muted description, aligned, no clustering
User manage-access page: the 4-column table is now one card per app — grant-access checkbox in the header, Groups/Roles as checkbox grids below. Field names (app_id, groups.<id>, roles.<id>) and select-all JS untouched

3. CSP was silently killing inline JS (`ef1c991`)

Verified live during doc work: the global CSP (script-src 'self') silently blocks inline scripts and on*= handlers — the manage-access Select All button did nothing, the OAuth Copy button was inert, and rotate-secret / passkey-delete submitted without their confirmation dialogs. All pre-existing (CSP unchanged since the Bulma era), but destructive actions running unconfirmed is a real hazard.

Fix: new internal/web/static/app.js (defer, CSP-safe) drives behavior via data-copy / data-confirm attributes plus select-all wiring; inline handlers removed from templates.

Documentation (`ef1c991`)

AGENTS.md now documents the flash-partial data contract (missing .Flash = runtime 500), the strict-CSP rules (UI conventions + must-not-do), load-bearing template strings asserted by tests, the dashboard handler location, and the CI CSS drift check. README gains the make css workflow and a corrected Go version.

Verification

go test -race ./... all green; build/vet/gofmt clean; committed CSS reproduces byte-for-byte from make css
Browser-verified desktop (1280×800) and mobile (375×812): dashboard renders granted apps, all pages stack cleanly; Select All now toggles live against a running server

Review

Second code review returned approve (0 blocking, 0 should-fix); its 4 actionable nits are incorporated in 4c9fe38. Known pre-existing issue left out of scope: an app with an empty Name would still 500 the dashboard via {{slice .Name 0 1}} (guarded by the required form field today).

Follow-up to #2 (deployed feedback). Three commits on top of what was merged. ## 1. Root page returned 500 for every logged-in user (65ab273) The redesigned dashboard template executes the shared flash partial, which evaluates `.Flash` — but the `/` handler rendered `dashboardData{User, Apps}` with no such field, so every GET / failed with a template error. Reproduced locally before fixing. Fix: the handler moved from an untestable `run.go` closure into `user.Handler.GetDashboard`, following the package's existing consumer-interface pattern: - new `user.AppLister` (backed by `*app.Service` in run.go) - new `userContextReader` func type (wired to `session.UserFromContext`; avoids the session→user import cycle) - `dashboardData` gains `Flash`, populated from `?flash=` like the other pages **Regression tests** (`internal/user/handler_test.go`): empty state; apps + flash partial end-to-end (fails if the `Flash` field disappears again); lister error → 500. All `user.NewHandler` call sites updated. ## 2. Checkbox grids + manage-access cleanup (65ab273 + 4c9fe38) - New `.check-list` grid component (1-col mobile / 2-col ≥sm): applied to role-edit **and** role-add group checkboxes, and the app-assignments Roles/Groups fieldsets — name + muted description, aligned, no clustering - User manage-access page: the 4-column table is now one card per app — grant-access checkbox in the header, Groups/Roles as checkbox grids below. Field names (`app_id`, `groups.<id>`, `roles.<id>`) and select-all JS untouched ## 3. CSP was silently killing inline JS (ef1c991) Verified live during doc work: the global CSP (`script-src 'self'`) silently blocks inline scripts and `on*=` handlers — the manage-access **Select All** button did nothing, the OAuth **Copy** button was inert, and **rotate-secret / passkey-delete submitted without their confirmation dialogs**. All pre-existing (CSP unchanged since the Bulma era), but destructive actions running unconfirmed is a real hazard. Fix: new `internal/web/static/app.js` (defer, CSP-safe) drives behavior via `data-copy` / `data-confirm` attributes plus select-all wiring; inline handlers removed from templates. ## Documentation (ef1c991) AGENTS.md now documents the flash-partial data contract (missing `.Flash` = runtime 500), the strict-CSP rules (UI conventions + must-not-do), load-bearing template strings asserted by tests, the dashboard handler location, and the CI CSS drift check. README gains the `make css` workflow and a corrected Go version. ## Verification - `go test -race ./...` all green; build/vet/gofmt clean; committed CSS reproduces byte-for-byte from `make css` - Browser-verified desktop (1280×800) and mobile (375×812): dashboard renders granted apps, all pages stack cleanly; Select All now toggles live against a running server ## Review Second code review returned **approve** (0 blocking, 0 should-fix); its 4 actionable nits are incorporated in 4c9fe38. Known pre-existing issue left out of scope: an app with an empty `Name` would still 500 the dashboard via `{{slice .Name 0 1}}` (guarded by the required form field today).

crueber added 2 commits

2026-08-29 20:35:14 +00:00

fix(ui): dashboard 500, checkbox grids, manage-access cards 65ab2733bd

- Move the / (My Apps) handler from a run.go closure into
  user.Handler.GetDashboard with Flash support: the redesign's flash
  partial evaluated .Flash on a data struct that lacked the field,
  making GET / return 500 for every logged-in user. New AppLister and
  userContextReader consumer interfaces; regression tests cover the
  empty state, apps + flash rendering, and lister errors.
- Add .check-list grid component (1-col mobile / 2-col sm+) and apply
  it to role-edit group checkboxes, app-assignment roles/groups, so
  options align on a grid instead of clustering.
- Replace the user manage-access 4-column table with a card per app:
  grant-access checkbox in the header, groups/roles as checkbox grids
  below. Field names and select-all behavior unchanged.

Deployed-feedback fixes for PR #2.

fix(ui): address review nits on feedback fixes 4c9fe38c0e

- Apply the check-list grid to the role-add form's group checkboxes so
  both role forms share one pattern
- Trim duplicated items-center/min-w-0 from .check-list component rules
  (templates already carry them); rebuild passage.css
- Single app.NewStore per inline test fixture, matching the shared
  fixture; move the .Flash regression-guard comment to the test that
  actually exercises the flash partial

crueber added 1 commit

2026-08-29 20:44:00 +00:00

fix(ui): CSP was silently killing inline JS; move behavior to app.js ef1c9919fc

The global CSP (script-src 'self') silently blocks inline <script>
blocks and on*= handler attributes. Verified live: the user manage-access
'Select All' button did nothing, the OAuth client-secret Copy button was
inert, and rotate-secret / passkey-delete submitted WITHOUT their
confirmation dialogs. All pre-existing (CSP unchanged since the Bulma
era), but destructive actions running unconfirmed is a real hazard.

- Add internal/web/static/app.js (defer, CSP-safe): data-copy,
  data-confirm (delegated), and select-all wiring
- Replace inline handlers in app_oauth.html, passkeys.html; drop the
  dead inline select-all script from user_apps.html
- Docs: AGENTS.md gains the flash-partial data contract, the strict-CSP
  rules (UI conventions + must-not-do), the load-bearing template strings
  note, the dashboard handler location, and the CI CSS drift check;
  README gains the make css workflow and corrects the Go version

crueber merged commit efe6c9530a into main

2026-08-29 20:45:46 +00:00

crueber referenced this pull request from a commit

2026-08-29 20:45:47 +00:00

Merge pull request 'Fix deployed-UI feedback: dashboard 500, checkbox grids, manage-access cards' (#3) from redesign/tailwind-v4 into main

crueber referenced this pull request

2026-08-29 20:53:32 +00:00

fix(ci): pin Tailwind scan scope so prose can't break the CSS drift check #4