Skip to content

fix(a11y): raise navigation text contrast - #2024

Merged
Asherlc merged 2 commits into
mainfrom
codex/issue-1990
Jul 26, 2026
Merged

Asherlc merged 2 commits into
mainfrom
codex/issue-1990

Conversation

@Asherlc

@Asherlc Asherlc commented Jul 26, 2026

Copy link
Copy Markdown
Owner

Summary

  • use the existing secondary text token for inactive iOS tab labels
  • use the matching muted text token for web header sign-out actions
  • add web and mobile regressions that enforce the WCAG AA 4.5:1 minimum for normal text

The promoted token measures 5.41:1 on the iOS tab background and 5.79:1 on the web header background. This follows the WCAG 2.2 minimum contrast guidance.

Validation

  • pnpm exec vitest run --project unit --project mobile 'packages/mobile/app/(tabs)/_layout.test.tsx' packages/web/src/components/AppHeader.test.tsx
  • pnpm test -- --run (836 files passed, 2 skipped; 13,734 tests passed, 21 skipped)
  • pnpm --dir packages/mobile typecheck
  • pnpm --dir packages/web typecheck
  • pnpm tsc --noEmit
  • pnpm tsc --noEmit (packages/server)
  • pnpm --dir packages/web build
  • pnpm exec biome check . --max-diagnostics=500
  • Docker-free policy lint gates

Fixes #1990


Summary by cubic

Increase navigation text contrast on iOS tabs and the web header to meet WCAG 2.2 AA (≥4.5:1). Fixes #1990 with token updates and stronger tests to prevent regressions.

  • Bug Fixes
    • Mobile: set inactive tab labels to colors.textSecondary (was colors.textTertiary) for AA contrast.
    • Web: switch “Sign out” buttons to text-muted (was text-subtle).
    • Tests: enforce AA contrast for inactive tab labels and sign-out actions, and assert the .text-muted class on web; current ratios are 5.41:1 (iOS tabs) and 5.79:1 (web header).

Written for commit b48f34d. Summary will update on new commits.

Review in cubic

Copilot AI review requested due to automatic review settings July 26, 2026 07:33
@Asherlc Asherlc linked an issue Jul 26, 2026 that may be closed by this pull request
@codereviewbot-ai

Copy link
Copy Markdown

🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews.

@cursor

cursor Bot commented Jul 26, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry @Asherlc, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in: 14 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: ASSERTIVE

Plan: Pro Plus

Run ID: f39146d2-77e7-44ef-8b4e-4eb3ee3b4557

📥 Commits

Reviewing files that changed from the base of the PR and between 1b88f01 and b48f34d.

📒 Files selected for processing (4)
  • packages/mobile/app/(tabs)/_layout.test.tsx
  • packages/mobile/app/(tabs)/_layout.tsx
  • packages/web/src/components/AppHeader.test.tsx
  • packages/web/src/components/AppHeader.tsx

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.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix a11y: raise navigation text contrast to WCAG AA

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Increase inactive iOS tab label contrast by promoting to the secondary text token.
• Update web header sign-out actions to use the muted text token.
• Add mobile and web regressions enforcing WCAG AA 4.5:1 contrast for normal text.
Diagram

graph TD
  A["Design tokens (colors)"] --> B["Mobile Tabs options"] --> C["Mobile tabs test"] --> F["WCAG AA 4.5:1"]
  A --> D["Web AppHeader"] --> E["Web header test"] --> F
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Extract a shared contrast helper for tests
  • ➕ Avoids duplicating luminance/contrast math across test suites
  • ➕ Encourages consistent AA assertions across platforms
  • ➖ Introduces an additional shared test utility module to maintain
  • ➖ May require wiring for both web and mobile test environments
2. Add token-level contrast audits (snapshot/table-driven)
  • ➕ Scales to cover many token/background pairs at once
  • ➕ Catches regressions even when components change
  • ➖ Harder to keep aligned with real component backgrounds
  • ➖ More up-front effort to define the correct matrix of pairs
3. Adopt an accessibility rule/lint gate for contrast
  • ➕ Prevents regressions earlier than unit tests
  • ➕ Standardizes enforcement across the codebase
  • ➖ Contrast linting is often approximate without real computed styles
  • ➖ Can create false positives/negatives and developer friction

