Fix: the chart drew nothing, and mobile truncated every chore name - #16
Merged
Conversation
Both found by rendering the app and looking at it, which I had not done until now. Neither was catchable from the data. The weekly chart rendered completely empty. Each bar column sat in a flex row with items-end, so the column took its content height - zero - and a bar sized as a percentage of nothing is nothing. The underlying numbers were correct throughout; I had verified them through the tooltips and concluded the chart worked. It did not. On a 390px phone the chore row could not fit an icon, a name, a meta line and three buttons, and the casualty was always the name: 'Water the...', 'Take out t...', 'Wash the ...'. The assignee collapsed to a bare emoji and 'two days ago' broke across two lines. The row now wraps, letting the action cluster drop to its own line when space runs out - a few pixels of height in exchange for the words that say what the row is about. Adds scripts/screenshots.mjs, which is what caught both, and puts real screenshots in the README. Playwright is a development-only dependency and the script is deliberately not in CI: a screenshot job that fails on a machine without a browser is a permanently red build for no benefit. 222 tests still green.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Both of these were found by rendering the app and looking at it, which I had not actually done until now. Neither was catchable from the data.
The weekly chart rendered completely empty
Each bar column sat in a flex row with
items-end, so the column took its content height — zero — and a bar sized as a percentage of nothing is nothing.The underlying numbers were correct the whole time. I had verified them through the per-bar tooltips and concluded the chart worked. It didn't: I'd checked the data and inferred the picture.
items-stretchplush-fullon the column fixes it, and the comment now says why those two classes are load-bearing.Mobile truncated every chore name
On a 390px phone the row could not fit an icon, a name, a meta line and three action buttons. The casualty was always the name:
The assignee collapsed to a bare emoji and an ellipsis, and "2 days ago" broke across two lines.
The row now wraps, letting the action cluster drop to its own line when space runs out. That costs a few pixels of height and buys back the words that tell you what the row is actually about.
Screenshots
Adds
scripts/screenshots.mjs— which is what caught both — and puts real screenshots in the README, light and dark.Playwright is a development-only dependency and the script is deliberately not in CI. A screenshot job that fails on a machine without a browser is a permanently red build for no benefit.
Also verified while looking
The money maths, by hand against the rendered page: Ana +£22.26, Ben +£6.86, Cal −£29.12 — sums to zero, settled in two payments for three people, exactly as the algorithm promises.
222 tests still green, lint and typecheck clean.