fix(habits): checklist 'Registrar' label + Reiniciar/Limpar icons - #335
Merged
Conversation
Fix J: the all-checklist-complete log confirmation button now reads "Registrar" / "Log" (was "Registrar hábito" / "Log habit"), matching the sibling short log actions in both locales. Fix K: the interactive checklist progress-row "Reiniciar" / "Limpar tudo" text buttons become icon buttons (lucide RotateCcw and X) on both web and mobile. The existing i18n strings move to aria-label / accessibilityLabel so screen-reader text is preserved. 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
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review: PR #335
Scope: PR #335 — fix(habits): checklist 'Registrar' label + Reiniciar/Limpar icons
Recommendation: APPROVE
Summary
Reviewed three logical changes: (1) shortening the checklist-complete confirm label in both i18n locales, (2) converting the interactive-mode reset/clear text buttons to icon buttons on both web and mobile, (3) updating web tests to query by aria-label instead of text content. All five changed files received a verdict. No Critical or High findings survived the adversarial pass.
Findings
Critical
None
High
None
Medium
None
Low / Info
[Info] No mobile mirror test for habit-checklist
- dimension: Parity (#9)
- location:
apps/mobile/__tests__/components/habits/(file absent) - issue:
apps/web/__tests__/components/habits/habit-checklist.test.tsxhas 24 tests; no mobile counterpart exists. The web tests were updated in this PR (getByLabelText instead of getByText), but there is nothing to update on mobile. - risk: Mobile-specific code paths (hitSlop, accessibilityLabel, TouchableOpacity) go untested.
- fix: Create
apps/mobile/__tests__/components/habits/habit-checklist.test.tsxmirroring the web suite, querying byaccessibilityLabelviagetByLabelText, adapted for React Native Testing Library. - reference: CLAUDE.md "Cross-platform parity (MANDATORY)"
- adversarial note: The mobile test file never existed before this PR (pre-existing tech debt, not introduced here). The component .tsx mirror is correctly paired. Downgraded to Info per the signal gate.
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | PAIRED (components); PARTIAL (test — pre-existing gap) |
| i18n-syncer | IN SYNC |
| contract-aligner | N/A (no shared types or endpoints changed) |
| security-reviewer | N/A (no orbit-api code changed) |
Validation
| Check | Result | Details |
|---|---|---|
| Lint | N/A | Permission gate in CI runner; author reports 0 errors |
| Type check | N/A | Permission gate in CI runner; author reports 0 errors |
| Tests | N/A | Permission gate in CI runner; author reports web 1744, mobile 716, shared 1235 |
| Build (api) | N/A | No backend changes |
Deferred
- Dim 5 (No-workaround): N/A — no defensive patches.
- Dim 7 (console.log): N/A — no logging in changed files.
- Dim 11 (Contract drift): N/A — no Zod schema or endpoint changes.
- Dim 12 (Security): N/A — no auth/XSS surface touched.
- Dim 13 (Backend hard rules): N/A — orbit-api not touched.
Backward-compat guard
Only i18n string values changed. No Zod schema fields added, removed, or renamed. Old mobile clients unaffected.
What's good
- Semantic tokens throughout (
var(--primary),var(--primary-pressed),var(--bg-elev),var(--status-bad)) — no raw slate values. transition-[color,background-color,transform]— not the bannedtransition-all.aria-hidden="true"on both icons;aria-label/accessibilityLabelon parent buttons — accessibility preserved.- Hit-target: 36px +
hitSlop={9}= 54px effective on mobile (above 44px minimum). strokeWidth={1.8}matches DESIGN.md default.- Both locales updated atomically; all callsites verified.
resetTextstyle cleanly removed from mobile;clearTextcorrectly retained (still used by theeditablecode path at line 449).- Web tests migrated from
getByTexttogetByLabelText— assert behavior, not implementation.
Recommendation
Approve. Correct, complete, well-executed across both platforms. The missing mobile test file is pre-existing tech debt not introduced by this PR — worth a follow-up issue, not a merge block.
🤖 Generated with Claude Code
|
thomasluizon
added a commit
that referenced
this pull request
Aug 19, 2026
…he five missing surfaces (#36) The canvas is the design of record now, and nothing said so where a worker would read it. /orchestrate reads the ticket, not the project. Twenty one existing tickets gained a comment naming the document that defines their surface. Their bodies were already corrected against D69 on 2026-08-16, so the comment adds the drawing rather than restating the job, and says plainly that the document is the later artefact where the two differ. Five surfaces gained a document in this run and had no ticket at all: #335 the notification bell and its list Orbit Avisos #336 search results, including which matched Orbit Busca #337 the step up code screen Orbit Verificacao #338 the error and static surfaces Orbit Estados #339 offline, and the dropped change Orbit Offline #335 is ordered behind #334, because rewriting the client while the server still writes /streak leaves the two disagreeing. #336 needs no API work at all: the server already returns searchMatches and computeHabitMatchBadges has zero consumers, so the app has been throwing away a better result than it draws. D4, the fourth system round, landed five of nine. The canvas read the whole brief, judged it lacked the headroom to build nine components plus their cards without leaving the system half edited, and wrote nothing that round rather than risk it. Sheet's mount contract, CapacityNotice's body, the Skeleton grid variant, the ListRow read only variant and EventRow are done. DayCell and MonthGrid, OtpInput, Pager and Columns stay on its todo list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Checklist UI tweaks
"Registrar Hábito" → "Registrar"
The all-checklist-complete log confirmation (
habits.checklistCompleteConfirm) read "Registrar Hábito" / "Log habit" — shortened to "Registrar" / "Log" (matching the sibling short log action). i18n value-only change, both locales.Reiniciar / Limpar tudo → icons
On the interactive checklist (logging view), the two text actions become icon buttons matching the existing 36×36 icon-button pattern:
RotateCcwXA11y preserved: the
resetChecklist/clearChecklisti18n strings now live on asaria-label(web) /accessibilityLabel(mobile); icons arearia-hidden. Both platforms.Validation
type-check + lint 0 errors; tests web 1744, mobile 716, shared 1235.
🤖 Generated with Claude Code