Recommendation: The PR’s approach (promote to safer existing tokens + targeted component regressions) is the best near-term fix: it improves contrast with minimal design churn and locks behavior where it matters (tabs + header actions). A small follow-up to extract the contrast helper into a shared test utility would reduce duplication without changing the strategy.

Files changed (4) +55 / -3

Bug fix (2) +3 / -3
_layout.tsxPromote inactive tab label color to textSecondary +1/-1

Promote inactive tab label color to textSecondary

• Changes the inactive tab tint color from the tertiary text token to the secondary text token to increase contrast on the tab bar background.

packages/mobile/app/(tabs)/_layout.tsx

AppHeader.tsxUse text-muted for header sign-out actions +2/-2

Use text-muted for header sign-out actions

• Updates both sign-out button class names from "text-subtle" to "text-muted" to align with the intended muted navigation token and improve contrast.

packages/web/src/components/AppHeader.tsx

Tests (2) +52 / -0
_layout.test.tsxAdd WCAG AA contrast regression for inactive tab labels +22/-0

Add WCAG AA contrast regression for inactive tab labels

• Introduces relative luminance and contrast ratio helpers in the test suite. Adds an assertion ensuring the inactive tab label color meets the WCAG AA 4.5:1 contrast ratio against the tab background.

packages/mobile/app/(tabs)/_layout.test.tsx

AppHeader.test.tsxAssert sign-out actions use AA-contrast token and meet 4.5:1 +30/-0

Assert sign-out actions use AA-contrast token and meet 4.5:1

• Adds contrast ratio helpers and imports surface/text token values for validation. Verifies both sign-out buttons use the "text-muted" class and asserts the selected token meets WCAG AA contrast against the header background.

packages/web/src/components/AppHeader.test.tsx

@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Mobile Preview

Scan to open on device:

QR code for dofek://preview/pr-2024

Channel pr-2024
Deep Link dofek://preview/pr-2024
Commit 770b804

To test on device:

  1. Build and install the preview client: PREVIEW_CHANNEL=pr-2024 pnpm expo prebuild --clean -p ios
  2. Or tap deep link on an existing preview build: dofek://preview/pr-2024

Each PR gets its own channel. Build a preview client with PREVIEW_CHANNEL=pr-{N} to test.

@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Storybook previews for 770b804e are ready:

This comment updates automatically on each PR push.

@qodo-code-review

qodo-code-review Bot commented Jul 26, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 174 rules

Grey Divider


Remediation recommended

1. Brittle className substring check ✓ Resolved 🐞 Bug ☼ Reliability
Description
AppHeader.test.tsx uses className.toContain("text-muted"), which can pass even when the element
does not actually have the standalone text-muted base class (e.g., if only hover:text-muted is
present). This weakens the regression test intended to enforce the sign-out action styling/token
choice.
Code

packages/web/src/components/AppHeader.test.tsx[R76-80]

+    const signOutActions = screen.getAllByRole("button", { name: "Sign out" });
+    expect(signOutActions).toHaveLength(2);
+    for (const signOutAction of signOutActions) {
+      expect(signOutAction.className).toContain("text-muted");
+    }
Relevance

⭐⭐⭐ High

Repo accepted replacing brittle className substring assertions with deterministic
classList.contains/toHaveClass checks.

PR-#1197

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new test uses substring matching for text-muted, which can match other class tokens containing
that substring. The repo already uses patterns where hover:text-muted appears alongside a
different base text color, demonstrating how toContain("text-muted") can be a false positive. A
past accepted bug also calls out this exact brittleness pattern and recommends
classList.contains()-style assertions.

packages/web/src/components/AppHeader.test.tsx[73-84]
packages/web/src/components/FileImportZone.tsx[383-390]
PR-#1197

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test currently checks for `"text-muted"` via substring matching on `element.className`, which can produce false positives (e.g., matching `hover:text-muted`).

## Issue Context
The goal of this test is to ensure the sign-out buttons use the intended base navigation text token/class.

## Fix Focus Areas
- packages/web/src/components/AppHeader.test.tsx[73-84]

## Suggested fix
Replace the substring assertion with an exact class token assertion, e.g.:
- `expect(signOutAction.classList.contains("text-muted")).toBe(true)`

