Skip to content

feat(carrier): brand + tokenize the dogfood web login gate - #1914

Merged
ctwoodwa merged 2 commits into
mainfrom
fed-login-brand-delight
Jul 7, 2026
Merged

feat(carrier): brand + tokenize the dogfood web login gate#1914
ctwoodwa merged 2 commits into
mainfrom
fed-login-brand-delight

Conversation

@ctwoodwa

@ctwoodwa ctwoodwa commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

The dogfood web-client login gate (apps/carrier/src/webclient/WebSessionGate.tsx) was
unbranded, un-tokened, and theme-locked — the CIC dogfood report flagged "login is dark with
no way to change it." Reworked per the Harborline Design Language product register
(_shared/design/harborline-design-language-2026-07-03.md):

  • Brand presence — new HarborlineMark component (apps/carrier/src/components/HarborlineMark.tsx)
    inlines the canonical waterline+beacon mark as SVG (no network fetch; source =
    harborline-www/public/favicon.svg, #06489c cobalt / #e97c48 beacon-amber). Sits above the
    "Harborline" wordmark + heading on both the login form and the session-checking splash — the one
    surface the doctrine names as a sanctioned quiet brand moment (§0/§6).
  • Tokenized — every raw neutral-* class replaced with the ui-react token vocabulary
    (bg-background/bg-card/text-foreground/text-muted-foreground/border-border, paired
    bg+fg per the Window.tsx theme-compliance lesson).
  • One vocabulary — adopted @shipyard/ui-react Button/Input/Alert in place of the
    hand-rolled controls; no packages/ui-react changes (no dist rebuild in this lane).
  • Theme toggle — mounted the existing ThemeToggle (wired to useCarrierTheme) top-right on
    both the login form and the session-checking splash — fixes the reported dark-with-no-toggle gap.
  • i18n — every string moved to a new top-level login.* block, added to en-US.ts (source) and
    all 19 other locale catalogs. Per this file's own established convention (every non-en locale here
    is currently an English-fallback scaffold, completenessFloor: 0, pending a translator pass) the
    new block ships as the same English-fallback scaffold — consistent with every other block in these
    files, not a new pattern. Glossary query (i18n:glossary:query) confirmed no existing terms to
    reuse. localeInit.drift.test.ts (key-parity across all 20 catalogs) passes.
  • Rationed delight — mark+form rise/fade as ONE group (~200ms, custom-cubic ease-out-quart,
    .web-session-enter in index.css), splash mark gets a gentle animate-pulse, button
    active:translate-y-px. All gated by Tailwind's motion-reduce: variant (built-in
    prefers-reduced-motion support) — no JS-side motion gating needed for these.
  • Accessibility — labels/for preserved, Alert carries aria-live="assertive", 44px touch
    targets (Input/Button size="lg" = 44/48px), focus-visible rings via the token components.

Verified live (Playwright, browser sandbox on)

