feat(web,shared): Wave 3 frontend structure — D36 dedup UI + D37 error page consolidation + D40 complete i18n#110
Merged
Merged
Conversation
…r page consolidation + D40 complete i18n (#110) Wave 3 of the debt roadmap bundles three frontend-structure items that the ROADMAP.md explicitly sequences together: D36 dedupes UI before D40 translates it, and D37 settles error pages before D40 localizes them. D36 — Extract duplicated UI: - HomePage adopts shared RecipeCard from components/recipe-list/ (local fork deleted; currentVersion badge row now renders) - BanDialog component + useBanUser hook extracted to components/admin/ + hooks/; AdminUsersPage + AdminUserDetailPage use shared BanDialog (detail page silent-swallow bug fixed with error-display element) - Section/Field form primitives extracted to components/form/; both RecipeCreatePage and RecipeEditPage import shared (EditSection/EditField deleted, 27 call sites renamed) D37 — Consolidate error pages: - ErrorPage.tsx is canonical (exports NotFoundPage + ServerErrorPage only; base ErrorPage un-exported; ForbiddenPage deleted; SEOHead noIndex added) - NotFoundPage.tsx deleted; router imports from ErrorPage.tsx - ErrorBoundary delegates 404 to NotFoundPage, 5xx to ServerErrorPage; generic fallback uses t() for Go Home/Reload/Oops - ErrorBoundary.test.tsx created (4 tests); ErrorPage.test.tsx updated D40 — Complete i18n: - ~175 new i18n keys added to both en.json and tr.json (BanDialog, compare, verifyEmail, legal, recipe form, 15 admin pages, common vocabulary) - Deterministic bidirectional parity test replaces sampled property test - 4 user-facing pages converted (RecipeCompare, VerifyEmail, Privacy, Terms — legal body kept in English with translated notice) - 2 partial recipe pages fully converted (divergent labels normalized: Dose (g), Extraction Time (s)) - 15 admin pages converted to use t() with useTranslation - 18 tr-locale spot-check test files created - VerifyEmailPage log-message drift fixed per web-page-logging spec Verification: make check + make lint + make fmt + make test all pass (893 web tests + 107 shared tests); all 7 grep gates clean; ROADMAP.md and TECHNICAL_DEBT.md updated (D36, D37, D40 marked resolved).
Contributor
|
Warning Review limit reached
Next review available in: 40 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR consolidates shared frontend UI, rewires error-page handling, and expands i18n coverage across the web app. It also adds tests, locale keys, and Wave 3 documentation updates for the same frontend-structure effort. ChangesWave 3 Frontend Structure
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant AdminUsersPage
participant useBanUser
participant BanDialog
participant adminApi
AdminUsersPage->>useBanUser: openBanDialog(user)
useBanUser->>BanDialog: banDialogUser, reason state
BanDialog->>useBanUser: onConfirm(reason)
useBanUser->>adminApi: banUser(userId, reason)
adminApi-->>useBanUser: success/failure
useBanUser->>AdminUsersPage: onSuccess(userId, isBanned)
useBanUser->>BanDialog: closeDialog / error state
sequenceDiagram
participant Router
participant RootErrorBoundary
participant NotFoundPage
participant ServerErrorPage
Router->>RootErrorBoundary: route error thrown
RootErrorBoundary->>RootErrorBoundary: check error.status
alt status is 404
RootErrorBoundary->>NotFoundPage: render
else status >= 500
RootErrorBoundary->>ServerErrorPage: render
else non-route error
RootErrorBoundary->>RootErrorBoundary: render translated generic fallback
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/web/src/pages/admin/AdminAuditLogPage.tsx (1)
60-65: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEntity-filter
<option>labels remain hardcoded English.Per this cohort's own i18n requirement ("Admin pages use t() for all user-visible strings" / "no user-visible literals remain outside t() calls"),
Users,Recipes,Equipment,Vendors,Taste Notes,Reportsshould be translated too.🌐 Proposed fix
- <option value='user'>Users</option> - <option value='recipe'>Recipes</option> - <option value='equipment'>Equipment</option> - <option value='vendor'>Vendors</option> - <option value='taste_note'>Taste Notes</option> - <option value='report'>Reports</option> + <option value='user'>{t('admin.audit.entityUser')}</option> + <option value='recipe'>{t('admin.audit.entityRecipe')}</option> + <option value='equipment'>{t('admin.audit.entityEquipment')}</option> + <option value='vendor'>{t('admin.audit.entityVendor')}</option> + <option value='taste_note'>{t('admin.audit.entityTasteNote')}</option> + <option value='report'>{t('admin.audit.entityReport')}</option>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/pages/admin/AdminAuditLogPage.tsx` around lines 60 - 65, The entity filter labels in AdminAuditLogPage are still hardcoded English strings; update the select options in AdminAuditLogPage to use t() for each user-visible label so the values for Users, Recipes, Equipment, Vendors, Taste Notes, and Reports are translated consistently with the rest of the admin page.apps/web/src/pages/admin/AdminCoffeeVarietiesPage.tsx (1)
590-592: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPagination "Page X of Y" text left untranslated.
This literal string sits between two now-translated pagination buttons (
common.previous/common.next) but wasn't converted, leaving an English fragment in the middle of otherwise localized UI.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/pages/admin/AdminCoffeeVarietiesPage.tsx` around lines 590 - 592, The pagination label in AdminCoffeeVarietiesPage remains hardcoded as “Page X of Y” while the surrounding controls are translated. Update the span in the pagination footer to use the same i18n approach as the previous/next buttons, replacing the literal text with a localized string that interpolates page values. Refer to the pagination markup near the Page {page} of {Math.ceil(total / 20)} display and keep the wording consistent with the existing translation keys used in this component.apps/web/src/pages/admin/AdminCompatibilityPage.tsx (1)
75-83: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTwo table headers left untranslated.
"Compatible" is localized via
t('admin.compatibility.compatible'), but the adjacent "Brew Method" and "Equipment Type" headers in the same row remain hardcoded English strings.🌐 Proposed fix
<th className='text-left py-2 px-3' style={{ color: 'var(--text-secondary)' }}> - Brew Method + {t('admin.compatibility.brewMethod')} </th> <th className='text-left py-2 px-3' style={{ color: 'var(--text-secondary)' }}> - Equipment Type + {t('admin.compatibility.equipmentType')} </th>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/pages/admin/AdminCompatibilityPage.tsx` around lines 75 - 83, The table header row in AdminCompatibilityPage still hardcodes “Brew Method” and “Equipment Type” instead of using localized strings. Update the header cells in the same table row as t('admin.compatibility.compatible') to use translation keys from the existing i18n setup, matching how Compatible is handled, so all three headers are translated consistently.apps/web/src/pages/TermsPage.tsx (1)
29-73: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSection body text still hardcoded — page is not fully localized.
Only the six section headings (
legal.terms.s1.title…s6.title) were converted tot(). Every paragraph body under those headings (e.g. "By using BrewForm, you agree to these terms...", "You are responsible for your account security...", etc.) remains hardcoded English. This contradicts the PR objective stating the terms page was "fully localized" — Turkish users will see a page with translated headings but English body copy throughout.🌐 Proposed fix (repeat for each section)
<h2 ...>{t('legal.terms.s1.title')}</h2> - <p> - By using BrewForm, you agree to these terms. If you don't agree, please don't use the - service. - </p> + <p>{t('legal.terms.s1.body')}</p>Add corresponding
legal.terms.s1.body…s6.bodykeys toen.json/tr.json.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/pages/TermsPage.tsx` around lines 29 - 73, The terms page is only partially localized because the section body paragraphs in TermsPage still contain hardcoded English text while the headings use t(). Update each section in TermsPage to read the paragraph content from translation keys as well, using matching symbols like legal.terms.s1.body through legal.terms.s6.body, and add those keys to the locale files so both English and Turkish render fully translated content.
🧹 Nitpick comments (9)
openspec/changes/wave-3-frontend-structure/specs/error-pages/spec.md (1)
213-213: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMarkdownlint MD037 flagged on this heading.
The
error.*/notFound.*wildcard patterns in this heading are likely being misparsed as emphasis markers. Wrapping them in backticks (already done partially) or escaping the asterisk should resolve the lint warning.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openspec/changes/wave-3-frontend-structure/specs/error-pages/spec.md` at line 213, The heading text for the error pages requirement is being misparsed by markdownlint MD037 because the wildcard patterns in the title read like emphasis. Update the heading in the spec so the `error.*` and `notFound.*` tokens are treated as code or escaped consistently, keeping the requirement wording intact while avoiding the lint warning.Source: Linters/SAST tools
packages/shared/src/i18n/i18n.test.ts (1)
14-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnreachable
if...throwafterexpect().toBe().
expect(typeof value).toBe('string')already throws on mismatch, so the subsequentif (typeof value !== 'string') throw ...is dead code.♻️ Simplify
it('every locale value is a string', () => { for (const [key, value] of Object.entries(enJson)) { - expect(typeof value).toBe('string'); - if (typeof value !== 'string') throw new Error(`en.json key ${key} is not a string`); + expect(typeof value, `en.json key ${key} is not a string`).toBe('string'); } for (const [key, value] of Object.entries(trJson)) { - expect(typeof value).toBe('string'); - if (typeof value !== 'string') throw new Error(`tr.json key ${key} is not a string`); + expect(typeof value, `tr.json key ${key} is not a string`).toBe('string'); } });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/i18n/i18n.test.ts` around lines 14 - 24, The `every locale value is a string` test in `i18n.test.ts` has redundant dead code because `expect(typeof value).toBe('string')` already fails on non-strings. Remove the following `if (typeof value !== 'string') throw ...` checks from both the `enJson` and `trJson` loops, and keep the test using the `Object.entries` iteration with the existing `expect` assertions only.openspec/changes/wave-3-frontend-structure/specs/i18n/spec.md (1)
56-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocumented key-naming convention doesn't match the delivered keys.
The spec prescribes
recipe.create.section.<name>,recipe.create.field.<name>,compare.<field>, etc., but the actualen.json/tr.jsonadditions userecipe.createPage.heading,recipe.form.basicInfo,recipe.editPage.*,compare.notFound,compare.seoTitle. Worth reconciling the spec text with what was actually shipped so it stays a reliable reference.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openspec/changes/wave-3-frontend-structure/specs/i18n/spec.md` around lines 56 - 66, The i18n key-naming rules in the spec do not match the keys that were actually delivered, so reconcile the documented convention with the shipped translations. Update the spec in i18n/spec.md to reflect the real naming used by the RecipeCreatePage, RecipeEditPage, RecipeComparePage, and related entries (for example the createPage/editPage/form-style keys and compare.notFound/compare.seoTitle), or adjust the delivered keys to match the spec, but make them consistent. Use the existing section that lists D40 key conventions and the referenced page-specific symbols to align the wording.apps/web/src/pages/admin/AdminCoffeeVarietiesPage.tsx (1)
515-525: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCategory badge shows raw enum value, not the translated label.
The category filter/select uses
t('admin.coffeeVarieties.catVariety')etc., but the table row badge renders the rawitem.categoryvalue (variety/processing/market_name), which is inconsistent with the rest of the now-localized page.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/pages/admin/AdminCoffeeVarietiesPage.tsx` around lines 515 - 525, The category badge in AdminCoffeeVarietiesPage is rendering the raw enum value from item.category instead of the localized label used elsewhere on the page. Update the table row badge rendering to map each category value to the corresponding translation key (the same ones used by the category filter/select, such as t('admin.coffeeVarieties.catVariety'), t('admin.coffeeVarieties.catProcessing'), and t('admin.coffeeVarieties.catMarketName')) before displaying it, while keeping CATEGORY_BADGE_COLORS keyed by the underlying category value.apps/web/src/pages/admin/AdminUserEditPage.tsx (1)
244-251: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winReused
auth.resetPassword.newPasswordkey may carry wrong context/tone.This admin-edit-any-user form reuses
auth.resetPassword.newPassword, which is presumably authored for the self-service password-reset flow (e.g. "Your new password"). If the underlying string uses second-person/possessive phrasing, it will read oddly here where an admin is setting another user's password. Consider a dedicatedadmin.users.newPasswordkey instead of sharing the reset-password string.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/pages/admin/AdminUserEditPage.tsx` around lines 244 - 251, The password label in AdminUserEditPage is reusing the self-service reset key, which can give the wrong tone/context for an admin editing another user. Update the label to use a dedicated admin-specific translation key instead of t('auth.resetPassword.newPassword'), and add the matching key under the admin users copy so the AdminUserEditPage form reads naturally in this flow.apps/web/src/pages/admin/AdminUsersPage.test.tsx (1)
16-18: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNo test coverage for the ban confirm/unban flow.
BanDialogis stubbed to() => nullandadminApionly mocksgetUsers/toggleAdmin— there's no test exercisingopenBanDialog→ confirm →adminApi.banUserorunban→adminApi.unbanUser. This is exactly the path with the critical reason-forwarding bug flagged inAdminUsersPage.tsx(Lines 31-41, 319-326); a test assertingbanUseris called with the typed reason would have caught it.Also applies to: 60-65, 77-95
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/pages/admin/AdminUsersPage.test.tsx` around lines 16 - 18, Add coverage for the ban/unban confirmation flow in AdminUsersPage tests instead of stubbing BanDialog to a no-op. Update the AdminUsersPage.test setup to render or simulate the BanDialog path so the openBanDialog → confirm → adminApi.banUser and unban → adminApi.unbanUser interactions are exercised. Use the existing AdminUsersPage, BanDialog, and adminApi mocks to assert the typed reason is forwarded into banUser, and verify the unban path still calls unbanUser correctly.apps/web/src/components/admin/BanDialog.tsx (1)
28-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove debug log out of render body.
log.debugat Line 34 runs on every render, including every keystroke in the reason textarea (sincesetReasonon Line 56 triggers a re-render). This produces log spam disproportionate to its diagnostic value.♻️ Proposed fix: log only on open/close transitions
export function BanDialog({ user, open, onClose, onConfirm, processing }: BanDialogProps) { const { t } = useTranslation(); const [reason, setReason] = useState(''); - if (!open) return null; + useEffect(() => { + if (open) log.debug({ userId: user.id }, 'BanDialog opened'); + }, [open, user.id]); - log.debug({ userId: user.id, open }, 'BanDialog render'); + if (!open) return null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/admin/BanDialog.tsx` around lines 28 - 35, The debug logging in BanDialog is inside the render path and fires on every re-render, including each reason input change. Move the log.debug call out of the component body and into a useEffect tied to the open state in BanDialog so it only logs on open/close transitions, using the existing BanDialogProps, user.id, and open values to keep the same context without render-time spam.apps/web/src/hooks/useBanUser.ts (1)
84-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated error-extraction + untranslated fallback strings.
Both
confirmBanandunbanrepeat(err as { message?: string })?.message || '...', and the fallback text is hardcoded English ('Failed to ban user.'/'Failed to unban user.'), inconsistent with this PR's i18n completion effort — this error is rendered directly inbanErrorat the call sites.♻️ Extract a shared helper and translate fallback text
+function getErrorMessage(err: unknown, fallback: string): string { + return (err as { message?: string })?.message || fallback; +}Then use
getErrorMessage(err, t('admin.users.banFailed'))(viauseTranslation()in the hook) at both call sites.Also applies to: 99-99
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/hooks/useBanUser.ts` at line 84, The error handling in useBanUser duplicates the same message extraction in both confirmBan and unban and still falls back to hardcoded English text. Add a shared helper in useBanUser to derive the message from err, then replace both inline expressions with that helper and pass translated fallback strings from useTranslation() (for example via t('admin.users.banFailed') and the corresponding unban key) so banError receives localized messages consistently.apps/web/src/hooks/useBanUser.test.ts (1)
1-161: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSolid coverage of the hook's own state machine (open/confirm success & failure/unban/clearError/closeDialog). Note: once
confirmBan's signature changes to acceptreasondirectly (see critical comment inuseBanUser.ts), these tests will need updating to callconfirmBan('Spam')instead of relying onsetReason.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/hooks/useBanUser.test.ts` around lines 1 - 161, The `useBanUser` tests still rely on `setReason` before calling `confirmBan`, but `confirmBan` will now take the reason directly. Update the `useBanUser` test cases that cover confirm-ban success, failure, and empty-reason behavior to call `confirmBan('Spam')` or `confirmBan('')` on the hook instance instead of setting state via `setReason`, and keep the assertions around `banDialogUser`, `processing`, `error`, and `onSuccess` aligned with the new `confirmBan` flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/components/form/Field.tsx`:
- Around line 14-24: The Field component’s label is not associated with the
rendered control, so update Field in Field.tsx to use an accessible label/input
pairing. Either wrap the single child inside the <label> in Field, or add an
explicit id/htmlFor contract to FieldProps and pass it through so the label in
Field correctly targets the child control.
In `@apps/web/src/pages/admin/AdminCachePage.tsx`:
- Around line 45-48: The AdminCachePage copy is only partially localized because
the descriptive body text in the cache warning and Deno KV explanation is still
hardcoded English. Update the paragraph content in AdminCachePage to use the
same i18n pattern as the headings/buttons, wiring both remaining body paragraphs
through t(...) (or equivalent translation keys) so all user-facing text on the
page is translated consistently.
- Around line 26-29: The AdminCachePage message styling is still inferring
success/error from the localized message text, so non-English failures can
render with the wrong color. Update the message state and rendering logic in
AdminCachePage to track an explicit status flag for the flush result instead of
checking message contents with string matching. Use the existing flushAll
success/error paths and the message rendering branch together so the color is
driven by the stored status, not by t('admin.cache.flushSuccess') or
t('admin.cache.flushError') text.
In `@apps/web/src/pages/admin/AdminUserDetailPage.tsx`:
- Around line 24-34: Update the useBanUser/BanDialog contract so confirmBan
accepts the ban reason directly instead of relying on internal state that
AdminUserDetailPage does not pass through. Fix the hook in useBanUser.ts by
threading reason into confirmBan and using it when submitting the ban, and keep
AdminUserDetailPage’s onConfirm wiring as-is since it already passes confirmBan
directly. Ensure the hook’s returned API matches the BanDialog expectations,
including any reason-related state or setters that are no longer needed.
In `@apps/web/src/pages/auth/VerifyEmailPage.tsx`:
- Around line 42-47: The error handling in VerifyEmailPage’s verification flow
is using the raw backend message instead of a localized user-facing string.
Update the catch block in VerifyEmailPage so that setErrorMessage always uses
the translated fallback from t('verifyEmail.failed') and does not read
err.message for display, while still keeping the existing log.error call for
debugging context.
In `@apps/web/src/pages/ErrorPage.test.tsx`:
- Around line 69-73: The current ErrorPage test only checks the document title
and does not validate the noIndex behavior controlled by SEOHead. Update the
NotFoundPage test to assert the robots meta tag (or equivalent SEOHead output)
includes noindex when NotFoundPage renders, using the existing
SEOHead/NotFoundPage symbols to locate the relevant setup. Keep the title
assertion only as a secondary check if needed, but make the main assertion
verify the noIndex prop’s effect.
In `@apps/web/src/pages/PrivacyPage.tsx`:
- Around line 32-72: The PrivacyPage is only partially localized because the
section body text is still hardcoded English while the headings already use
translation keys. Update each paragraph in PrivacyPage to use i18n keys
alongside the existing legal.privacy.s1.title through legal.privacy.s6.title
references, keeping the body copy in the locale files instead of inline
literals. Apply this consistently for all six sections so the rendered content
stays fully localized under tr and other locales.
- Line 27: The PrivacyPage date display is using the browser default formatting
instead of the app’s active language. Update PrivacyPage to destructure locale
from useTranslation() and pass that locale into the date formatting in the
legal.lastUpdated paragraph so the displayed date matches the translated page
language. Keep the change scoped to the existing PrivacyPage component and the
current new Date().toLocaleDateString() usage.
In `@openspec/changes/wave-3-frontend-structure/design.md`:
- Line 11: The design document has unlabeled fenced blocks that trigger
markdownlint; update the diagram fences in the markdown content to use an
explicit text fence instead of an untyped fence. Make the same change for both
affected fences in the document so the content stays identical while the docs
check passes.
In `@openspec/changes/wave-3-frontend-structure/proposal.md`:
- Line 11: The opening fenced diagram block in the proposal markdown is untyped
and will fail markdownlint; update the initial fence in the spec document to
include a language tag like text so the block is properly annotated. Locate the
opening fenced section in the proposal content and make the fence consistent
with the rest of the document’s markdown lint requirements.
In
`@openspec/changes/wave-3-frontend-structure/specs/web-shared-components/spec.md`:
- Around line 40-53: The BanDialog and useBanUser contracts are inconsistent:
BanDialog.onConfirm(reason) passes the typed reason, but useBanUser.confirmBan()
currently takes no arguments and relies on separate hook state. Update the spec
so a single flow is used across BanDialog, useBanUser, and AdminUserDetailPage:
either have the dialog set the hook’s reason state before confirmBan() runs, or
change confirmBan to accept the reason and thread it through consistently.
Ensure the chosen API is reflected in the BanDialogProps, useBanUser return
shape, and any consuming page/component requirements.
In `@openspec/changes/wave-3-frontend-structure/tasks.md`:
- Around line 12-17: The verification steps are in the wrong order, with test
running before check; update this task block so each `make check-*` step comes
before the corresponding `make test-*` step. Apply the same ordering
consistently in the later repeated verification steps, keeping the existing step
names like `make check-web`, `make test-web`, `make lint`, and `make test`
aligned in check-then-test sequence.
- Around line 265-273: Fix the malformed translation-key examples in the
AdminUsersPage task description: the instruction text currently contains broken
t(...) samples with missing closing punctuation, which can be misread or copied
incorrectly. Update the references around the AdminUsersPage literals so the
intended keys are written cleanly and consistently, especially the action/error
translation examples, while keeping the guidance aligned with the existing
t('admin.users.*') and t('common.*') symbols.
- Around line 3-7: The checklist text in tasks.md suggests bare folder imports
for shared components, but the repo requires explicit .ts/.tsx specifiers.
Update the HomePage import example to use only the explicit barrel path for
RecipeCard, and similarly remove any bare folder import guidance elsewhere in
the checklist (including the referenced form examples) so the instructions point
implementers to compliant import paths.
---
Outside diff comments:
In `@apps/web/src/pages/admin/AdminAuditLogPage.tsx`:
- Around line 60-65: The entity filter labels in AdminAuditLogPage are still
hardcoded English strings; update the select options in AdminAuditLogPage to use
t() for each user-visible label so the values for Users, Recipes, Equipment,
Vendors, Taste Notes, and Reports are translated consistently with the rest of
the admin page.
In `@apps/web/src/pages/admin/AdminCoffeeVarietiesPage.tsx`:
- Around line 590-592: The pagination label in AdminCoffeeVarietiesPage remains
hardcoded as “Page X of Y” while the surrounding controls are translated. Update
the span in the pagination footer to use the same i18n approach as the
previous/next buttons, replacing the literal text with a localized string that
interpolates page values. Refer to the pagination markup near the Page {page} of
{Math.ceil(total / 20)} display and keep the wording consistent with the
existing translation keys used in this component.
In `@apps/web/src/pages/admin/AdminCompatibilityPage.tsx`:
- Around line 75-83: The table header row in AdminCompatibilityPage still
hardcodes “Brew Method” and “Equipment Type” instead of using localized strings.
Update the header cells in the same table row as
t('admin.compatibility.compatible') to use translation keys from the existing
i18n setup, matching how Compatible is handled, so all three headers are
translated consistently.
In `@apps/web/src/pages/TermsPage.tsx`:
- Around line 29-73: The terms page is only partially localized because the
section body paragraphs in TermsPage still contain hardcoded English text while
the headings use t(). Update each section in TermsPage to read the paragraph
content from translation keys as well, using matching symbols like
legal.terms.s1.body through legal.terms.s6.body, and add those keys to the
locale files so both English and Turkish render fully translated content.
---
Nitpick comments:
In `@apps/web/src/components/admin/BanDialog.tsx`:
- Around line 28-35: The debug logging in BanDialog is inside the render path
and fires on every re-render, including each reason input change. Move the
log.debug call out of the component body and into a useEffect tied to the open
state in BanDialog so it only logs on open/close transitions, using the existing
BanDialogProps, user.id, and open values to keep the same context without
render-time spam.
In `@apps/web/src/hooks/useBanUser.test.ts`:
- Around line 1-161: The `useBanUser` tests still rely on `setReason` before
calling `confirmBan`, but `confirmBan` will now take the reason directly. Update
the `useBanUser` test cases that cover confirm-ban success, failure, and
empty-reason behavior to call `confirmBan('Spam')` or `confirmBan('')` on the
hook instance instead of setting state via `setReason`, and keep the assertions
around `banDialogUser`, `processing`, `error`, and `onSuccess` aligned with the
new `confirmBan` flow.
In `@apps/web/src/hooks/useBanUser.ts`:
- Line 84: The error handling in useBanUser duplicates the same message
extraction in both confirmBan and unban and still falls back to hardcoded
English text. Add a shared helper in useBanUser to derive the message from err,
then replace both inline expressions with that helper and pass translated
fallback strings from useTranslation() (for example via
t('admin.users.banFailed') and the corresponding unban key) so banError receives
localized messages consistently.
In `@apps/web/src/pages/admin/AdminCoffeeVarietiesPage.tsx`:
- Around line 515-525: The category badge in AdminCoffeeVarietiesPage is
rendering the raw enum value from item.category instead of the localized label
used elsewhere on the page. Update the table row badge rendering to map each
category value to the corresponding translation key (the same ones used by the
category filter/select, such as t('admin.coffeeVarieties.catVariety'),
t('admin.coffeeVarieties.catProcessing'), and
t('admin.coffeeVarieties.catMarketName')) before displaying it, while keeping
CATEGORY_BADGE_COLORS keyed by the underlying category value.
In `@apps/web/src/pages/admin/AdminUserEditPage.tsx`:
- Around line 244-251: The password label in AdminUserEditPage is reusing the
self-service reset key, which can give the wrong tone/context for an admin
editing another user. Update the label to use a dedicated admin-specific
translation key instead of t('auth.resetPassword.newPassword'), and add the
matching key under the admin users copy so the AdminUserEditPage form reads
naturally in this flow.
In `@apps/web/src/pages/admin/AdminUsersPage.test.tsx`:
- Around line 16-18: Add coverage for the ban/unban confirmation flow in
AdminUsersPage tests instead of stubbing BanDialog to a no-op. Update the
AdminUsersPage.test setup to render or simulate the BanDialog path so the
openBanDialog → confirm → adminApi.banUser and unban → adminApi.unbanUser
interactions are exercised. Use the existing AdminUsersPage, BanDialog, and
adminApi mocks to assert the typed reason is forwarded into banUser, and verify
the unban path still calls unbanUser correctly.
In `@openspec/changes/wave-3-frontend-structure/specs/error-pages/spec.md`:
- Line 213: The heading text for the error pages requirement is being misparsed
by markdownlint MD037 because the wildcard patterns in the title read like
emphasis. Update the heading in the spec so the `error.*` and `notFound.*`
tokens are treated as code or escaped consistently, keeping the requirement
wording intact while avoiding the lint warning.
In `@openspec/changes/wave-3-frontend-structure/specs/i18n/spec.md`:
- Around line 56-66: The i18n key-naming rules in the spec do not match the keys
that were actually delivered, so reconcile the documented convention with the
shipped translations. Update the spec in i18n/spec.md to reflect the real naming
used by the RecipeCreatePage, RecipeEditPage, RecipeComparePage, and related
entries (for example the createPage/editPage/form-style keys and
compare.notFound/compare.seoTitle), or adjust the delivered keys to match the
spec, but make them consistent. Use the existing section that lists D40 key
conventions and the referenced page-specific symbols to align the wording.
In `@packages/shared/src/i18n/i18n.test.ts`:
- Around line 14-24: The `every locale value is a string` test in `i18n.test.ts`
has redundant dead code because `expect(typeof value).toBe('string')` already
fails on non-strings. Remove the following `if (typeof value !== 'string') throw
...` checks from both the `enJson` and `trJson` loops, and keep the test using
the `Object.entries` iteration with the existing `expect` assertions only.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 21dc16fb-5fd8-47ad-94c0-cadd6850da1d
📒 Files selected for processing (67)
apps/web/src/components/ErrorBoundary.test.tsxapps/web/src/components/ErrorBoundary.tsxapps/web/src/components/admin/BanDialog.test.tsxapps/web/src/components/admin/BanDialog.tsxapps/web/src/components/form/Field.tsxapps/web/src/components/form/Section.tsxapps/web/src/components/form/index.tsapps/web/src/hooks/useBanUser.test.tsapps/web/src/hooks/useBanUser.tsapps/web/src/pages/ErrorPage.test.tsxapps/web/src/pages/ErrorPage.tsxapps/web/src/pages/HomePage.tsxapps/web/src/pages/NotFoundPage.tsxapps/web/src/pages/PrivacyPage.test.tsxapps/web/src/pages/PrivacyPage.tsxapps/web/src/pages/TermsPage.test.tsxapps/web/src/pages/TermsPage.tsxapps/web/src/pages/admin/AdminAuditLogPage.test.tsxapps/web/src/pages/admin/AdminAuditLogPage.tsxapps/web/src/pages/admin/AdminBadgesPage.test.tsxapps/web/src/pages/admin/AdminBadgesPage.tsxapps/web/src/pages/admin/AdminCachePage.test.tsxapps/web/src/pages/admin/AdminCachePage.tsxapps/web/src/pages/admin/AdminCoffeeVarietiesPage.test.tsxapps/web/src/pages/admin/AdminCoffeeVarietiesPage.tsxapps/web/src/pages/admin/AdminCompatibilityPage.test.tsxapps/web/src/pages/admin/AdminCompatibilityPage.tsxapps/web/src/pages/admin/AdminDashboard.test.tsxapps/web/src/pages/admin/AdminDashboard.tsxapps/web/src/pages/admin/AdminEquipmentPage.test.tsxapps/web/src/pages/admin/AdminEquipmentPage.tsxapps/web/src/pages/admin/AdminLayout.test.tsxapps/web/src/pages/admin/AdminLayout.tsxapps/web/src/pages/admin/AdminRecipesPage.test.tsxapps/web/src/pages/admin/AdminRecipesPage.tsxapps/web/src/pages/admin/AdminTasteNotesPage.test.tsxapps/web/src/pages/admin/AdminTasteNotesPage.tsxapps/web/src/pages/admin/AdminUserCreatePage.test.tsxapps/web/src/pages/admin/AdminUserCreatePage.tsxapps/web/src/pages/admin/AdminUserDetailPage.test.tsxapps/web/src/pages/admin/AdminUserDetailPage.tsxapps/web/src/pages/admin/AdminUserEditPage.test.tsxapps/web/src/pages/admin/AdminUserEditPage.tsxapps/web/src/pages/admin/AdminUsersPage.test.tsxapps/web/src/pages/admin/AdminUsersPage.tsxapps/web/src/pages/admin/AdminVendorsPage.test.tsxapps/web/src/pages/admin/AdminVendorsPage.tsxapps/web/src/pages/auth/VerifyEmailPage.test.tsxapps/web/src/pages/auth/VerifyEmailPage.tsxapps/web/src/pages/recipes/RecipeComparePage.test.tsxapps/web/src/pages/recipes/RecipeComparePage.tsxapps/web/src/pages/recipes/RecipeCreatePage.tsxapps/web/src/pages/recipes/RecipeEditPage.test.tsxapps/web/src/pages/recipes/RecipeEditPage.tsxapps/web/src/router.tsxopenspec/changes/wave-3-frontend-structure/design.mdopenspec/changes/wave-3-frontend-structure/proposal.mdopenspec/changes/wave-3-frontend-structure/specs/error-pages/spec.mdopenspec/changes/wave-3-frontend-structure/specs/i18n/spec.mdopenspec/changes/wave-3-frontend-structure/specs/recipe-list/spec.mdopenspec/changes/wave-3-frontend-structure/specs/web-shared-components/spec.mdopenspec/changes/wave-3-frontend-structure/tasks.mdpackages/shared/src/i18n/en.jsonpackages/shared/src/i18n/i18n.test.tspackages/shared/src/i18n/tr.jsonplans/ROADMAP.mdplans/TECHNICAL_DEBT.md
💤 Files with no reviewable changes (1)
- apps/web/src/pages/NotFoundPage.tsx
Comment on lines
+24
to
+34
| const { | ||
| banDialogUser, | ||
| processing: banProcessing, | ||
| error: banError, | ||
| openBanDialog, | ||
| confirmBan, | ||
| unban, | ||
| closeDialog, | ||
| } = useBanUser((_userId, isBanned) => { | ||
| setUser((prev) => prev ? { ...prev, isBanned } : prev); | ||
| }); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Downstream impact of the useBanUser/BanDialog contract mismatch.
confirmBan is passed directly as onConfirm={confirmBan} (Line 310) without ever calling the hook's reason/setReason (not even destructured here). Root cause and proposed fix are in useBanUser.ts; once confirmBan is updated to accept reason as a parameter, this wiring will work correctly as-is (no change needed here beyond the hook's own fix).
Also applies to: 305-313
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/pages/admin/AdminUserDetailPage.tsx` around lines 24 - 34,
Update the useBanUser/BanDialog contract so confirmBan accepts the ban reason
directly instead of relying on internal state that AdminUserDetailPage does not
pass through. Fix the hook in useBanUser.ts by threading reason into confirmBan
and using it when submitting the ban, and keep AdminUserDetailPage’s onConfirm
wiring as-is since it already passes confirmBan directly. Ensure the hook’s
returned API matches the BanDialog expectations, including any reason-related
state or setters that are no longer needed.
| them into one change keeps the sequencing coherent, lets the i18n work (D40) land its key sets | ||
| correctly the first time, and produces a single spec archive that documents the frontend structural | ||
| shift as one unit. | ||
|
|
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Tag the opening diagram fence.
This untyped fenced block will trip markdownlint. Add a language tag such as text to keep the spec doc lint-clean.
Suggested fix
-```
+```text📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openspec/changes/wave-3-frontend-structure/proposal.md` at line 11, The
opening fenced diagram block in the proposal markdown is untyped and will fail
markdownlint; update the initial fence in the spec document to include a
language tag like text so the block is properly annotated. Locate the opening
fenced section in the proposal content and make the fence consistent with the
rest of the document’s markdown lint requirements.
- Field.tsx: wrap children in <label> for accessible label/input pairing
- AdminCachePage.tsx: i18n body text + status flag for message styling
- useBanUser.ts: confirmBan accepts reason directly; shared getErrorMessage helper
- BanDialog.tsx: move log.debug from render path into useEffect on open
- useBanUser.test.ts: align tests with new confirmBan(reason) API
- AdminUsersPage.test.tsx: real BanDialog coverage for ban/unban flow
- AdminUserDetailPage.tsx: compatible with confirmBan(reason) (noop)
- VerifyEmailPage.tsx: always use t('verifyEmail.failed'), not err.message
- PrivacyPage/TermsPage: i18n body text + locale-aware date formatting
- AdminAuditLogPage: entity filter labels via t()
- AdminCoffeeVarietiesPage: pagination via common.pagination key; translated
category badge
- AdminCompatibilityPage: table headers via t()
- AdminUserEditPage: admin-specific key for password label
- ErrorPage.test.tsx: verify robots noindex meta tag
- i18n.test.ts: remove redundant dead-code throw statements
- en.json/tr.json: add 30+ new translation keys
- openspec docs: untyped fences, task ordering, malformed t(), bare imports,
spec consistency for confirmBan API and i18n key naming
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/hooks/useBanUser.ts (1)
78-92: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
unbanlacks an in-flight guard. The unban buttons inapps/web/src/pages/admin/AdminUsersPage.tsxandapps/web/src/pages/admin/AdminUserDetailPage.tsxcallunban()directly and never disable onprocessing, so a fast double-click can send duplicateunbanUserrequests. Wire the unban path toprocessingor add a local guard before the request.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/hooks/useBanUser.ts` around lines 78 - 92, The unban path in useBanUser lacks an in-flight guard, so repeated clicks can trigger duplicate adminApi.unbanUser requests. Add protection in the unban callback itself by checking processing or a local pending flag before starting the request, and make sure the AdminUsersPage/AdminUserDetailPage unban actions use that guarded unban flow so a second click is ignored while the first request is running.
🧹 Nitpick comments (1)
apps/web/src/hooks/useBanUser.ts (1)
12-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRepeated inline user-shape type.
The
{ id: string; username: string; displayName: string | null }shape is duplicated acrossbanDialogUser,openBanDialog's parameter, and the internal state declaration. Extracting aBanDialogUsertype would reduce duplication.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/hooks/useBanUser.ts` around lines 12 - 19, The user object shape is duplicated in useBanUser across banDialogUser, openBanDialog, and the related state, so extract it into a shared BanDialogUser type. Update the useBanUser hook’s type declarations to reference that alias everywhere the same { id, username, displayName } shape is used, keeping the function signature and state consistent without repeating the inline object type.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/components/form/Field.tsx`:
- Around line 14-24: The Field component is applying typography classes on the
wrapping label, which causes nested form controls to inherit `text-sm
font-medium` under Tailwind v4 Preflight. Update Field so the typography classes
are scoped to the inner label text span (or explicitly reset them on the child
control), and keep the wrapper label only for layout/color; use the Field and
FieldProps symbols to locate the JSX in the Field component.
In `@apps/web/src/hooks/useBanUser.ts`:
- Around line 22-24: getErrorMessage in useBanUser is leaking raw backend
err.message and using hardcoded English fallbacks, so update the hook to stop
returning server text entirely and instead return a stable error identifier or
non-null fallback that can be translated by the caller. Adjust the ban/unban
handling in useBanUser so the consuming page translates the result with t(),
matching the localized pattern already used for admin.users.removeAdminError and
the VerifyEmailPage fix. Keep the hook focused on mapping errors, and move any
user-facing message selection out of getErrorMessage.
---
Outside diff comments:
In `@apps/web/src/hooks/useBanUser.ts`:
- Around line 78-92: The unban path in useBanUser lacks an in-flight guard, so
repeated clicks can trigger duplicate adminApi.unbanUser requests. Add
protection in the unban callback itself by checking processing or a local
pending flag before starting the request, and make sure the
AdminUsersPage/AdminUserDetailPage unban actions use that guarded unban flow so
a second click is ignored while the first request is running.
---
Nitpick comments:
In `@apps/web/src/hooks/useBanUser.ts`:
- Around line 12-19: The user object shape is duplicated in useBanUser across
banDialogUser, openBanDialog, and the related state, so extract it into a shared
BanDialogUser type. Update the useBanUser hook’s type declarations to reference
that alias everywhere the same { id, username, displayName } shape is used,
keeping the function signature and state consistent without repeating the inline
object type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2c3521e6-cc3e-4608-9be1-75637a4c6dde
📒 Files selected for processing (22)
apps/web/src/components/admin/BanDialog.tsxapps/web/src/components/form/Field.tsxapps/web/src/hooks/useBanUser.test.tsapps/web/src/hooks/useBanUser.tsapps/web/src/pages/ErrorPage.test.tsxapps/web/src/pages/PrivacyPage.tsxapps/web/src/pages/TermsPage.tsxapps/web/src/pages/admin/AdminAuditLogPage.tsxapps/web/src/pages/admin/AdminCachePage.tsxapps/web/src/pages/admin/AdminCoffeeVarietiesPage.tsxapps/web/src/pages/admin/AdminCompatibilityPage.tsxapps/web/src/pages/admin/AdminUserEditPage.tsxapps/web/src/pages/admin/AdminUsersPage.test.tsxapps/web/src/pages/auth/VerifyEmailPage.tsxopenspec/changes/wave-3-frontend-structure/design.mdopenspec/changes/wave-3-frontend-structure/specs/error-pages/spec.mdopenspec/changes/wave-3-frontend-structure/specs/i18n/spec.mdopenspec/changes/wave-3-frontend-structure/specs/web-shared-components/spec.mdopenspec/changes/wave-3-frontend-structure/tasks.mdpackages/shared/src/i18n/en.jsonpackages/shared/src/i18n/i18n.test.tspackages/shared/src/i18n/tr.json
✅ Files skipped from review due to trivial changes (5)
- apps/web/src/pages/admin/AdminAuditLogPage.tsx
- openspec/changes/wave-3-frontend-structure/tasks.md
- apps/web/src/pages/admin/AdminUserEditPage.tsx
- openspec/changes/wave-3-frontend-structure/design.md
- packages/shared/src/i18n/tr.json
🚧 Files skipped from review as they are similar to previous changes (12)
- apps/web/src/components/admin/BanDialog.tsx
- packages/shared/src/i18n/i18n.test.ts
- apps/web/src/pages/admin/AdminCompatibilityPage.tsx
- apps/web/src/pages/admin/AdminCachePage.tsx
- packages/shared/src/i18n/en.json
- apps/web/src/pages/TermsPage.tsx
- apps/web/src/pages/PrivacyPage.tsx
- apps/web/src/pages/ErrorPage.test.tsx
- openspec/changes/wave-3-frontend-structure/specs/web-shared-components/spec.md
- apps/web/src/pages/admin/AdminCoffeeVarietiesPage.tsx
- openspec/changes/wave-3-frontend-structure/specs/error-pages/spec.md
- openspec/changes/wave-3-frontend-structure/specs/i18n/spec.md
…oping, BanUser audit, i18n keys
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.
Wave 3 — Frontend Structure (D36 + D37 + D40)
Wave 3 of the debt roadmap bundles three frontend-structure items that the
ROADMAP.mdexplicitly sequences together because the order matters: D36 dedupes UI before D40 translates it (so each string is touched once), and D37 settles which error pages exist before D40 localizes them.D36 — Extract duplicated UI
currentVersionbadge row)RecipeCardfromcomponents/recipe-list/BanDialog+useBanUserextracted tocomponents/admin/+hooks/; errors surfaced on both call sitesEditSection/EditField)components/form/; both pages import shared (27 call sites renamed)New files:
BanDialog.tsx,useBanUser.ts,Section.tsx,Field.tsx,form/index.ts,BanDialog.test.tsx(7 tests),useBanUser.test.ts(9 tests)D37 — Consolidate error pages
ErrorPage.tsxis the canonical module (exportsNotFoundPage+ServerErrorPageonly; baseErrorPageun-exported;ForbiddenPagedeleted;<SEOHead noIndex>added to all variants)NotFoundPage.tsxdeleted; router imports fromErrorPage.tsxErrorBoundarydelegates 404 →<NotFoundPage />, 5xx →<ServerErrorPage />; generic fallback usest()for Go Home/Reload/OopsErrorBoundary.test.tsxcreated (4 tests);ErrorPage.test.tsxupdatedD40 — Complete i18n
en.jsonandtr.json(BanDialog, compare, verifyEmail, legal, recipe form, 15 admin pages, common vocabulary)RecipeComparePage,VerifyEmailPage,PrivacyPage,TermsPage— legal body kept in English with translated notice per design Decision 7)Dose (g),Extraction Time (s)— both pages use the samerecipe.form.dosekey)t()withuseTranslation; existingadmin.*nav keys wired intoAdminLayout;common.*vocabulary reused (noadmin.common.*duplication)VerifyEmailPagelog-message drift fixed ('token verification failed'→'verifyEmail failed'perweb-page-loggingspec)PrivacyPage+TermsPageadd mount/unmount logs (pre-existing gap)Verification
make check(type-check all workspaces)make lintmake fmtmake test(all tests via Docker).tsxfile lacksuseTranslationNotFoundPage.tsximportsRecipeCardin pagesEditSection/EditFieldForbiddenPageErrorPagenot exportednotFound.*keysFiles changed (67)
NotFoundPage.tsx)Design decisions
See
openspec/changes/wave-3-frontend-structure/design.mdfor the 11 design decisions, including:text-primary(composed version)ForbiddenPage(no clean trigger without changingRequireAuthUX)common.*vocabulary (noadmin.common.*duplication)catchblocks (out of scope for i18n)OpenSpec change
openspec/changes/wave-3-frontend-structure/— 65/65 tasks complete, 4 specs (web-shared-components, error-pages, i18n, recipe-list).Summary by CodeRabbit