Claude: changes for issue #188 #189
No reviewers
Labels
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
Grandiras/Ponente!189
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "claude/issue-188"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #188.
🤖 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.
✅ Verdict: approve — no blocking issues found.
Solid, well-scoped fix for issue #188.
--column-railis 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.boardrule — since.column/.column-header/.column-cards/.column-addare 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.columnmax-heightoffset is correctly recomputed from3remto0.5rem + 2.25remto track the new board padding — a detail that would have been easy to miss. No duplicate/conflicting.cardpadding rule remains after the removal; the only other later.column-addrule in the same media query only touchesmin-height/padding(touch-target section) and doesn't fight the newmargin.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
🤖 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.
✅ 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-insetis introduced in:rootat1.15rem(matching--column-rail, so no desktop visual change) and reused for.card's horizontal padding; the phone override drops it to0.85rem, consistent with the phone.cardrule further down that overrides only the vertical padding while keepingvar(--card-inset)for horizontal..column-headerleft padding iscalc(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 newlabelLeft/textLeftassertion in the spec.--phone-pager-h: 1.625rem— checked this against the actual.board-dot/.board-paginationrules: an 8px dot, no dot padding/border, but the pill (.board-pagination) itself haspadding: 0.5rem 0.75remandborder: 1px solid, giving8px + 2×8px + 2×1px = 26px = 1.625rem. The math the comment claims is correct..board-dot-activeonly 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'smax-heightcorrectly subtractsvar(--phone-nav-h) + 0.5rem (board padding) + var(--phone-pager-h) + 2 × var(--phone-pager-gap), and.board-pagination'sbottomisvar(--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..cardpadding rule in the touch-targets section leaves only a comment behind; the remaining.column-addrule there setspadding(notmargin), so it still doesn't conflict with themarginset on.column-addin the board block.mobile.spec.jsadditions (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