Add the A4 report designer and fix band layout
Brings in the A4 (millimetre) report designer alongside the existing matrix editor, migrates the frontend from react-scripts to Vite/Vitest, and fixes four defects found while using the A4 band tools. Layout — layout.js now implements the standard banded-report model: everything prints where it was drawn, and a band growing per data row is the only thing that moves anything. Previously `paginate` re-flowed every band from the top margin, so a band placed halfway down the page previewed at the top, and `header` was treated as a page header and hoisted above the statics drawn before it. A header is a COLUMN header: it prints in place and repeats only on a continuation page. Statics now take part too — one drawn below a band keeps its drawn gap as that band grows, and follows onto the next page rather than vanishing when pushed past the bottom margin. The displacement is keyed on the band's design bottom, not on an instance's laid-out y: every row of a band shares one design position, so keying on the instance measured the gap from row 1 and let the static overlap rows 2..n. Band children — a child's x is relative to its band, as its y already was. layout() emitted it raw, so a field printed measured from the paper edge instead of the band's left margin. Instances now carry bandX. Band placement — a band's x comes from the page margins, so creation takes only the click's Y (the factory's x was being overwritten by the spread order) and dragging is locked to the vertical axis. Bands render as a strip, with top/bottom rules only, matching the matrix editor. fromApiReport reflows on load so reports saved with a stray x straighten themselves. Matrix bands are constrained to vertical too. Drag lifecycle — useElementDrag re-subscribed its document listeners on every render during a drag, because the effect depended on an unmemoized onDrag. Each re-subscribe removed the mouseup listener before adding the next, measured as add/remove/add/remove/add across two mousemoves, and a release landing in that gap was lost: the element kept following the cursor until the next click. Snapping made it routine, since a held guide stops x/y changing and leaves the guide overlay's render sitting between the move and the release. onDrag and dragData now go through refs so the effect depends on [isDragging] alone. Note: the matrix editor has no test coverage, so its band constraint is verified by reading only. The A4 suite is 201 passing tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -8,8 +8,22 @@
|
||||
- Inspector CSS: `ObjectInspector.css`
|
||||
- Data resolver utils: `utils/dataResolver.js` — `resolveDBTextValue`, `getAvailableObjects`, `getAvailableFields`
|
||||
- Expression evaluator: `utils/expressionEvaluator.js` — recursive-descent, no eval
|
||||
- A4 designer (sibling tree, mm-based): `frontend/src/components/ReportEditor/a4/`
|
||||
|
||||
## Element Type Pattern
|
||||
## Two editors, deliberately duplicated
|
||||
|
||||
The matrix editor (`EditorCanvas.jsx`, `models/Element.jsx`, `ResizeHandles.jsx`,
|
||||
`ObjectInspector.jsx`, `ReportEditor.jsx`) is treated as frozen. The A4 designer under
|
||||
`a4/` copies similar logic rather than extracting shared abstractions — per the plan at
|
||||
`.claude/plans/a4-report-designer.md`. Do not "DRY up" the two; the duplication is the point.
|
||||
|
||||
Key unit difference: matrix bounds are **inclusive** (`x + width - 1`); A4 bounds in
|
||||
`a4/geometry.js` are **exclusive** (a 40mm box at x=10 has right edge 50).
|
||||
|
||||
Safely reusable by both: `hooks/useElementDrag.jsx` (emits raw pixel deltas, snapping left
|
||||
to the consumer), `hooks/useElementSelection.jsx`, `DataContext`, `BandContext`.
|
||||
|
||||
## Element Type Pattern (matrix editor)
|
||||
|
||||
Adding a new element type requires touching 8 files:
|
||||
1. `models/Element.jsx` — new class + register in `elementClasses` map in `fromJSON`
|
||||
@@ -50,3 +64,20 @@ resolveDBTextValue(reportData, objectKey, fieldPath, parentBandId, { currentBand
|
||||
|
||||
`[objectKey.fieldPath]` for top-level fields, `[fieldPath]` for band-relative fields.
|
||||
Strip brackets, split on first `.` to get objectKey and fieldPath.
|
||||
|
||||
## Build tooling
|
||||
|
||||
**Vite + Vitest** (no CRA/Jest). `npm run dev` / `npm start` → port 5172;
|
||||
`npm test` → `vitest run`; `npm run build` → `frontend/build/`.
|
||||
Config: `frontend/vite.config.mjs`. Env vars are `import.meta.env.VITE_*`.
|
||||
**Any new production file containing JSX must be named `.jsx`** — a `.js` file
|
||||
with JSX passes tests and the dev server but fails `vite build`.
|
||||
See [vite-vitest-migration](vite-vitest-migration.md).
|
||||
|
||||
## Detailed notes
|
||||
|
||||
- [Vite + Vitest build tooling](vite-vitest-migration.md) — the JSX-in-.js build trap, axios mocks, jsdom `border: none`
|
||||
- [A4 entry points](a4-entry-points.md) — report `kind` decides the editor/print route; the two contexts A4ReportEditor now needs in tests
|
||||
- [A4 smart guides](a4-smart-guides.md) — px-not-mm snap threshold; why useElementDrag must stay untouched; the drag-test gotcha snapping introduces
|
||||
- [Testing React in this repo](testing-react.md) — no testing-library installed; jsdom/React event gotchas that cost real debugging time
|
||||
- [Decimal number inputs](decimal-inputs.md) — why `type="number"` breaks mm/float entry
|
||||
|
||||
@@ -0,0 +1,44 @@
|
||||
---
|
||||
name: a4-designer-conventions
|
||||
description: Non-obvious invariants of the A4 report designer — line element shape, mm-vs-px flooring, and why matrix inspector panels must not be imported
|
||||
metadata:
|
||||
type: project
|
||||
---
|
||||
|
||||
Conventions in `frontend/src/components/ReportEditor/a4/` that are easy to get
|
||||
wrong because they contradict the matrix editor sitting next to them.
|
||||
|
||||
**Lines carry only one dimension.** An `a4-hline` has `width` + `thickness`
|
||||
and NO `height`; an `a4-vline` has `height` + `thickness` and NO `width`.
|
||||
Anything computing a box from an element must substitute `thickness` for the
|
||||
missing axis — `getA4Bounds` in `geometry.js` does this, and it is the
|
||||
function to reach for rather than reading `el.width`/`el.height` directly.
|
||||
Reading the raw props for a line compares against `undefined` and passes
|
||||
vacuously in tests while being wrong in the product.
|
||||
|
||||
**Screen floors hairlines, print must never.** Screen renderers clamp a line's
|
||||
painted thickness and a rect's border to `Math.max(1, …)` px, because 0.3mm at
|
||||
50% zoom is 0.57px and browsers can round it away to nothing. `PrintDocument`
|
||||
deliberately does no such thing — the printed thickness is what a steel rule
|
||||
measures. If a test asserts a floored value on the print path, the test is
|
||||
wrong, not the code.
|
||||
|
||||
**Do not import the matrix inspector panels.** `ExpressionEditorPanel.jsx`
|
||||
looks tempting for the A4 expression section but is coupled two ways: it finds
|
||||
a parent band with `el.type === 'band'` (A4 bands are `a4-band`) and its
|
||||
alignment select writes `element.alignment`, whereas every A4 element and
|
||||
renderer reads `element.align`. Reusing it silently creates a dead second
|
||||
alignment property. The duplication between the two inspectors is accepted
|
||||
deliberately — see the plan's "matrix editor is not modified" section.
|
||||
|
||||
**`fillColor: null` means no fill,** distinct from white, which would hide
|
||||
whatever a rect is framing. A colour input cannot express null, so a checkbox
|
||||
owns that state. It round-trips through Django's JSONField as SQL null.
|
||||
|
||||
**Float inputs use `MmInput`,** never `<input type="number">`: a number input
|
||||
returns `""` for the intermediate value `"12."`, so the decimal point appears
|
||||
not to type. `MmInput` (in `A4Inspector.jsx`) holds a draft string while
|
||||
focused. Font size in pt is the one exception — its step is 0.5 and users
|
||||
never type a long decimal tail there.
|
||||
|
||||
Related: [[vite-vitest-migration]]
|
||||
@@ -0,0 +1,46 @@
|
||||
---
|
||||
name: a4-entry-points
|
||||
description: How the two report editors are reached and dispatched between — kind decides the route, and why the A4 editor now needs Router + NomenclatureData context in tests
|
||||
metadata:
|
||||
type: project
|
||||
---
|
||||
|
||||
Phase 7 wired the A4 designer to real operator entry points. Three things here
|
||||
are easy to get wrong later.
|
||||
|
||||
**Report `kind` decides the route, never a default.** `Main.jsx` fetches
|
||||
`/api/reports/?name=notice_report` and branches: `kind === 'a4'` → dynamic
|
||||
`import()` of `a4/print/printReport` and `printA4Report(report, data)`;
|
||||
otherwise the existing `ReportPreviewModal`. "Design Notice" routes to
|
||||
`/report-editor-a4` or `/report-editor` by the same field.
|
||||
|
||||
**Why it matters:** a matrix report opened in the mm editor reads its character
|
||||
x/y as millimetres and looks like a corrupt layout rather than a routing bug —
|
||||
a slow, confusing failure. The A4 editor's auto-load therefore filters on
|
||||
`?kind=a4&name=...`, not name alone; verified against the real DB that
|
||||
`?kind=a4&name=<a matrix report>` returns 0 rows.
|
||||
|
||||
**How to apply:** any new entry point that opens a saved report must resolve
|
||||
`kind` before choosing an editor or a render path. Never assume a name implies
|
||||
a kind.
|
||||
|
||||
**`A4ReportEditor` now requires two contexts.** It calls `useLocation`/
|
||||
`useNavigate` (router-state auto-load) and `useNomenclatureData` (sample-data
|
||||
fallback). Both throw outside their providers, so any jsdom test mounting the
|
||||
whole editor needs `<MemoryRouter>` plus
|
||||
`vi.mock('../../../../contexts/NomenclatureDataContext', ...)`. Mocking the
|
||||
nomenclature hook rather than standing up the provider keeps the SSE client and
|
||||
auth context out of tests that are really about drag geometry.
|
||||
|
||||
**Why:** adding these deps silently broke four existing `a4Guides` tests with
|
||||
"useLocation() may be used only in the context of a <Router>" — the component
|
||||
was fine, the harness was stale.
|
||||
|
||||
**`window.print()` is not gated by transient user activation** the way
|
||||
`window.open` is, so an `await` before it is safe. The A4 print chunk is still
|
||||
prefetched on hover/focus of the Print Notice button, purely to shorten the gap
|
||||
on a slow station. This was reasoned, not verified in a real browser — if Print
|
||||
Notice is ever reported as doing nothing on click, re-test that assumption
|
||||
first.
|
||||
|
||||
Related: [[a4-print-path]], [[a4-designer-conventions]], [[testing-react]]
|
||||
@@ -0,0 +1,35 @@
|
||||
---
|
||||
name: a4-print-path
|
||||
description: Two non-obvious constraints in the A4 report print path — the print-root must be a direct body child, and flushSync (not rAF) must precede window.print()
|
||||
metadata:
|
||||
type: project
|
||||
---
|
||||
|
||||
Two constraints in `frontend/src/components/ReportEditor/a4/print/` that are easy to
|
||||
silently break and whose failure mode is a blank or wrong printed page.
|
||||
|
||||
**1. `.print-root` must sit on the element appended directly to `<body>`.**
|
||||
`print.css` hides the app with `body > *:not(.print-root)` — a CHILD combinator. The app
|
||||
mounts into `<div id="root">`, itself a direct body child, so the print container is its
|
||||
sibling. If the class lived only on `PrintDocument`'s inner div, the appended container
|
||||
would match the hiding rule and print a blank page.
|
||||
|
||||
**Why:** the selector is the entire mechanism for hiding the editor during print; there is
|
||||
no other switch.
|
||||
**How to apply:** when changing how the print container mounts, keep the class on the
|
||||
container itself. `printReport.js` sets it; `PrintDocument` repeats it for the standalone
|
||||
case.
|
||||
|
||||
**2. `flushSync` — never `requestAnimationFrame` — before `window.print()`.**
|
||||
`window.print()` reads the DOM synchronously while React 18 commits asynchronously, so an
|
||||
unflushed render prints an empty container. rAF was the first fix tried and is wrong: it
|
||||
does not fire in a background or hidden tab, so a print click would hang instead of
|
||||
printing.
|
||||
|
||||
**Why:** discovered when tests showed `window.print` never being reached behind a
|
||||
double-rAF await; the background-tab case is the production version of the same bug.
|
||||
**How to apply:** any new print entry point wraps its `root.render` in `flushSync` from
|
||||
`react-dom`.
|
||||
|
||||
Related: `.a4-print-page` is deliberately NOT `.a4-page` — `A4Canvas.css` already owns
|
||||
`.a4-page` for the on-screen editor canvas, and both stylesheets load together.
|
||||
@@ -0,0 +1,35 @@
|
||||
# A4 smart guides (`a4/guides.js`)
|
||||
|
||||
Pure functions, no React/DOM. `buildGuideCandidates(elements, excludeIds, pageSetup)`
|
||||
→ `{V, H}`; `resolveSnap(draftRect, candidates, zoom)` → `{x, y, guides}`;
|
||||
plus `unionRect(boxes)` and `snapToleranceMm(zoom)`.
|
||||
|
||||
## The threshold is in SCREEN PIXELS, not mm
|
||||
|
||||
`SNAP_PX = 6`, converted per-call via `SNAP_PX / (PX_PER_MM * zoom)`. This is
|
||||
load-bearing, not a detail: a fixed mm threshold feels sticky zoomed in and
|
||||
useless zoomed out. Do not "simplify" it to a constant mm value. The property
|
||||
worth asserting is `tolMm * PX_PER_MM * zoom === SNAP_PX` at every zoom.
|
||||
|
||||
## Integration seams in `A4Canvas.jsx`
|
||||
|
||||
- Candidates are built **once** in `handleElementDragStart` into a `useRef`.
|
||||
Rebuilding per mousemove is the one thing that makes dragging laggy.
|
||||
- Multi-select snaps the **union rect**, then applies one delta to every member.
|
||||
Snapping members independently tears the group apart.
|
||||
- `hooks/useElementDrag.jsx` is shared with the matrix editor and forwards only
|
||||
pixel deltas — **never widen it**. Alt-to-suppress is tracked with a
|
||||
window-level `keydown`/`keyup` listener in A4Canvas, plus a `blur` reset (an
|
||||
alt-tab otherwise leaves snapping permanently off, because the keyup lands in
|
||||
another window).
|
||||
- Bands emit no candidates (their edges *are* the margins, already candidates),
|
||||
but band children do.
|
||||
|
||||
## Gotcha: snapping breaks drag tests that assume raw deltas
|
||||
|
||||
Any pre-existing test that drags an element and asserts the exact px→mm result
|
||||
can fail once a candidate happens to sit within ~1.6mm of where it lands. This
|
||||
bit `a4Elements.test.js`'s rect case: it drifted 0.05mm from the 20mm left
|
||||
margin and legitimately snapped. The fix is to hold Alt around the drag
|
||||
(`withAltHeld` helper in that file), not to weaken the assertion — the test is
|
||||
about unit conversion, so switching the magnet off states its intent.
|
||||
@@ -0,0 +1,40 @@
|
||||
# Decimal / float number inputs
|
||||
|
||||
Rule: for a field where the user types a **decimal** value (millimetres, rates, any float),
|
||||
use `type="text"` + `inputMode="decimal"` with local draft-string state — **not**
|
||||
`type="number"`.
|
||||
|
||||
## Why
|
||||
|
||||
Two independent failure modes combine:
|
||||
|
||||
1. **`type="number"` sanitises partial input.** While the user is typing `12.7`, the
|
||||
intermediate value is `"12."`, which is not a valid number. The browser (and jsdom)
|
||||
report `input.value === ""`. The decimal point appears not to type at all.
|
||||
|
||||
2. **A naive `parseFloat` + `isNaN` guard drops the same intermediate.** `parseFloat("12.")`
|
||||
is `12`, so a commit-on-every-change handler rewrites the field to `"12"` and the caret
|
||||
jumps — same visible symptom, different cause.
|
||||
|
||||
## The working pattern
|
||||
|
||||
Keep the raw string in local state while focused; commit only when it parses cleanly;
|
||||
re-sync from the prop only when not focused (so drag/resize updates still flow in).
|
||||
|
||||
```jsx
|
||||
const [draft, setDraft] = useState(String(value ?? ''));
|
||||
const [isFocused, setIsFocused] = useState(false);
|
||||
|
||||
useEffect(() => {
|
||||
if (!isFocused) setDraft(String(value ?? ''));
|
||||
}, [value, isFocused]);
|
||||
|
||||
// onChange: setDraft(next) always; commit only if next is a complete number
|
||||
// onBlur: if the draft is empty/unparseable, restore String(value); else commit rounded
|
||||
```
|
||||
|
||||
Reference implementation: `MmInput` in
|
||||
`frontend/src/components/ReportEditor/a4/A4Inspector.jsx`.
|
||||
|
||||
Note the matrix editor's `ObjectInspector.jsx` uses `parseInt` and integer cells, so it
|
||||
does not need this — only float-valued fields do.
|
||||
@@ -0,0 +1,65 @@
|
||||
# Testing React components in ScalesApp
|
||||
|
||||
## What's available
|
||||
|
||||
- **Vitest + jsdom** (`npm test` → `vitest run`). Run a single suite with:
|
||||
`npx vitest run <path-to-test-file>`. Use `vi.*` (not `jest.*`); `globals: true`
|
||||
is set, so `describe`/`test`/`expect`/`vi` need no import. See
|
||||
[[vite-vitest-migration]].
|
||||
- **`@testing-library/react` is NOT installed.** Don't assume it; mount with
|
||||
`createRoot` from `react-dom/client` and `act` from `react` instead.
|
||||
- There is no `src/setupTests.js`. Set `global.IS_REACT_ACT_ENVIRONMENT = true` in the
|
||||
test file itself, or React logs "testing environment is not configured to support act(...)".
|
||||
- Import `act` from `react`, not `react-dom/test-utils` (the latter is deprecated in React 18
|
||||
and prints a warning on every call).
|
||||
|
||||
## Three gotchas that produce fake failures
|
||||
|
||||
These each look like a component bug and are not. Verify with a tiny probe test before
|
||||
"fixing" component code.
|
||||
|
||||
1. **React's `onBlur`/`onFocus` map to native `focusout`/`focusin`.**
|
||||
Dispatching `new FocusEvent('blur')` does nothing — the handler never runs, so the
|
||||
component looks broken. Dispatch `focusout`/`focusin` instead.
|
||||
|
||||
2. **`MouseEvent` coerces `clientX`/`clientY` to integers.**
|
||||
Passing a fractional pixel coordinate silently truncates. In a mm-based editor that
|
||||
shows up as a ~0.3mm discrepancy that looks like a rounding bug in the conversion code.
|
||||
Use whole pixels in tests and derive expected values from those same integers.
|
||||
|
||||
3. **`<input type="number">` in jsdom returns `""` for a partial value like `"12."`.**
|
||||
Real browsers sanitise similarly. See [[decimal-inputs]].
|
||||
|
||||
## jsdom has no layout
|
||||
|
||||
`getBoundingClientRect()` returns all zeros. For canvas/positioning components, stub it
|
||||
per test to pin a known origin:
|
||||
|
||||
```js
|
||||
Element.prototype.getBoundingClientRect = function () {
|
||||
if (this.classList.contains('a4-page')) {
|
||||
return { left: 100, top: 50, /* ...*/ toJSON() {} };
|
||||
}
|
||||
return { left: 0, top: 0, /* ... */ toJSON() {} };
|
||||
};
|
||||
```
|
||||
|
||||
## Driving controlled inputs
|
||||
|
||||
React overrides the `value` setter, so `input.value = x` alone won't trigger `onChange`.
|
||||
Use the native setter, then dispatch `input`:
|
||||
|
||||
```js
|
||||
const setter = Object.getOwnPropertyDescriptor(
|
||||
window.HTMLInputElement.prototype, 'value'
|
||||
).set;
|
||||
setter.call(input, '12.');
|
||||
input.dispatchEvent(new Event('input', { bubbles: true }));
|
||||
```
|
||||
|
||||
## Backend round-trips without a live server
|
||||
|
||||
Outbound HTTP from the agent sandbox may be blocked even when the dev server is running.
|
||||
To exercise real DRF serializers/filters/DB, use Django's test client directly against the
|
||||
dev database — pass `APIClient(SERVER_NAME='localhost')`, since the default `testserver`
|
||||
host is rejected by `ALLOWED_HOSTS`.
|
||||
@@ -0,0 +1,52 @@
|
||||
---
|
||||
name: vite-vitest-migration
|
||||
description: Frontend build is Vite + Vitest (not CRA/Jest) — the JSX-in-.js trap, why axios no longer needs stubbing, and the jsdom border assertion gotcha
|
||||
metadata:
|
||||
type: project
|
||||
---
|
||||
|
||||
The frontend migrated from Create React App / Jest to **Vite + Vitest**. Config
|
||||
lives in `frontend/vite.config.mjs` (`.mjs`, not `.js` — Vite's native config
|
||||
loader warns about ESM in a `.js` config).
|
||||
|
||||
Replaces the old `jest-axios-esm` note, which is now obsolete.
|
||||
|
||||
**Vitest transforms axios fine, so the old blanket `jest.mock('axios')` hack is
|
||||
gone.** Three of the six a4 suites dropped it entirely. `a4EntryPoints` still
|
||||
mocks axios, but for a *behavioural* reason: it drives the report auto-load via
|
||||
`api.get.mockResolvedValue(...)`, so the instance `services/api.js` builds at
|
||||
import time must be a mock. Distinguish the two reasons before deleting a mock.
|
||||
|
||||
**The JSX-in-`.js` trap — this one fails at BUILD time only.** CRA's Babel
|
||||
parsed JSX inside `.js` files; Vite does not by default. Setting
|
||||
`oxc: { include: /\.(m?ts|[jt]sx|js)$/, lang: 'jsx' }` fixes the dev server and
|
||||
Vitest, but **not** `vite build`: the bundled path (Rolldown,
|
||||
`builtin:vite-transform`) derives the language from the file extension and
|
||||
ignores `lang`. So a `.js` file containing JSX will pass tests and the dev
|
||||
server, then fail the production build.
|
||||
|
||||
**How to apply:** any new production file containing JSX must be named `.jsx`.
|
||||
The `oxc` override exists only so the existing a4 `__tests__/*.test.js` files
|
||||
keep working. Importers use extensionless specifiers, so renaming a file
|
||||
`.js` → `.jsx` needs no import changes (this is how `print/printReport.jsx` was
|
||||
renamed without touching the dynamic `import()` in `Main.jsx`).
|
||||
|
||||
**Env vars are `import.meta.env.VITE_*`**, not `process.env.REACT_APP_*`;
|
||||
`process.env` does not exist in the browser. The `|| 'http://localhost:…'`
|
||||
fallbacks are the offline-deployment safety net — keep them. The dev-server
|
||||
port (5172, in `vite.config.mjs`) must match Django's `CORS_ALLOWED_ORIGINS`.
|
||||
Renaming these vars means touching `Dockerfile`, `docker-compose.yml`,
|
||||
`.env.docker` and the gitignored root `.env` too, or the deployed bundle
|
||||
silently falls back to localhost.
|
||||
|
||||
**Newer jsdom round-trips `border: none`.** It now reports
|
||||
`borderStyle === 'none'` with `borderWidth === 'medium'` (the CSS initial
|
||||
keyword). Assert on **`borderStyle`**, not `borderWidth === ''` — the latter was
|
||||
written around an old-jsdom quirk and passed vacuously. Two `a4Elements` tests
|
||||
had to be corrected for this during the migration; the components were correct.
|
||||
|
||||
**`global.IS_REACT_ACT_ENVIRONMENT = true` is no longer required** (Vitest's
|
||||
jsdom environment sets it), but it is harmless and still present in all six
|
||||
suites.
|
||||
|
||||
Related: [[testing-react]], [[a4-print-path]], [[a4-entry-points]]
|
||||
Reference in New Issue
Block a user