This ensures the base text color class is actually applied (and won’t accidentally pass due to `hover:` variants or other class substrings).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

2. Duplicate contrast helpers in _layout.test 📘 Rule violation ⚙ Maintainability
Description
The contrast calculation helpers (relativeLuminance, contrastRatio) are defined inline in
multiple test files (web and mobile) even though the implementations are identical, resulting in
duplicated shared logic. This violates the requirement to centralize shared helpers (e.g., in
test-helpers.ts or a shared domain module) to prevent drift and reduce maintenance cost.
Code

packages/mobile/app/(tabs)/_layout.test.tsx[R10-24]

+function relativeLuminance(hexColor: string): number {
+  const linearChannel = (start: number) => {
+    const channel = Number.parseInt(hexColor.slice(start, start + 2), 16) / 255;
+    return channel <= 0.04045 ? channel / 12.92 : ((channel + 0.055) / 1.055) ** 2.4;
+  };
+  return 0.2126 * linearChannel(1) + 0.7152 * linearChannel(3) + 0.0722 * linearChannel(5);
+}
+
+function contrastRatio(foreground: string, background: string): number {
+  const foregroundLuminance = relativeLuminance(foreground);
+  const backgroundLuminance = relativeLuminance(background);
+  const lighter = Math.max(foregroundLuminance, backgroundLuminance);
+  const darker = Math.min(foregroundLuminance, backgroundLuminance);
+  return (lighter + 0.05) / (darker + 0.05);
+}
Relevance

⭐ Low

Team previously rejected extracting shared web+mobile helper for duplicated logic; prefers keeping
platform copies.

PR-#1525

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 773554 requires shared test utilities used by multiple test files to be placed in a
test-helpers.ts module rather than duplicated, and the PR introduces identical
relativeLuminance/contrastRatio implementations in both
packages/web/src/components/AppHeader.test.tsx and packages/mobile/app/(tabs)/_layout.test.tsx,
demonstrating duplicated shared test logic. Additionally, PR Compliance ID 773501 flags identical
logic implemented separately under packages/web and packages/mobile instead of being centralized
in a shared domain package, which this PR does by adding the same contrast helpers in those two
platform-specific test files.

Rule 773554: Localize shared test helpers into test-helpers.ts
Rule 773501: Mirror web and mobile changes and centralize shared logic in domain packages
packages/mobile/app/(tabs)/_layout.test.tsx[10-24]
packages/web/src/components/AppHeader.test.tsx[8-22]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`relativeLuminance`/`contrastRatio` are duplicated across multiple test files (and effectively across web and mobile), even though the implementations are identical. Consolidate these helpers into a shared module (either per-area `test-helpers.ts` as required for shared test utilities, and/or a shared domain module) and import them from the tests to avoid divergence.

## Issue Context
PR Compliance ID 773554 requires shared test utilities used by multiple test files to be centralized into a `test-helpers.ts` file rather than being redefined inline. Separately, PR Compliance ID 773501 highlights that identical logic duplicated under `packages/web` and `packages/mobile` should be centralized in a shared domain package when feasible; these WCAG contrast helpers are identical and could potentially be exported from an existing shared module (e.g., `packages/scoring/src/colors.ts`, possibly via an export like `@dofek/scoring/colors`) so future formula changes are made once.

## Fix Focus Areas
- packages/mobile/app/(tabs)/_layout.test.tsx[10-24]
- packages/web/src/components/AppHeader.test.tsx[8-22]
- packages/scoring/src/colors.ts[1-80]
- (new) packages/mobile/app/(tabs)/test-helpers.ts
- (new) packages/web/src/components/test-helpers.ts

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread packages/web/src/components/AppHeader.test.tsx
@codereviewbot-ai

Copy link
Copy Markdown

🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews.

@Asherlc

Asherlc commented Jul 26, 2026

Copy link
Copy Markdown
Owner Author

Review follow-up: fixed the exact class-token assertion in b48f34d and resolved the inline thread. I am keeping the optional contrast helpers platform-local: sharing them across the web/mobile package boundary would require a test-only public package export, which conflicts with the repository rule against production exports solely for testability; Qodo also cites the team precedent preferring platform copies for this case. The focused 12 tests, Biome, and web typecheck pass after the change.

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.

Small navigation labels fail text contrast on both web and iOS

2 participants