Claude: changes for issue #188 #189

Merged
Grandiras merged 2 commits from claude/issue-188 into main 2026-09-11 01:22:08 +00:00
Collaborator

Closes #188.

Closes #188.
Phone board: give the width back to the cards
All checks were successful
CI / build (pull_request) Successful in 36s
a5f18c16ec
Three insets stack between the edge of a phone and the first letter of a
card — the board's padding, the column's rail, the card's own padding —
and each was worth about 1.15rem, which is what they are worth on a
desktop. There they come out of a 320px column with a page around it; on
a 412px phone the column *is* the page, so the same three were 98px, a
quarter of the device spent before a word, and titles wrapped to two
lines that had no need to.

All three shrink together, to roughly two thirds each (0.5rem / 0.7rem /
0.85rem). Cutting one of them to nothing would have read as crowded:
what makes a board legible at a glance is that the steps page → column →
card are visible, and a column whose cards touch its edges has lost one.
32px of line comes back and a card is ~20% shorter. The phone block also
stops padding .card *more* than the desktop does — it was 1.1rem, filed
under touch targets, but the target is the whole 130px tile.

The column's inset is now one token, --column-rail, because four things
have to start on the same line: the header's label, the hairline under
it, the cards and the add-card button. The hairline was the one that
would have been forgotten — it is an absolutely positioned ::after with
its own copy of the number.

The pagination pill's clearance is unchanged and now written out rather
than folded into a bare `3rem`, so shrinking the padding above it cannot
quietly eat it.

mobile.spec.js measures all three insets, demands each is still there,
demands under a tenth of the screen goes before the text, and checks the
rail's users line up. Verified against a phone viewport with the app
running: text starts at 32.8px rather than 48.8px, the column and the
pill still clear each other by the same margin, and desktop and tablet
renders are pixel-identical.

Closes #188

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Author
Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/464); the reply will link the full session transcript. <!-- claude-run:pr_review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-189/run-464.md) — updating every ~5 minutes while the task runs.
claude-reviewer approved these changes 2026-09-11 01:05:16 +00:00
Dismissed
claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Solid, well-scoped fix for issue #188. --column-rail is introduced once in :root (1.15rem) and reused everywhere the rail was previously a hard-coded 1.15rem (.column-header, .column-filter-panel, .column-cards, .column-add, .column-header::after), then overridden to 0.7rem inside the phone .board rule — since .column/.column-header/.column-cards/.column-add are all descendants of .board, the custom property cascades correctly to all of them. Verified against the actual PR branch file: the phone media query (max-width: 599px, line 6136) now shrinks board padding (0.75rem → 0.5rem), the rail (1.15rem → 0.7rem) and card padding (previously an errant 1.1rem/1.15rem duplicate override further down, now removed) together, and the .column max-height offset is correctly recomputed from 3rem to 0.5rem + 2.25rem to track the new board padding — a detail that would have been easy to miss. No duplicate/conflicting .card padding rule remains after the removal; the only other later .column-add rule in the same media query only touches min-height/padding (touch-target section) and doesn't fight the new margin.

The Playwright changes consolidate three sign-in-heavy tests into one and add assertions for the three insets, the label/hairline alignment via the shared rail token, and a "<10% of viewport before text" budget — all consistent with the CSS change and the numbers claimed in the comments (8px + 11.2px + 13.6px ≈ 32.8px < 37.5px budget). Docs (docs/frontend.md) and both changelog locales are updated per the repo's conventions. I don't see any correctness or omission issues.


