fix(habits): render habit tags on the Today row and detail drawer - #304
Conversation
Tags were stored and editable but never shown outside the edit modal. NormalizedHabit already carries `tags`, so this is a client-only render — no API or shared-type change.
- Today row: up to 3 tag chips (color dot + name) on a single non-wrapping line, then a +N counter, so a habit with up to 10 tags never grows the row height. - Detail drawer: all tags, wrapping, centred under the title (full-view surface; the edit modal already lists all). - Web + mobile parity.
Tests: row tag render + 3-cap/+N on web and mobile; drawer tag render on web (and made the AppOverlay test mock faithful to the real {titleContent || title}).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
There was a problem hiding this comment.
Code Review — PR #304
Recommendation: NEEDS WORK (1 Medium finding — no Critical/High)
Summary
Pure display fix that correctly wires NormalizedHabit.tags to both the Today row (3-cap + overflow) and the detail drawer (all tags, wrapping). Production components are fully paired web ↔ mobile. One gap: the web drawer test got a tag test but the mobile drawer test file doesn't exist.
Findings
Medium
[MEDIUM] Mobile habit-detail-drawer has no test file; web drawer tag test has no mirror
- dimension: Parity (#9) + Tests (CLAUDE.md "Every new feature needs tests")
- location:
apps/mobile/__tests__/components/habits/habit-detail-drawer.test.tsx(missing) - issue:
apps/web/__tests__/components/habits/habit-detail-drawer.test.tsxwas updated to add a "renders the habit tags in the detail header" test (lines 202–218). No equivalent mobile file exists — the mobile drawer component has never had a test file and this PR does not create one, despite adding a new rendering path (thetitleBlockcondition change at line 144 +drawerTagRowblock at lines 169–180). - risk: The drawer tag-rendering path on mobile has zero test coverage; a future regression goes undetected.
- fix: Add
apps/mobile/__tests__/components/habits/habit-detail-drawer.test.tsxwith at minimum a test mirroring the web "renders the habit tags in the detail header" case. Use thereact-test-renderer+collectStringspattern from the newhabit-row.test.tsxalready in this PR; mockBottomSheetModalsimilarly to howAppOverlayis mocked on web. - reference: CLAUDE.md rule 9 (Cross-platform parity MANDATORY)
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | PARTIAL — production components fully paired; web drawer test updated, mobile drawer test absent |
| i18n-syncer | IN SYNC — no new keys; tag names are user data; +N is not localizable copy |
| contract-aligner | N/A |
| security-reviewer | N/A |
Validation
| Check | Result |
|---|---|
| Lint | N/A (sandbox blocked; PR states clean) |
| Type check | N/A (sandbox blocked; PR states clean) |
| Tests | N/A (sandbox blocked; PR states web 171 / mobile 54 green) |
| Build (api) | N/A |
What's good
- Root cause: the fix is minimal and correct — no API or type changes needed.
- Clean parity on both production components: logic is identical, only platform primitives differ (StyleSheet+tokens vs Tailwind+CSS vars).
MAX_VISIBLE_TAGS = 3extracted identically on both platforms.- Semantic tokens only (
tokens.fg3/var(--fg-3)) — no hardcoded colors. aria-hiddenon color dots — correct accessibility treatment.AppOverlaymock fidelity fix is a genuine improvement that prevents silent test gaps.react-test-rendererpattern in the new mobile row test is the right call for heavy-RN components.
Recommendation
Add apps/mobile/__tests__/components/habits/habit-detail-drawer.test.tsx to close the test gap, then this is good to merge.
🤖 Generated with Claude Code
There was a problem hiding this comment.
PR Review: fix(habits): render habit tags on the Today row and detail drawer
Verdict: APPROVE - No Critical or High findings.
Summary: Production components are fully paired web and mobile with identical logic. Semantic tokens only (tokens.fg3 / var(--fg-3)), scheme-aware, no hardcoded colors. MAX_VISIBLE_TAGS = 3 extracted identically on both platforms. aria-hidden on color dots is correct. The AppOverlay mock fidelity fix is a genuine improvement. No i18n drift, no contract changes, no security surface touched.
[MEDIUM - non-blocking] Mobile habit-detail-drawer test missing: apps/web/tests/components/habits/habit-detail-drawer.test.tsx was updated with a 'renders the habit tags in the detail header' test, but no equivalent apps/mobile/tests/components/habits/habit-detail-drawer.test.tsx exists. The PR correctly added habit-row.test.tsx on mobile, but the drawer side was missed. Suggested fix: add the mobile drawer test using the react-test-renderer + collectStrings pattern already established in the new mobile habit-row.test.tsx. This can follow in a fast-follow commit.
|
There was a problem hiding this comment.
PR Review: APPROVE (0 Critical, 0 High, 1 Medium)
MEDIUM: Missing mobile habit-detail-drawer test. apps/mobile/tests/components/habits/habit-detail-drawer.test.tsx does not exist. The web side added a test asserting tag names appear in the drawer header; the mobile drawer received the same rendering code but has no mirror test. Risk: a future refactor could silently regress mobile drawer tag rendering while the web test still passes. Fix: add a mobile mirror test following the pattern of habit-row.test.tsx added in this PR (can land as a follow-up before merge).
What is good: correct scoping (NormalizedHabit already carries tags, no API change needed); behavioral parity on both platforms (max 3 chips plus +N overflow on the row, all tags in the drawer header, MAX_VISIBLE_TAGS=3 consistent); token compliance (var(--fg-3)/tokens.fg3 for text, tag.color only on the 6px dot); test coverage on three of four surfaces with overflow cap exercised by a 10-tag fixture; AppOverlay mock fix (titleContent || title) is a genuine faithfulness improvement; no AI-slop.



Problem
A habit's tags were created and stored correctly (verified in prod: the tag existed and was linked), and they show in the edit modal — but they were never rendered on the Today list row or the detail/stats drawer. So a tagged habit looked untagged everywhere except the editor.
Root cause
Pure display gap.
NormalizedHabitalready carriestags, and both the Today row and the detail drawer already receive aNormalizedHabit— the data was there, the components just never drew it. No API or shared-type change needed.Fix (web + mobile parity)
+Noverflow counter. With the 10-tag max, the row height never grows and never wraps into an ugly stack.Tests
+N.AppOverlaymock faithful to the real component ({titleContent || title}instead of droppingtitleContent), which it had been silently ignoring.No i18n keys (tag names are user data). No backend changes.
🤖 Generated with Claude Code