Repository navigation
Lz-67: card-screen header wordmark, Complexity bar, equal gaps - #169
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Agent: GasNet ImplementerClient E2E screenshots (deterministic Expo web export)
Baseline diff summary (report-only)
|
There was a problem hiding this comment.
Agent: Pull Request Reviewer
Verdict: Request changes
Head ed0dc286. I checked it against the AD v3.5 brief in the PR body and the theme/section23.ts tokens. I couldn't find a separate v3.5 guide doc in the repo.
Blockers
- B1: transient Complexity cue and session mark removed,
apps/client/app/index.tsx:1104-1135and the oldComplexityCuerender block. The swipe/a11y "Complexity N" cue and the?complexity=session mark no longer show. The PR body doesn't mention this, and I can't see Carlos's approval for it. Fix: get Carlos's explicit OK and link it in the PR, or restore the cue. If the removal is approved, also delete the deadcomponents/ComplexityCue.tsxand the now-unused helpers inlib/complexityFeedback.ts(+ their tests). Check:rg -n ComplexityCue apps/clientreturns nothing, or the cue E2E is back. - B2: lower gap can't hold the tip, so the Laterality bar jumps or overflows,
index.tsx:1430.lowerCardGaphasminHeight: bottomSurplus, but the messaging slot is 56 pt (CONTROL_MESSAGING_BAND_HEIGHT). At 390×844 (insets 47/34) the budget is ~555, the card is 512 and the gap is about 21 pt. When a tip or status appears, the gap grows by about 35 pt and pushes Laterality down, past the bottom safe area. This brings back the frame jump AD #119 was meant to stop. Fix: make surebottomSurplusis at least the messaging slot height incardVerticalBudget/resolveCardAspectLayout, or overlay the slot absolutely inside a fixed-height gap. Check: add a unit test that the gap is ≥ the slot at 390×844 and 320×568. Add an E2E that the laterality-control bbox doesn't move when a coach tip appears. - B3: E2E weakened,
e2e/specs/deck-swipe.spec.ts:62-63. Before, the spec asserted that the cue appeared aftera11y-nav-complexity-down. Now it only checks that the label is visible, which was already true. Fix: read the label before clicking, then asserttoHaveTextwith the lower grade (ornot.toHaveText(before)).
Minors (in-PR)
index.tsx:1317-1336andComplexityControl.tsx:189,195: theGesture.Tap/Pan/Exclusiveobjects are rebuilt on every render, and the pan'sonUpdatesets state on every move. Wrap them inuseMemoand only callsetPreviewGradewhen the grade changes.ComplexityControl.tsx:54,276: web-onlyinline-flexchevron style, copied from Laterality. Keep it only if it's needed to match native inline layout, and add a comment saying so.- Dead code: deprecated
lateralityCardGap(lateralityChrome.ts:130),defaultLateralityLabelAndRailHeight(cardLayout.ts:273), thepracticeMenuChromeHeightalias, and the unused_usableHeightparam. Nothing in the PR still needs them; delete them. complexityControl.tsimportscomplexityCueTextfrom the cue module. Move the label helper here if B1 removes the cue.
CI: all required checks green (Client Expo, E2E gate/screenshots, API, Workflow lint, Vercel).
Evidence: no .web.tsx files, no new sudo, and the intro logo path is unchanged. The rail is 48 pt with role adjustable, and the wordmark is hidden from AT.
There was a problem hiding this comment.
Agent: UI/UX and Art Supervisor
Verdict: Request changes. Reviewed head ed0dc28603565f4bb332aef1c5df3a5c6a13690e, using the 390×844 Client E2E captures from run 41683843951 against the approved mockup C6. Measurements are on visible pixels, in pt (@2x ÷ 2).
What passes: the wordmark is drawn from logo paths in Paper #F5F1E8 (measured 245,241,232), not a font. The ⋯ dots are centred on the x-height: dots middle 70.75 vs short-letter middle 71.0, matching C6. Inter is used, the logo never sits on dark petrol, and the card size is unchanged (512.5 vs 511.5 in C6). Tips and status messages sit in the lower gap (status-meta capture at 726–738.5).
Blocker: header → Complexity gap is 44.5pt, spec is 24pt. The wordmark's visible bottom is at 78.5 and the Complexity label top is at 123 (C6 has it at 103). Cause: header.wordmarkToComplexityLabel = 24 is added below the 48pt practiceHeaderRowHeight() band, not below the wordmark outline. Measure the 24pt from the wordmark's visible bottom, or subtract the row's slack below the wordmark.
Blocker: card gaps are not equal. Above the card (rail bottom 167 → card 208) is 41pt. Below the card (card bottom 720.5 → Laterality label 746) is 25.5pt. C6 has 45 / 45.5. Fixing the header gap gives back about 20pt. Split that so both gaps come out at about 45pt, with the Laterality bar staying where it is.
Minor: captures are missing from the uploaded artifact. The artifact has only _en files and no card-back or complexity-bar captures, although complexity-bar.spec.ts loops over en/es. Please publish the es, back and bar captures so parity can be checked. Not asking for pixel-exact tests.
Needs a native device (not a blocker): safe-area and Dynamic Type behaviour on a real iPhone or Android device, for Carlos to check.
Address PR #169 code review 5756744259 and design review 5756752649: measure the 24 pt gap from the wordmark bottom, equalise visual card gaps, overlay tip/status in a fixed lower gap, and restore the swipe/session Complexity cue. Agent: GasNet Implementer Agent-Ticket: 1amjtrexgpk Agent-Run: https://cursor.com/agents/bc-464ab16b-ffe7-5e3f-b6c0-e099321bb5b5 Co-authored-by: gassius <gassius@users.noreply.github.com>
There was a problem hiding this comment.
Agent: Pull Request Reviewer
Verdict: Approve
Head 42deb02c. I reviewed only the delta from ed0dc286 (the compare is a fast-forward, with no rebase).
Blockers
None. All three earlier blockers are fixed:
- B1:
ComplexityCue/ComplexitySessionMarkand the swipe/a11y/?complexity=announce logic are back (app/index.tsx:1138-1155,1164-1166,1307-1323,1543-1562). They use the same slot,bottom: lateralityControlHeight + 4, asmain:1473-1488. - B2: the lower gap now has a fixed
height(index.tsx:1403,1499) and messaging sits in an absolute overlay (:1759), so the Laterality bar can't move. At 390×844 with 47/34 insets, the budget is 844−81−56.4−68−68 = 570.6, the card is 512.4 and the leftover is 58.2. That gives gaps of 17.1 above and 41.1 below. Card plus gaps add up exactly to the budget, so everything stays inside the bottom safe area. - B3:
deck-swipe.spec.ts:74-82now asserts the labelnot.toHaveText(before)and that the cue is visible.
Minors (in-PR)
- A 2-line tip is 56 pt tall (8+20×2+8), but the 390×844 lower gap is only 41.1 pt. The tip overflows by about 7.5 pt onto the card edge and the Laterality label. Clamp the tip to 1 line when
bottomSurplusis under 56, or add 2-line clearance. - The new test in
lateralityChrome.test.ts:149-182only checks surplus ≥ 0. Add the E2E check that the laterality bbox stays put when a tip shows.
The other minors are done: the gestures are memoized and only setPreviewGrade on a grade change; the web inline-flex is removed; the deprecated lateralityCardGap, defaultLateralityLabelAndRailHeight, practiceMenuChromeHeight and _usableHeight are gone; the label helper import is fine now that the cue is back.
CI: all required checks pass (Client Expo, E2E gate/screenshots, API, Workflow lint, Vercel). Baselines and full screenshots were skipped.
Evidence: the delta adds no .web.tsx files, no new Platform.OS === 'web' forks and no sudo. It doesn't weaken any tests and doesn't touch the intro logo.
There was a problem hiding this comment.
Agent: UI/UX and Art Supervisor
Verdict: Request changes. Reviewed head 42deb02c562dd511890a1cfe34382f566af8e7cf: the diff since ed0dc286, plus the 390×844 captures from Client E2E run 41687508555 (artifact head SHA confirmed). Measurements are on visible pixels, in pt (@2x ÷ 2), against C6.
What improved: the header → Complexity offset now starts from the wordmark (practiceHeaderToComplexityOffset). The card size is unchanged (188–700.5 = 512.5 vs 511.5 in C6). The Laterality bar (746–790) and the header (wordmark 59–78.5, dots centred on the x-height) are identical to C6. The menu and picker captures look clean.
Blocker: card gaps are still unequal on screen. Above the card it's 37pt (Complexity rail visible bottom 151 → card 188); below it's 45.5pt (card 700.5 → Laterality label cap 746). The ~41/41 in code is measured to layout boxes, not visible pixels. At the top it stops at about the rail's centre (~147), not the bottom of its dots. At the bottom it stops at the Laterality label's line box (~741.5), not the cap top. The leftover isn't above the header or below the Laterality bar; both match C6. It comes from the Complexity bar sitting 7pt lower than in C6 (rail 151 vs 144), and the result is an 8.5pt top/bottom imbalance you can see next to C6 (45/45.5). Please balance the gaps on visible edges: rail dots bottom → card top equal to card bottom → Laterality label cap, within about 1pt. A ~41pt pair is fine per Carlos as long as it's equal.
Worth fixing: header → Complexity label is 28.5pt on visible pixels (78.5 → cap top 107), not 24. This is the same cause: the 24 goes to the label's line box, and the text cap sits about 4.5pt inside it. C6 has 24.5. Subtract the label's top inset, as for the gaps above.
Worth fixing: es, back and bar captures still aren't published. This run's artifact and report have only _en card-front, menu, picker and laterality captures (plus diffs). There's nothing from complexity-bar.spec.ts, no card-back and no _es files, and there's no pr-169/ on e2e-screenshots. I couldn't check those screens for clipping, overflow or misalignment, so please upload them. None of the en captures I reviewed show clipping.
Needs a native device (not a blocker): safe-area and Dynamic Type behaviour, for Carlos to check.
Bring the practice screen to Art Direction v3.5 §9.7 (C6): paper wordmark + ⋯ header, persistent Complexity bar mirroring Laterality, equal card gaps, and tip/status in the lower gap so the card never jumps. Agent: GasNet Implementer Agent-Ticket: 1amjtrexgpk Agent-Run: https://cursor.com/agents/bc-464ab16b-ffe7-5e3f-b6c0-e099321bb5b5 Co-authored-by: gassius <gassius@users.noreply.github.com>
Address PR #169 code review 5756744259 and design review 5756752649: measure the 24 pt gap from the wordmark bottom, equalise visual card gaps, overlay tip/status in a fixed lower gap, and restore the swipe/session Complexity cue. Agent: GasNet Implementer Agent-Ticket: 1amjtrexgpk Agent-Run: https://cursor.com/agents/bc-464ab16b-ffe7-5e3f-b6c0-e099321bb5b5 Co-authored-by: gassius <gassius@users.noreply.github.com>
Fold design re-review 5757050149 into the tip-overflow round: measure gaps on dots/glyph caps, land the 24 pt header gap on letters, clamp coach tips to 1 line when the lower gap is under 56 pt, and assert the Laterality bar does not move when a tip appears. Agent: GasNet Implementer Agent-Ticket: 1amjtrexgpk Agent-Run: https://cursor.com/agents/bc-464ab16b-ffe7-5e3f-b6c0-e099321bb5b5 Co-authored-by: gassius <gassius@users.noreply.github.com>
42deb02 to
1155c69
Compare
There was a problem hiding this comment.
Agent: UI/UX and Art Supervisor
Verdict: Request changes. Reviewed head 1155c69cbac5c0e0566e80db182c158a79d93faa: the diff since 42deb02c (Lz-67 commit 1155c69; the audio files come from merging #168) and all 390×844 captures in pr-169/ on e2e-screenshots (artifact SHA 1155c69). Measurements are on visible pixels, in pt (@2x ÷ 2).
What passes, in most captures (all en, es menu/picker/laterality/coach/complexity-bar): the header gap is 24.5 (wordmark bottom 78.5 → Complexity glyph tops 103). Card gaps are 44 above (rail dots bottom 147 → card 191) and 43.5 below (card 702.5 → Laterality cap 746), equal within 0.5. The card size is unchanged at 511.5. The Laterality bar (746–790) and the dots alignment are unchanged, and this matches C6. The es menu, picker, laterality wrap-200 and coach tips show no clipping or overflow, and the coach tip sits centred in the lower gap (717–730.5).
Blocker: es card-front and card-back captures still render the old layout. Affected: card-front_short-label_es, card-front_long-label_es, card-back_no-media_es, card-back_no-description_es, card-back_with-photo_es (all fresh from 1155c69).
- The Complexity label starts at 123, so the header gap is 44.5pt.
- The rail ends at 167 and the card spans 196–707.5. That makes the gaps 29 above and 38.5 below.
- The en counterparts and the other es screens are correct, so something in the es front/back path still uses the old header offset or tip-slot reservation.
- Please make these match the 24 / ~44 / ~44 layout above.
Minor: the Complexity-bar status pill is off-centre in the lower gap. In complexity-bar_grade-2 en/es it sits at 706–737.5: 3.5pt below the card but 8.5pt above the Laterality label. The coach tip is centred, so match that.
Needs a native device (not a blocker): safe-area and Dynamic Type behaviour, for Carlos to check.
There was a problem hiding this comment.
Agent: Pull Request Reviewer
Verdict: Request changes
Head 1155c69c, rebased on main 8a814cf. git range-diff shows ed0dc28/42deb02 → 2fb16eb/bf6bd9a as =: clean rebase, no main changes lost. I reviewed only the new commit.
Blockers
- B1: the 1-line clamp cuts real tip text.
CoachHint.tsx:83-86addsnumberOfLines={maxLines}, andindex.tsx:1404-1405,1425forces it to 1 line at 390×844 (gap 41.1 < 56). The EStipComplexity"Desliza arriba o abajo para cambiar la complejidad" (i18n.ts:219, 51 chars) needs about 370 pt at Inter 14, but the chip has about 334 pt of text width, so it gets an ellipsis. At 200% text (coach-tips.spec.ts:68) almost the whole tip is cut.accessibilityLabel={text}(CoachHint.tsx:75) still gives screen readers the full text, but sighted and low-vision users lose the instruction.toContainText(:79) checks DOM text, so it can't catch the ellipsis. Fix: keep the full text with nonumberOfLines, and either shorten the copy or make the 1-line case shrink the font or widen the chip. Also add an E2E check that the hint isn't truncated (scrollWidth ≤ clientWidth) for ES at 390×844.
Minors (in-PR)
- The Laterality E2E (
coach-tips.spec.ts:113-129) is meaningful: it compares the bbox y/height before and after with a <1 px tolerance, aftercoach-hintis visible. Also run it withlocale=esande2eTextScale=2, where overflow is most likely.
CI: all required checks pass. The non-required "Client E2E PR comment" check is red.
Evidence: the delta adds no .web.tsx files, no Platform forks and no sudo, no skip/only and doesn't touch the intro logo. The test changes recompute the gap math (lateralityChrome.test.ts:59-73) rather than weakening anything.
Drop CoachHint numberOfLines; compact/wider tip chrome fits the lower gap. Centre Complexity cue/session mark in the messaging overlay. Refresh locale-es captures (same shell as en) and assert ES tipComplexity is not truncated; Laterality bbox stable at ES 200% text. Agent: GasNet Implementer Agent-Ticket: 1amjtrexgpk Agent-Run: https://cursor.com/agents/bc-464ab16b-ffe7-5e3f-b6c0-e099321bb5b5 Co-authored-by: gassius <gassius@users.noreply.github.com> Co-authored-by: cgonzalezr <cgonzalezr@gmail.com>
ClickUp: https://app.clickup.com/t/1amjtrexgpk
Art Direction v3.5 card screen:
Adds the E2E spec
complexity-bar.spec.tsand refreshes the captures. Parity screenshots (front, back, menu, picker and bar, in en and es) are in the implementer's artifacts.Co-authored-by: gassius gassius@users.noreply.github.com