⏱ 1m 43s · 18 turns · tokens: 29.1k in / 6.5k out (+809.0k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Solid, well-scoped fix for issue #188. `--column-rail` is introduced once in `:root` (1.15rem) and reused everywhere the rail was previously a hard-coded 1.15rem (`.column-header`, `.column-filter-panel`, `.column-cards`, `.column-add`, `.column-header::after`), then overridden to 0.7rem inside the phone `.board` rule — since `.column`/`.column-header`/`.column-cards`/`.column-add` are all descendants of `.board`, the custom property cascades correctly to all of them. Verified against the actual PR branch file: the phone media query (`max-width: 599px`, line 6136) now shrinks board padding (0.75rem → 0.5rem), the rail (1.15rem → 0.7rem) and card padding (previously an errant 1.1rem/1.15rem duplicate override further down, now removed) together, and the `.column` `max-height` offset is correctly recomputed from `3rem` to `0.5rem + 2.25rem` to track the new board padding — a detail that would have been easy to miss. No duplicate/conflicting `.card` padding rule remains after the removal; the only other later `.column-add` rule in the same media query only touches `min-height`/`padding` (touch-target section) and doesn't fight the new `margin`. The Playwright changes consolidate three sign-in-heavy tests into one and add assertions for the three insets, the label/hairline alignment via the shared rail token, and a "<10% of viewport before text" budget — all consistent with the CSS change and the numbers claimed in the comments (8px + 11.2px + 13.6px ≈ 32.8px < 37.5px budget). Docs (`docs/frontend.md`) and both changelog locales are updated per the repo's conventions. I don't see any correctness or omission issues. --- ⏱ 1m 43s · 18 turns · tokens: 29.1k in / 6.5k out (+809.0k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-189/run-464.md) <!-- claude-verdict:approve --> <!-- claude-reviewed-head:a5f18c16ec19414c98cf5700fb2ec2390be84f9e -->
Phone board: put the column's name on the cards' line, centre the pager
All checks were successful
CI / build (pull_request) Successful in 45s
0dd50523fe
Two follow-ups on the phone board's spacing.

The column header's label sat on `--column-rail`, the line the card *tiles*
start from, while every card title under it sat one card-inset further in —
so the heading hung 14px out to the left of the whole stack. The label is
text and the line text belongs on is the one the titles use, so the phone
block pads it to `calc(var(--column-rail) + var(--card-inset))`. Only the
left: on the right there is no text line to join, just the tiles' edge, and
the count and the filter toggle are already on it. `--card-inset` is a token
for the same reason `--column-rail` is — two rules now have to agree about it.

The pagination pill is fixed above the nav bar while the column above it is
sized by a max-height that leaves room for it, so the gap over the pill and
the gap under it came from two unrelated declarations: 0.4px and 9.6px, which
read as the pill resting on the column rather than standing between it and
the bar. The strip is now `--phone-pager-h` plus two `--phone-pager-gap`, the
column subtracts all of it and the pill is placed one gap up, so the two are
equal by construction. The column does not move; the pill drops 4.6px.

`mobile.spec.js` measures both (each fails on the old CSS), `docs/frontend.md`
carries the reasoning, and the unreleased changelog line gains a clause.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Author
Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/467); the reply will link the full session transcript. <!-- claude-run:pr_review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-189/run-467.md) — updating every ~5 minutes while the task runs.
claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Re-reviewed the second commit that landed since my last pass (0dd5052, "put the column's name on the cards' line, centre the pager") — the first commit (a5f18c1) was already covered and approved.