Ran apps/carrier in --mode webclient dev; screenshotted the real gate:

  • Light / desktop (1280×800) — login form, mark + wordmark + heading, tokenized inputs, primary
    button. Clean.
  • Dark / desktop (1280×800) — theme toggle switches instantly; mark holds its own brand colors on
    the dark card surface; all text readable.
  • Dark / mobile (390×844) — card reflows correctly, no overflow, touch targets comfortable.
  • Light / mobile (390×844) — same, light theme.
  • Also exercised: the session-checking splash (mark + pulsing + "Checking your session…"), and the
    error path (wrong credentials → tokenized Alert with "Sign-in failed. Check your username and
    password.").

Gates (foreground)

  • npx tsc -b --noEmit (apps/carrier): 0 errors touching any changed file (a handful of
    pre-existing errors from unbuilt sibling packages — @shipyard/contracts/rule-engine/
    carrier-sdk/hull — cleared once those were built locally; matches the documented
    worktree-dotnet-build-geometry gap, not a regression).
  • npx eslint on all touched files: clean, 0 warnings.
  • npx vitest run (full apps/carrier suite): 1374 passed, 1 unrelated failure — a
    darwin-only real-TTS-invocation test (hull-invoker.test.ts) that timed out; pre-existing,
    environment-dependent, untouched by this PR.
  • WebSessionGate.test.tsx + no-node-builtins-bundle.test.ts (the webclient bundle guard): all
    green (guard test no-ops without a built dist/, as documented — CI's
    check-carrier-renderer-node-free.mjs is the unconditional enforcer).
  • src/i18n/ suite (locale drift/parity across all 20 catalogs): green.

Followups (noted, not blocking)

  • The §2.3 product-interactive-blue convergence (retiring #0f62fe for a brand-cobalt tint) is an
    open CIC item — this PR keeps the existing --color-primary for the submit button, unchanged.
  • No native-translator pass yet for the new login.* strings — same as every other block in these
    scaffold locale files; tracked by the existing i18n rollout workstream.

@standing-pattern: none claimed (net-new UI surface, not a rebind-cohort pair)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added a branded sign-in experience with a new Harborline mark and updated login screen styling.
    • Expanded sign-in copy support across multiple languages, including headings, labels, button states, and error messages.
    • Added a subtle entrance animation for the login form.
  • Bug Fixes

    • Improved sign-in error messaging so network issues and invalid credentials show clearer feedback.
    • Updated the session-checking view to use localized text and branded visuals.

The webclient login threshold (WebSessionGate.tsx) was unbranded, un-tokened, and
theme-locked (no way to switch out of dark mode pre-login per the CIC dogfood
report). Rework it to the Harborline design language's product register:

- Inline the Harborline waterline+beacon mark (no network fetch) above the
  wordmark and a rise/fade entrance, the one sanctioned quiet brand moment for
  this surface.
- Replace every raw neutral-* class with the ui-react token vocabulary
  (bg-background/bg-card/text-foreground/border-border) and adopt the ui-react
  Button/Input/Alert components in place of hand-rolled controls.
- Mount the existing ThemeToggle top-right on both the login form and the
  session-checking splash, wired to useCarrierTheme.
- Move every user-facing string to a new top-level login.* i18n block across
  all 20 locale catalogs (English-fallback scaffold, matching this file's
  existing convention for untranslated locales).
- Rationed, reduced-motion-safe entrance/focus/press motion per the design
  language's delight policy; 44px touch targets; aria-live error alert.

Refs: harborline-design-language-2026-07-03.md
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@ctwoodwa, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 42 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ce6acb5a-baef-4b62-b87d-ec2cffe78e85

📥 Commits

Reviewing files that changed from the base of the PR and between a4fdbc9 and f98cfbc.

📒 Files selected for processing (26)
  • apps/carrier/src/components/HarborlineMark.tsx
  • apps/carrier/src/components/ThemeToggle.tsx
  • apps/carrier/src/i18n/locales/ar-AE.ts
  • apps/carrier/src/i18n/locales/ar.ts
  • apps/carrier/src/i18n/locales/bn.ts
  • apps/carrier/src/i18n/locales/de-DE.ts
  • apps/carrier/src/i18n/locales/en-GB.ts
  • apps/carrier/src/i18n/locales/en-US.ts
  • apps/carrier/src/i18n/locales/es-419.ts
  • apps/carrier/src/i18n/locales/es-ES.ts
  • apps/carrier/src/i18n/locales/fr-CA.ts
  • apps/carrier/src/i18n/locales/fr-FR.ts
  • apps/carrier/src/i18n/locales/hi-IN.ts
  • apps/carrier/src/i18n/locales/it-IT.ts
  • apps/carrier/src/i18n/locales/ja-JP.ts
  • apps/carrier/src/i18n/locales/nl-NL.ts
  • apps/carrier/src/i18n/locales/pt-BR.ts
  • apps/carrier/src/i18n/locales/pt-PT.ts
  • apps/carrier/src/i18n/locales/ru-RU.ts
  • apps/carrier/src/i18n/locales/ur-PK.ts
  • apps/carrier/src/i18n/locales/ur.ts
  • apps/carrier/src/i18n/locales/zh-CN.ts
  • apps/carrier/src/i18n/locales/zh-TW.ts
  • apps/carrier/src/index.css
  • apps/carrier/src/webclient/WebSessionGate.tsx
  • apps/carrier/src/webclient/webSessionClient.ts
📝 Walkthrough

Walkthrough

Adds a HarborlineMark SVG brand component, a web-session-rise CSS animation, and a new login translation namespace across all carrier locale files. WebSessionGate is reworked to use a shared shell, tokenized styles, the new mark, an Alert component, and localized copy/error messages.

Changes

Web login gate rework

Layer / File(s) Summary
HarborlineMark brand component
apps/carrier/src/components/HarborlineMark.tsx
New exported HarborlineMark component renders a fixed inline SVG brand badge with configurable size and className.
Login enter animation styles
apps/carrier/src/index.css
Adds web-session-rise keyframes and a .web-session-enter class for the login form's entrance animation.
WebSessionGate shell, login form, and checking screen
apps/carrier/src/webclient/WebSessionGate.tsx
Adds shared WebSessionShell wrapper, switches login UI to i18n strings, maps errors to translated network/invalid messages via Alert, applies tokenized styling and the enter animation, and reworks WebSessionChecking to show a pulsing HarborlineMark with localized status text.
Login translation keys across locale files
apps/carrier/src/i18n/locales/*.ts
Adds a login object (heading, subtitle, username/password labels, submit/submitting, checkingSession, errorNetwork, errorInvalid) to all 22 locale bundles.

Estimated code review effort: 2 (Simple) | ~15 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant WebLogin
  participant AuthAPI
  participant Translation

  User->>WebLogin: submit credentials
  WebLogin->>AuthAPI: POST login request
  AuthAPI-->>WebLogin: WebLoginError with status
  WebLogin->>Translation: t('login.errorNetwork' or 'login.errorInvalid')
  Translation-->>WebLogin: localized message
  WebLogin-->>User: render Alert with message
Loading

Related Issues: Not provided.

Related PRs: Not provided.

Suggested labels: i18n, ui, web-client

Suggested reviewers: Not provided.

praise: nice touch adding the web-session-enter animation and HarborlineMark pulse to give the login/checking states a cohesive branded feel.

nit: with 22 near-identical locale diffs, a quick automated key-parity check (e.g., diff against en-US keys) would help confirm all locales define the same login.* keys before merge.

question: for non-English locales beyond en-US, are the added strings placeholder English scaffolding intentionally, pending translation, or should they be flagged for translators now?

        🐦 chirps in Harborline blue,
        a brand new mark takes flight,
        login screens rise and fade in view,
        eighteen tongues learn to say "sign in" tonight.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is detailed, but it omits required template sections like ICM Stage, Affected Packages, and the checklist. Add the missing template sections: ICM Stage, Affected Packages, and the checklist items, even if some are marked not applicable.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely reflects the main change: branding and tokenizing the carrier web login gate.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fed-login-brand-delight

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (7)
apps/carrier/src/i18n/locales/nl-NL.ts (1)

2121-2135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

nit: missing per-key TODO markers on the new login.* scaffold.

Same pattern gap as the other locale files in this PR — login.* lacks the // TODO: nl translation markers used elsewhere.

🤖 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/carrier/src/i18n/locales/nl-NL.ts` around lines 2121 - 2135, Add the
missing per-key scaffold markers for the new login entries in the nl-NL locale
object. Update the `login` section so each key under `login` includes the same
`// TODO: nl translation` convention used elsewhere in the locale files, keeping
the existing English fallback values intact.
apps/carrier/src/i18n/locales/it-IT.ts (1)

2121-2135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

nit: missing per-key TODO markers on the new login.* scaffold.

Same as the sibling locale bundles — login.* keys skip the per-key // TODO: it translation marker that every other fallback string in this file carries.

🤖 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/carrier/src/i18n/locales/it-IT.ts` around lines 2121 - 2135, The new
login scaffold in the locale bundle is missing the per-key TODO markers used
elsewhere for fallback strings. Update the login section in the locale object so
each `login.*` entry carries the same `// TODO: it translation` marker
convention as other untranslated keys, keeping the existing `login` grouping and
symbol names intact.
apps/carrier/src/i18n/locales/ja-JP.ts (1)

2121-2135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

nit: missing per-key TODO markers on the new login.* scaffold.

Same pattern gap as the other locale files — consider adding // TODO: ja translation per key to stay consistent with the file's convention.

🤖 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/carrier/src/i18n/locales/ja-JP.ts` around lines 2121 - 2135, The new
login scaffold in the locale object is missing the per-key translation TODO
markers used elsewhere in this file. Update the `login` entries in `ja-JP.ts` so
each key under `login` has the consistent `// TODO: ja translation` marker,
matching the existing scaffold convention and keeping the `login` block aligned
with the other locale files.
apps/carrier/src/i18n/locales/pt-PT.ts (1)

2121-2135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

nit: missing per-key TODO markers on the new login.* scaffold.

Same as the sibling locale bundles — consider // TODO: pt translation per key for consistency with completeness tracking.

🤖 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/carrier/src/i18n/locales/pt-PT.ts` around lines 2121 - 2135, The new
login scaffold in pt-PT is missing the per-key TODO markers used by the other
locale bundles for translation tracking. Update the login block in the locale
object so each `login.*` entry is annotated consistently with a `// TODO: pt
translation` note, keeping the structure aligned with the existing locale
convention in this file and the sibling locale bundles.
apps/carrier/src/i18n/locales/hi-IN.ts (1)

2121-2135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

nit: missing per-key TODO markers on the new login.* scaffold.

Same issue as the other locale bundles in this PR — every other fallback key here uses // TODO: hi translation per the file's own convention, but the new login.* keys don't. Worth aligning for consistency with completeness tracking.

🤖 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/carrier/src/i18n/locales/hi-IN.ts` around lines 2121 - 2135, The new
login scaffold in hi-IN.ts is missing the per-key fallback TODO markers used
elsewhere in this locale bundle. Update the `login` object entries (`heading`,
`subtitle`, `usernameLabel`, `passwordLabel`, `submit`, `submitting`,
`checkingSession`, `errorNetwork`, `errorInvalid`) to follow the same `// TODO:
hi translation` convention so completeness tracking stays consistent with the
other locale keys.
apps/carrier/src/i18n/locales/pt-BR.ts (1)

2121-2135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

nit: missing per-key TODO markers on the new login.* scaffold.

Same as the sibling locale bundles — consider // TODO: pt translation per key for consistency with completeness tracking.

🤖 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/carrier/src/i18n/locales/pt-BR.ts` around lines 2121 - 2135, The new
login scaffold in the pt-BR locale is missing the per-key TODO markers used
elsewhere for untranslated entries. Update the `login` object entries
(`heading`, `subtitle`, `usernameLabel`, `passwordLabel`, `submit`,
`submitting`, `checkingSession`, `errorNetwork`, `errorInvalid`) to follow the
same completeness-tracking convention by adding the appropriate `// TODO: pt
translation` markers consistently. Keep the structure aligned with the sibling
locale bundles so the `login` section remains easy to audit later.
apps/carrier/src/i18n/locales/fr-FR.ts (1)

2121-2135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

nit: missing per-key TODO markers on the new login.* scaffold.

Every other English-fallback key in this file (common.*, shell.*, nav.*, etc.) carries an inline // TODO: fr translation comment per the file's documented convention (line 8). The new login.* block only has a block-level comment noting it's pending translation, but no per-key markers.

If the roster/completeness tooling scans for these // TODO: comments to compute completenessFloor stats, these keys could be silently miscounted as translated.

✏️ Suggested per-key markers
   login: {
-    heading: 'Sign in to Harborline',
-    subtitle: 'Sign in with your account on this node.',
-    usernameLabel: 'Username',
-    passwordLabel: 'Password',
-    submit: 'Sign in',
-    submitting: 'Signing in…',
-    checkingSession: 'Checking your session…',
-    errorNetwork: 'Cannot reach the node. Check the connection and try again.',
-    errorInvalid: 'Sign-in failed. Check your username and password.',
+    heading: 'Sign in to Harborline', // TODO: fr translation
+    subtitle: 'Sign in with your account on this node.', // TODO: fr translation
+    usernameLabel: 'Username', // TODO: fr translation
+    passwordLabel: 'Password', // TODO: fr translation
+    submit: 'Sign in', // TODO: fr translation
+    submitting: 'Signing in…', // TODO: fr translation
+    checkingSession: 'Checking your session…', // TODO: fr translation
+    errorNetwork: 'Cannot reach the node. Check the connection and try again.', // TODO: fr translation
+    errorInvalid: 'Sign-in failed. Check your username and password.', // TODO: fr translation
   },
🤖 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/carrier/src/i18n/locales/fr-FR.ts` around lines 2121 - 2135, The new
login.* scaffold is missing the per-key TODO markers used elsewhere in this
file, so add the inline `// TODO: fr translation` comment to each key in the
`login` block. Keep the existing English fallback values, but mirror the
established convention from other localized sections so the completeness tooling
can correctly recognize these entries as untranslated.
🤖 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/carrier/src/i18n/locales/ar-AE.ts`:
- Around line 2168-2185: The new login.* strings in ar-AE.ts are still English,
which breaks the locale’s no-English-chrome promise. Update the login block in
the ar-AE locale with proper Arabic translations for the WebSessionGate flow, or
if this screen is intentionally exempt, add a clear file-header note carving out
the exception. Keep the existing login object shape and keys intact so the
WebSessionGate.tsx lookup still works.

---

Nitpick comments:
In `@apps/carrier/src/i18n/locales/fr-FR.ts`:
- Around line 2121-2135: The new login.* scaffold is missing the per-key TODO
markers used elsewhere in this file, so add the inline `// TODO: fr translation`
comment to each key in the `login` block. Keep the existing English fallback
values, but mirror the established convention from other localized sections so
the completeness tooling can correctly recognize these entries as untranslated.

In `@apps/carrier/src/i18n/locales/hi-IN.ts`:
- Around line 2121-2135: The new login scaffold in hi-IN.ts is missing the
per-key fallback TODO markers used elsewhere in this locale bundle. Update the
`login` object entries (`heading`, `subtitle`, `usernameLabel`, `passwordLabel`,
`submit`, `submitting`, `checkingSession`, `errorNetwork`, `errorInvalid`) to
follow the same `// TODO: hi translation` convention so completeness tracking
stays consistent with the other locale keys.

In `@apps/carrier/src/i18n/locales/it-IT.ts`:
- Around line 2121-2135: The new login scaffold in the locale bundle is missing
the per-key TODO markers used elsewhere for fallback strings. Update the login
section in the locale object so each `login.*` entry carries the same `// TODO:
it translation` marker convention as other untranslated keys, keeping the
existing `login` grouping and symbol names intact.

In `@apps/carrier/src/i18n/locales/ja-JP.ts`:
- Around line 2121-2135: The new login scaffold in the locale object is missing
the per-key translation TODO markers used elsewhere in this file. Update the
`login` entries in `ja-JP.ts` so each key under `login` has the consistent `//
TODO: ja translation` marker, matching the existing scaffold convention and
keeping the `login` block aligned with the other locale files.

In `@apps/carrier/src/i18n/locales/nl-NL.ts`:
- Around line 2121-2135: Add the missing per-key scaffold markers for the new
login entries in the nl-NL locale object. Update the `login` section so each key
under `login` includes the same `// TODO: nl translation` convention used
elsewhere in the locale files, keeping the existing English fallback values
intact.

In `@apps/carrier/src/i18n/locales/pt-BR.ts`:
- Around line 2121-2135: The new login scaffold in the pt-BR locale is missing
the per-key TODO markers used elsewhere for untranslated entries. Update the
`login` object entries (`heading`, `subtitle`, `usernameLabel`, `passwordLabel`,
`submit`, `submitting`, `checkingSession`, `errorNetwork`, `errorInvalid`) to
follow the same completeness-tracking convention by adding the appropriate `//
TODO: pt translation` markers consistently. Keep the structure aligned with the
sibling locale bundles so the `login` section remains easy to audit later.

In `@apps/carrier/src/i18n/locales/pt-PT.ts`:
- Around line 2121-2135: The new login scaffold in pt-PT is missing the per-key
TODO markers used by the other locale bundles for translation tracking. Update
the login block in the locale object so each `login.*` entry is annotated
consistently with a `// TODO: pt translation` note, keeping the structure
aligned with the existing locale convention in this file and the sibling locale
bundles.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c7651b01-9b85-47f9-af8a-d40c668ac33a

📥 Commits

Reviewing files that changed from the base of the PR and between 5e79209 and a4fdbc9.

📒 Files selected for processing (24)
  • apps/carrier/src/components/HarborlineMark.tsx
  • apps/carrier/src/i18n/locales/ar-AE.ts
  • apps/carrier/src/i18n/locales/ar.ts
  • apps/carrier/src/i18n/locales/bn.ts
  • apps/carrier/src/i18n/locales/de-DE.ts
  • apps/carrier/src/i18n/locales/en-GB.ts
  • apps/carrier/src/i18n/locales/en-US.ts
  • apps/carrier/src/i18n/locales/es-419.ts
  • apps/carrier/src/i18n/locales/es-ES.ts
  • apps/carrier/src/i18n/locales/fr-CA.ts
  • apps/carrier/src/i18n/locales/fr-FR.ts
  • apps/carrier/src/i18n/locales/hi-IN.ts
  • apps/carrier/src/i18n/locales/it-IT.ts
  • apps/carrier/src/i18n/locales/ja-JP.ts
  • apps/carrier/src/i18n/locales/nl-NL.ts
  • apps/carrier/src/i18n/locales/pt-BR.ts
  • apps/carrier/src/i18n/locales/pt-PT.ts
  • apps/carrier/src/i18n/locales/ru-RU.ts
  • apps/carrier/src/i18n/locales/ur-PK.ts
  • apps/carrier/src/i18n/locales/ur.ts
  • apps/carrier/src/i18n/locales/zh-CN.ts
  • apps/carrier/src/i18n/locales/zh-TW.ts
  • apps/carrier/src/index.css
  • apps/carrier/src/webclient/WebSessionGate.tsx

Comment on lines +2168 to +2185
// Web-client login gate (2026-07-07 brand/UX rework — WebSessionGate.tsx). Web-profile only;
// the desktop build dead-code-eliminates this module (no login there). English fallback
// pending a dedicated translator pass (matches the scaffold convention used elsewhere in this
// file — see the file-header note).
login: {
heading: 'Sign in to Harborline',
subtitle: 'Sign in with your account on this node.',
usernameLabel: 'Username',
passwordLabel: 'Password',
submit: 'Sign in',
submitting: 'Signing in…',
checkingSession: 'Checking your session…',
errorNetwork: 'Cannot reach the node. Check the connection and try again.',
errorInvalid: 'Sign-in failed. Check your username and password.',
},
}


Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

issue: login.* breaks ar-AE's "no English chrome" guarantee

ar-AE is documented as the one fully-translated Arabic bundle ("the app no longer renders English chrome under ar-AE"), but the new login block ships pure English with a comment borrowing the scaffold convention from ar.ts/ru-RU.ts/etc. Since this is the first screen a user sees, it's a visible regression against this file's own completeness claim — worth a translator pass (or at least a note in the file header carving out this exception) before merge.

🤖 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/carrier/src/i18n/locales/ar-AE.ts` around lines 2168 - 2185, The new
login.* strings in ar-AE.ts are still English, which breaks the locale’s
no-English-chrome promise. Update the login block in the ar-AE locale with
proper Arabic translations for the WebSessionGate flow, or if this screen is
intentionally exempt, add a clear file-header note carving out the exception.
Keep the existing login object shape and keys intact so the WebSessionGate.tsx
lookup still works.

…/F5)

Addresses the design-reviewer's APPROVE-WITH-FOLLOWUPS verdict on the login-gate
rework (PR #1914):

- F1 (major): the rise/fade keyframe rule was unlayered, so it always outranked
  Tailwind v4's motion-reduce:animate-none utility (which lives in @layer
  utilities) regardless of source order. Added an explicit, also-unlayered
  @media (prefers-reduced-motion: reduce) override in index.css, which competes
  on equal footing and wins on source order. Verified with Playwright's
  reduced-motion emulation (animationName resolves to "none").

- F2 (major): the standalone pre-auth ThemeToggle mount had ~20px segments,
  under the WCAG 2.2 target-size floor. Added a size prop ('compact' default,
  unchanged; 'comfortable' for standalone mounts) and wired the login gate to
  'comfortable' (44px). The in-menu/Settings-row usage is untouched.

- F3: webWhoami had no request timeout, so a hung node could pulse the splash
  forever. Bounded the boot fetch with AbortSignal.timeout(8000) so it falls
  through to the honest login-screen fallback instead of hanging indefinitely.

- F4: localized ThemeToggle's own strings (label + per-option label/ariaLabel)
  via a new themeToggle.* i18n block across all 20 locale catalogs, matching
  the English-fallback scaffold convention already used in these files.

- F5: the mark + wordmark + heading triple-announced "Harborline" to a screen
  reader. Added a `decorative` mode to HarborlineMark (aria-hidden, no
  role/aria-label) and aria-hidden the redundant wordmark text, leaving the
  heading as the one accessible name for the composed lockup.

Naming (F6, "Sign in to Harborline") is left as-is per the reviewer's note —
that's an open CIC decision, not an engineering fix.

Refs: council-verdict on shipyard#1914
@ctwoodwa
ctwoodwa enabled auto-merge (squash) July 7, 2026 09:51
@ctwoodwa

ctwoodwa commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Design-review fixes pushed (commit f98cfbc) and auto-merge armed:

  • F1 (major, unlayered animation defeating motion-reduce) — fixed with an explicit unlayered @media (prefers-reduced-motion: reduce) override; verified with Playwright reduced-motion emulation (animationNamenone).
  • F2 (major, ~20px touch targets) — added a size prop to ThemeToggle (compact default unchanged, comfortable for standalone mounts); the login gate now uses 44px segments, verified via bounding-box measurement at 390px width.
  • F3 — bounded webWhoami's boot fetch with AbortSignal.timeout(8000) so a hung node falls through to the login screen instead of pulsing forever.
  • F4 — localized ThemeToggle's own strings via a new themeToggle.* i18n block across all 20 locale catalogs.
  • F5HarborlineMark gained a decorative mode (aria-hidden, no role/label); the redundant wordmark text is aria-hidden too. Verified: svg[aria-hidden]="true", role=null, heading is the sole accessible name.
  • F6 left untouched per your note (open CIC naming decision).

All gates re-run foreground and green: tsc -b --noEmit (0 errors), eslint src (0 warnings), scoped vitest run (57 passed). Auto-merge armed (gh pr merge --squash --auto).

@ctwoodwa
ctwoodwa merged commit c0c640f into main Jul 7, 2026
34 checks passed
@ctwoodwa
ctwoodwa deleted the fed-login-brand-delight branch July 7, 2026 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant