mirror of
https://github.com/lrnselfreliance/wrolpi.git
synced 2026-10-26 19:01:39 -04:00
Seven review comments, all valid. The common thread is the one the guard was supposed to catch and did not: geometry converted to rem beside geometry that stayed px. The guard was narrower than its name. It missed: - `transform`, `flex` and `flex-basis`, so `translateX(26px)` and `flex: 1 1 200px` were invisible on properties nobody thought to list; - every NEGATIVE length, because the lookbehind treated the minus sign as part of the preceding token -- `left: -7px` and `margin-top: -2.5px` position the eyelet on a tag whose diamond HAD been converted, and `right: -3px` / `bottom: -3px` place the corner badge on a converted icon. Each is a piece of geometry that stopped tracking the piece it belongs to. With both fixed it found nine values, including one no comment mentioned (`flex: 1 1 200px`). All nine converted, except the three in a dead `.toggle` / `.slider` block: nothing has referenced those classes since Toggle became a Mantine Switch, and it was ALREADY broken -- track and knob in rem, travel still `translateX(26px)`, so the thumb stopped ~4px short of a 54px track. Dead CSS that is also wrong is a trap for whoever revives it, so it is deleted. The tag eyelet is centred with `translateY(-50%)` rather than a negative margin of half its own height, so the two pieces cannot drift again whatever the scale. Also from the review: - A runtime check, which is what the source guards were missing. Three cases in ui-layout.cy.js measure the real thing: the root font-size is the declared multiple of 16, our text AND a Mantine control both grow by the same factor when the scale changes, and a hairline does not. The second is the one that catches `theme.scale` being set and silently cancelling the root. - `max-height: max(..., 56.25cqw)` now follows a plain rem declaration. An engine without container query units throws out the whole declaration, which would have left the poster with NO cap -- a portrait cover standing as tall as the card is wide. Older engines clamp; newer ones take the second. - `pxToRem` in components/ui/scale.ts, replacing the `/ 16` written out in CardGroup and Placeholder. - The map overlays: 20 values across the layer panel, add-pin dialog, search panel and context menu, whose type had grown 10% while their padding had not. Left in px on purpose: the context menu's `left`/`top` (a real click coordinate), maplibre's own `ScaleControl` maxWidth, and the `calc(100% - 100px)` reserve for maplibre's px-sized controls. The `100vh - 120px` reserve DID become rem, since the chrome it reserves for is ours. - The keyboard-shortcuts spec no longer deletes `.mantine-Notifications-root` from the DOM. The reviewer was right that it is brittle; the endpoints are mocked instead, now that the probe named them: `/api/tag`, `/api/events/feed`, `/api/files/search` and `/api/files/worker_status`. Two of those were already "mocked" as `/api/tags` and `/api/events`, which nothing calls -- the wrong paths were the reason a toast appeared at all. Verified under CI's condition (`forceNetworkError` on everything unmocked): 12 passing with no DOM surgery. What is NOT done, deliberately: about 176 numeric geometry values remain in 23 feature files. The map overlays were the visible ones. Sweeping the rest is a change to 23 files nobody has reviewed, so instead the guard's name now says "in our stylesheets" rather than implying the whole app, and a new ratchet keeps numeric geometry out of the component library so the eleventh inline `gap: 10` cannot be added there. Both new guards planted against. Verified: 1142 jest / 65 suites, 251 ui-layout + 10 modal-stack component tests, all 50 e2e specs, clean build, map overlays measured to track the scale (panel minWidth 200 -> 220, search 320 -> 352) and screenshotted. |
||
|---|---|---|
| .. | ||
| api | ||
| components | ||
| contexts | ||
| hooks | ||
| themes | ||
| types | ||
| api.js | ||
| App.css | ||
| App.js | ||
| DashboardPage.js | ||
| DashboardPage.test.js | ||
| Events.js | ||
| import-cycles.test.js | ||
| index.css | ||
| index.js | ||
| logo.svg | ||
| reportWebVitals.js | ||
| semantic-baseline.json | ||
| semantic-baseline.test.js | ||
| setupTests.js | ||
| Tags.cy.js | ||
| Tags.js | ||
| Tags.test.js | ||
| test-fixtures.js | ||
| test-fixtures.test.js | ||
| test-utils.js | ||
| test-utils.test.js | ||