Verified against the actual PR branch rather than just the diff text:

  • --card-inset is introduced in :root at 1.15rem (matching --column-rail, so no desktop visual change) and reused for .card's horizontal padding; the phone override drops it to 0.85rem, consistent with the phone .card rule further down that overrides only the vertical padding while keeping var(--card-inset) for horizontal.
  • The phone .column-header left padding is calc(var(--column-rail) + var(--card-inset)) = 0.7rem + 0.85rem = 1.55rem, which is what puts the label on the card-title line rather than the tile edge — matches the comment's reasoning and the new labelLeft/textLeft assertion in the spec.
  • --phone-pager-h: 1.625rem — checked this against the actual .board-dot/.board-pagination rules: an 8px dot, no dot padding/border, but the pill (.board-pagination) itself has padding: 0.5rem 0.75rem and border: 1px solid, giving 8px + 2×8px + 2×1px = 26px = 1.625rem. The math the comment claims is correct. .board-dot-active only changes width, not height, so the resting-state height is stable; the drag-expanded pagination bar (min-height 64px) is a pre-existing, transient state this token was never meant to cover.
  • .column's max-height correctly subtracts var(--phone-nav-h) + 0.5rem (board padding) + var(--phone-pager-h) + 2 × var(--phone-pager-gap), and .board-pagination's bottom is var(--phone-pager-gap) + var(--phone-nav-h) — the two gaps are now genuinely the same token, fixing the drift the previous 0.4px/9.6px asymmetry had.
  • The removed .card padding rule in the touch-targets section leaves only a comment behind; the remaining .column-add rule there sets padding (not margin), so it still doesn't conflict with the margin set on .column-add in the board block.
  • mobile.spec.js additions (labelLeft≈textLeft, pager.aboveThePill≈pager.belowThePill) exercise exactly the two new invariants, and the selectors they query (.column-name, .card-title, .board-pagination) all exist.

Docs and both changelog locales are updated consistent with the change. No correctness issues found; nothing here should block merge.


⏱ 1m 11s · 18 turns · tokens: 31.8k in / 6.1k out (+856.1k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Re-reviewed the second commit that landed since my last pass (`0dd5052`, "put the column's name on the cards' line, centre the pager") — the first commit (`a5f18c1`) was already covered and approved. Verified against the actual PR branch rather than just the diff text: - `--card-inset` is introduced in `:root` at `1.15rem` (matching `--column-rail`, so no desktop visual change) and reused for `.card`'s horizontal padding; the phone override drops it to `0.85rem`, consistent with the phone `.card` rule further down that overrides only the vertical padding while keeping `var(--card-inset)` for horizontal. - The phone `.column-header` left padding is `calc(var(--column-rail) + var(--card-inset))` = `0.7rem + 0.85rem = 1.55rem`, which is what puts the label on the card-title line rather than the tile edge — matches the comment's reasoning and the new `labelLeft`/`textLeft` assertion in the spec. - `--phone-pager-h: 1.625rem` — checked this against the actual `.board-dot`/`.board-pagination` rules: an 8px dot, no dot padding/border, but the pill (`.board-pagination`) itself has `padding: 0.5rem 0.75rem` and `border: 1px solid`, giving `8px + 2×8px + 2×1px = 26px = 1.625rem`. The math the comment claims is correct. `.board-dot-active` only changes width, not height, so the resting-state height is stable; the drag-expanded pagination bar (min-height 64px) is a pre-existing, transient state this token was never meant to cover. - `.column`'s `max-height` correctly subtracts `var(--phone-nav-h) + 0.5rem (board padding) + var(--phone-pager-h) + 2 × var(--phone-pager-gap)`, and `.board-pagination`'s `bottom` is `var(--phone-pager-gap) + var(--phone-nav-h)` — the two gaps are now genuinely the same token, fixing the drift the previous 0.4px/9.6px asymmetry had. - The removed `.card` padding rule in the touch-targets section leaves only a comment behind; the remaining `.column-add` rule there sets `padding` (not `margin`), so it still doesn't conflict with the `margin` set on `.column-add` in the board block. - `mobile.spec.js` additions (`labelLeft`≈`textLeft`, `pager.aboveThePill`≈`pager.belowThePill`) exercise exactly the two new invariants, and the selectors they query (`.column-name`, `.card-title`, `.board-pagination`) all exist. Docs and both changelog locales are updated consistent with the change. No correctness issues found; nothing here should block merge. --- ⏱ 1m 11s · 18 turns · tokens: 31.8k in / 6.1k out (+856.1k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-189/run-467.md) <!-- claude-verdict:approve --> <!-- claude-reviewed-head:0dd50523fec91e670d7ed816459e835bc3646b6e -->
Grandiras deleted branch claude/issue-188 2026-09-11 01:22:09 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Grandiras/Ponente!189
No description provided.