Files
edr-platform/docs/hr-finance-nav-redesign-report.md
2026-08-25 00:11:39 +03:00

162 lines
10 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# HR + Finance shell redesign — report (reconstructed 2026-08-25)
> Also published as an artifact:
> https://claude.ai/code/artifact/5e0b90d8-e9dd-462b-819a-6aa19b83fc50
> **Why this file says "reconstructed".** The original report was destroyed with the rest of
> the working tree on 2026-08-24 and was never committed to git. The restore the owner ran
> came from a **2026-08-22** backup, which predates the entire redesign. This document
> describes the shell **as it stands in the tree today**, re-verified in a browser on
> 2026-08-25 — not as it was remembered. Where the current state differs from the record of
> the original, the difference is called out rather than smoothed over.
## What was recovered vs. rebuilt
Recovery used two sources that the wipe did not touch, in preference to rewriting anything:
| Source | What it gave back |
| --- | --- |
| `~/.claude/file-history/<session>/` (pre-edit snapshots) | HR `nav-model.ts` (400 lines), `useApprovalCounts.ts`, HR `App.tsx`, `permissions.ts`, `AuthContext.tsx`, `RequireAuth.tsx`, both API bug fixes, the e2e Phase 0 matrix, the security write-up, the scoped seeder |
| The session transcript `74e086a2….jsonl` (Write/Edit replay) | The whole `e2e-hr-finance/` Playwright suite — 8 files, replayed with no failed or ambiguous edits |
Everything above is **byte-for-byte the original**, not a re-derivation.
The following had no snapshot — they were last written on 2026-08-23, in a session whose
transcript is absent — and were **rebuilt from their contracts** (the recovered `nav-model.ts`
specifies the exact `SidebarItem` shape the sidebar must support, which made this far less
speculative than it sounds):
- `packages/ui-common/…/Sidebar.tsx` and `DashboardLayout.tsx`
- both apps' `AppShell.tsx` and `CommandPalette.tsx`
- `finance-web`'s `nav-model.ts`
- the `@custom-variant dark` line in both apps' `index.css`
- the HR dashboard's "Needs you today" panel and the Finance overview tiles
**Not recovered, and not invented:** `docs/prompts/hr-finance-ui-redesign-prompt.md`,
`docs/prompts/hr-finance-e2e-prompt.md`, and the original `hr-finance-nav-redesign-plan.md`.
These were inputs, not outputs; writing plausible replacements would misrepresent what drove
the work. They are simply gone.
## The shell as it now stands
**Grouping.** Both apps group subject-first with a personal "My work" group on top. The test
is "does this screen show only my own record?", mirroring `viewOwn` vs `viewAll`.
- HR: 25 screens → **9 top-level rows** — Overview, My work, Approvals, People, Time & leave,
Payroll, Talent, Reports, Settings.
- Finance: 13 screens → **8 top-level rows** — Overview, Ledger, Revenue, Spend, Planning,
Fixed assets, Reports, Setup.
Nav gates mirror route gates. A group with no permitted child is not rendered at all, and a
group holding exactly one permitted screen collapses to a flat row keeping the group's icon —
confirmed live: `hr_manager` sees Payroll as a single "Payroll runs" row, not a group.
**Group headers carry no `href`.** They render as `<button>`, so a header can never navigate
into something the reader is gated out of.
**Command palette.** Cmd/Ctrl-K, built from `visibleEntries` — the same gated source the
sidebar renders — so the palette cannot offer a screen the sidebar hides. A visible "Search
⌘K" pill sits beside the breadcrumb, because nobody discovers a shortcut that is never shown.
HR: 27 actions (25 screens + profile + logout). Finance: 14 (13 screens + logout).
**Responsive rail** behind an opt-in `responsive` prop, default off, so the ~94 other
`@edr/ui-common` consumers are untouched: ≥1280 the full 256px column, 9001279 a 64px icon
rail whose groups open a flyout, <900 hidden behind a hamburger opening an overlay drawer that
closes on navigate, Escape, backdrop, and on widening. All five behaviours verified.
## Defects found and fixed
Nine were recorded against the original seven slices. The ones that live in code today, and
are verified in the current tree:
1. **`<aside>` had no `overflow-y-auto`** and sat in `min-h-screen`, so HR's 25 entries
(~1,180px) pushed *Reports* and *My profile* below the fold and scrolled the whole page.
Now `sticky top-0 h-screen overflow-y-auto`. Finance's 13 entries fit, which is why the
Finance-only browser pass never saw it.
2. **Active-row matching was per-item prefix matching**, so `/leave/approvals` highlighted
*both* "My leave" and "Leave approvals". Now longest-match-wins over the whole tree, with
whole-segment boundaries. Verified: exactly one `aria-current="page"` on all 39 routes.
3. **`min-w-0` on the content column.** A flex child will not shrink below its content's
intrinsic width, so one wide table stretched the column and scrolled the *page* sideways,
header included. Verified: 0 horizontal overflow at 1440 / 1100 / 820 / 390.
4. **Every `dark:` utility in the shell was dead.** The shell toggles a `dark` *class*;
Tailwind v4 defaults `dark:` to the OS preference. `ui-common` declares
`@custom-variant dark (&:where(.dark, .dark *))` in its own stylesheet, but each app runs a
*second* Tailwind build over ui-common's source via `@source`, and the app's sheet loads
later so the app's plain `text-slate-800` beat ui-common's class-scoped
`dark:text-slate-100`. Fixed by declaring the same variant in each app's `index.css`.
Measured today: wordmark **14.08:1 light, 16.36:1 dark** in both apps.
**Any new Mantine + Tailwind app in this repo needs that line.**
5. **Persisted group expansion did not survive a reload**, even though it was stored
correctly: the initial state spread the auto-expand map *whole*, and that map says `false`
for every group not holding the current page, overwriting what the reader left open. Only
its `true` entries may be merged. Generalise: *an "open the active section" map is not a
statement about the other sections.* Verified: a group opened by hand is still open after
reload, and still open after navigating into a different section.
6. **Dead header controls.** The globe now drives the `setLanguage()` that already existed
unreachable in `src/i18n/index.ts` (en / አማርኛ, persists across reload verified). The bell
is off in both apps: there is no notification source, and a never-clearing red dot is a lie.
HR's Profile goes to `/me`; Finance hides it, having no profile screen.
7. **The app title rendered twice per screen.** The top bar now carries a breadcrumb
(`Group / Screen`) instead. Verified present and correct on all 39 routes.
8. **Two API/permission bugs** (recovered from snapshots, re-verified live as `hr.demo`, a
plain `hr_manager`):
- `/leave/approvals` was unreachable by anyone but a super admin nav gate, route gate and
badge hook all required `approve_l1:leave_request` alone, a permission the seed grants to
no role, position, or position-type. Widened to `[approve_l1, approve_l2]`; `NavEntry.gate`
and `can()` are typed `string | string[]` with OR semantics. **Now lands on the page.**
- `GET /job-positions` 500'd for any non-super-admin: `JobPositionsRepository.findPage`
referenced `position.organization_id`, a column `hr.job_positions` has never had. Fixed by
joining `iam.positions` via `EXISTS`. **Now 200, no 5xx.**
### One defect found during this reconstruction
**The command palette searched group names.** Mantine's default Spotlight filter matches on
`description` as well as `label`, and the group is carried in `description` so typing "lea"
also returned *Attendance register* (via "Time & leave"), pushing the wanted screen down the
list. The original recorded this as an open trade-off and later as resolved; the resolution
did not survive, so it was fixed here with an explicit `filter` that matches the screen name
only. The group remains visible as context. Verified: `"lea"` returns 5 leave screens and does
**not** match *Attendance register*.
## Verification actually performed (2026-08-25)
Type-checks: `@edr/ui-common`, `@edr/hr-web`, `@edr/finance-web` all pass.
Regression control for the ~94 other consumers: `@edr/freight-backoffice` type-checks to
**exactly 956 errors**, the same count recorded before the redesign, and **none mention
`Sidebar` or `DashboardLayout`**.
Driven in a real browser against the live stack (hr-api :3005, finance-api :3004,
passenger-api :4000, hr-web :5185, finance-web :5186) on `smart_office_prod`:
| Check | Result |
| --- | --- |
| Routes walked, both apps, both themes | 25 HR + 14 Finance × light/dark = **78 route-visits** |
| Page errors | **0** |
| Exactly one active nav row | **39/39 routes** |
| Breadcrumb present and correct | **39/39 routes** |
| Horizontal overflow @ 1440/1100/820/390 | **0** |
| Palette, language, theme, rail, flyout, drawer, persistence | all pass, **0 page errors** |
| Non-super-admin pass as `hr.demo` (`hr_manager`) | 9 nav rows, both regressions clear, **0 5xx** |
**Failing requests seen, and why they are not shell defects:** a `401 GET /api/v1/me` on the
login page (the pre-auth session probe, expected), and `404`s on
`/api/v1/employee-profiles/me` and `/api/v1/leave-balances/me` those accounts have no HR
profile or leave-balance rows. They are data-level and appear identically for both accounts.
**Not done:** no before/after image pair exists nothing was screenshotted before the
redesign, and the pre-redesign state no longer exists to photograph. The "before" survives
only as the measurements quoted above.
## Follow-ups
- `docs/prompts/` is gone (see above). If those prompts matter, they need to be rewritten from
intent, by a human.
- Finance's palette has 14 actions where the original record says 15. The current count is
internally consistent (13 screens + logout; Finance has no profile screen), so the
discrepancy is most likely one extra action in the original that is not reconstructible.
- The e2e Phase 0 matrix (`docs/hr-finance-ui-e2e-matrix.md`) and the `e2e-hr-finance/` suite
are both restored, but the suite has **not** been executed in this pass.
- **None of this is committed.** The tree has been lost once already with this work
uncommitted; committing it is the single highest-value next action.