From 20804e6bc6e42fe5f5b7f6821c1a2876fc1fe290 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Sat, 19 Sep 2026 08:38:40 +0000 Subject: [PATCH] fix(mobile): keep disabled primary button labels legible Explorer finding: composer-repo-chosen: The primary 'Start session' label is low-contrast olive-on-olive in the dark theme even though the form is complete, so the main action reads as disabled and is hard to read. The user-agent explorer found this while using the app like a user. One finding per item; the explorer never edits product code. Flow: composer-repo-chosen Found on revision: fd04ca1ef Repro: 1. set this state first: account e2e-mobile-cloud-android@example.com signed in, $10 credits, 3 Code Reviewer pull requests, a second organization to switch to, display size 1440x3200, system day mode, portrait. 2. open the app on emulator-5554 3. reach the screen the capture names (composer-repo-chosen) 4. the capture shows the defect named below Observed: The primary 'Start session' label is low-contrast olive-on-olive in the dark theme even though the form is complete, so the main action reads as disabled and is hard to read. Expected: the screen renders without this defect Evidence (from the device run): - ~/.local/share/kwf/findings/explorer-composer-repo-chosen-the-primary-start-session-l-dfecb6b6/composer-repo-chosen.png --- apps/mobile/scripts/generate-theme-colors.mjs | 1 + .../agents/markdown-palette.test.ts | 1 + .../agents/new-session-start-button.tsx | 21 +++++----- .../bootstrap-error-screen.mounted.test.tsx | 5 ++- .../src/components/ui/button.mounted.test.tsx | 41 +++++++++++++++++++ apps/mobile/src/components/ui/button.tsx | 17 +++++++- apps/mobile/src/global.css | 7 ++++ .../src/lib/hooks/theme-colors.generated.ts | 2 + .../hooks/use-theme-colors.contrast.test.ts | 15 +++++++ 9 files changed, 98 insertions(+), 12 deletions(-) diff --git a/apps/mobile/scripts/generate-theme-colors.mjs b/apps/mobile/scripts/generate-theme-colors.mjs index 246d9fc115..51768576fd 100644 --- a/apps/mobile/scripts/generate-theme-colors.mjs +++ b/apps/mobile/scripts/generate-theme-colors.mjs @@ -16,6 +16,7 @@ export const TOKEN_KEYS = [ ['foreground', '--foreground'], ['primary', '--primary'], ['primaryForeground', '--primary-foreground'], + ['primaryDisabled', '--primary-disabled'], ['secondary', '--secondary'], ['secondaryForeground', '--secondary-foreground'], ['muted', '--muted'], diff --git a/apps/mobile/src/components/agents/markdown-palette.test.ts b/apps/mobile/src/components/agents/markdown-palette.test.ts index d287b16231..4d52fc33a4 100644 --- a/apps/mobile/src/components/agents/markdown-palette.test.ts +++ b/apps/mobile/src/components/agents/markdown-palette.test.ts @@ -8,6 +8,7 @@ const colors = { foreground: '#14130F', primary: '#4F5A10', primaryForeground: '#FFFFFF', + primaryDisabled: '#6F6A61', secondary: '#F0EEE6', secondaryForeground: '#14130F', muted: '#F0EEE6', diff --git a/apps/mobile/src/components/agents/new-session-start-button.tsx b/apps/mobile/src/components/agents/new-session-start-button.tsx index 9755d99360..258f82fb7c 100644 --- a/apps/mobile/src/components/agents/new-session-start-button.tsx +++ b/apps/mobile/src/components/agents/new-session-start-button.tsx @@ -1,9 +1,7 @@ -import { ActivityIndicator } from '@/components/ui/activity-indicator'; import { useTranslation } from 'react-i18next'; import { Button } from '@/components/ui/button'; import { Text } from '@/components/ui/text'; -import { useThemeColors } from '@/lib/hooks/use-theme-colors'; type NewSessionStartButtonProps = { isCloneEntry: boolean; @@ -16,7 +14,9 @@ type NewSessionStartButtonProps = { /** * The new-session Start submit button. The Continue form shows a busy label * (import for a live CLI, clone for Cloud Agent) and keeps the visible child; - * the ordinary form swaps in a spinner. + * the ordinary form swaps in the Button's own busy spinner. Both pass + * `loading` so the busy state keeps the brand fill instead of the muted + * disabled fill. */ export function NewSessionStartButton({ isCloneEntry, @@ -25,7 +25,6 @@ export function NewSessionStartButton({ isStarting, onStartSession, }: Readonly) { - const colors = useThemeColors(); const { t } = useTranslation(); if (isCloneEntry) { @@ -49,12 +48,14 @@ export function NewSessionStartButton({ } return ( - ); } diff --git a/apps/mobile/src/components/bootstrap-error-screen.mounted.test.tsx b/apps/mobile/src/components/bootstrap-error-screen.mounted.test.tsx index b2f44d42cd..7b94bf32d3 100644 --- a/apps/mobile/src/components/bootstrap-error-screen.mounted.test.tsx +++ b/apps/mobile/src/components/bootstrap-error-screen.mounted.test.tsx @@ -116,7 +116,10 @@ describe('BootstrapErrorScreen primaryLoading', () => { const primary = findPressableByAccessibilityLabel(renderer.root, 'Retry loading account'); expect(primary.props.disabled).toBe(true); expect(primary.props.accessibilityState).toEqual({ disabled: true, busy: true }); - expect(primary.props.className).toContain('opacity-50'); + // A busy primary keeps its brand fill: dimming the whole control would + // composite the ink label into the washed fill (about 2.4:1 in light). + expect(primary.props.className).toContain('bg-primary'); + expect(primary.props.className).not.toContain('opacity-50'); // The busy indicator is the button's inline spinner; the label stays. expect( renderer.root.findAll( diff --git a/apps/mobile/src/components/ui/button.mounted.test.tsx b/apps/mobile/src/components/ui/button.mounted.test.tsx index 8fb7805ca5..0c47ee01a0 100644 --- a/apps/mobile/src/components/ui/button.mounted.test.tsx +++ b/apps/mobile/src/components/ui/button.mounted.test.tsx @@ -167,6 +167,47 @@ describe('Button native target contract', () => { expect(button.findByType(NativeText).children).toEqual(['Retry']); }); + it('keeps a disabled primary label legible instead of dimming the brand pair', () => { + const button = renderButton({ disabled: true }); + const classes = (button.props.className as string).split(' '); + // bg-primary-disabled must win over the variant's bg-primary so the disabled + // fill is the muted surface the ink label stays readable on. + expect(classes).toContain('bg-primary-disabled'); + expect(classes).not.toContain('bg-primary'); + expect(classes).not.toContain('opacity-50'); + // The label keeps the ink colour; the disabled fill is chosen to contrast + // with it, so hard-coded primaryForeground children stay legible too. + const label = button.findByType(NativeText).props.className as string; + expect(label).toContain('text-primary-foreground'); + expect(label).not.toContain('text-muted-foreground'); + }); + + it('keeps a hard-coded primaryForeground child on a fill that contrasts with it', () => { + const button = renderButton({ + disabled: true, + children: createElement(Text, { className: 'text-primary-foreground' }, 'Save'), + }); + const classes = (button.props.className as string).split(' '); + expect(classes).toContain('bg-primary-disabled'); + expect(classes).not.toContain('bg-muted'); + expect(button.findByType(NativeText).props.className).toContain('text-primary-foreground'); + }); + + it('keeps a busy primary on its brand fill so it still reads as working', () => { + const button = renderButton({ loading: true }); + const classes = (button.props.className as string).split(' '); + expect(classes).toContain('bg-primary'); + expect(classes).not.toContain('bg-primary-disabled'); + expect(classes).not.toContain('opacity-50'); + }); + + it('keeps the dimmed treatment for a disabled non-primary variant', () => { + const button = renderButton({ variant: 'outline', disabled: true }); + const classes = (button.props.className as string).split(' '); + expect(classes).toContain('opacity-50'); + expect(classes).not.toContain('bg-primary-disabled'); + }); + it('keeps an enabled action connected to its visible outcome', () => { function Action() { const [started, setStarted] = useState(false); diff --git a/apps/mobile/src/components/ui/button.tsx b/apps/mobile/src/components/ui/button.tsx index 84a470cb06..d8d1961fc8 100644 --- a/apps/mobile/src/components/ui/button.tsx +++ b/apps/mobile/src/components/ui/button.tsx @@ -103,10 +103,25 @@ function Button({ }: ButtonProps) { const colors = useThemeColors(); const isDisabled = Boolean(disabled) || Boolean(loading); + const isPrimary = (variant ?? 'default') === 'default'; + // A default-variant fill is a saturated brand colour with a contrasting ink + // label. Halving the whole control's opacity for the disabled state + // composites that pair into olive-on-olive (about 2.4:1 in light, 4.4:1 in + // dark), and it also washes out any child that hard-codes primaryForeground. + // A disabled (not busy) primary instead takes a muted fill that still + // contrasts with the ink label, so the label stays legible. A busy primary + // keeps the brand fill and its spinner so it still reads as working. + const isMutedDisabled = isDisabled && !loading && isPrimary; + const isDimmed = isDisabled && !isPrimary; return ( { }); }); +describe('disabled primary fill contrast (WCAG AA text)', () => { + // A disabled default-variant Button paints primaryForeground on + // `--primary-disabled`; the pair must clear 4.5:1 in both themes so the + // label (and any child that hard-codes primaryForeground) stays legible. + it('light theme: primaryForeground vs primaryDisabled >= 4.5:1', () => { + const ratio = contrastRatio(lightColors.primaryForeground, lightColors.primaryDisabled); + expect(ratio).toBeGreaterThanOrEqual(MIN_TEXT_RATIO); + }); + + it('dark theme: primaryForeground vs primaryDisabled >= 4.5:1', () => { + const ratio = contrastRatio(darkColors.primaryForeground, darkColors.primaryDisabled); + expect(ratio).toBeGreaterThanOrEqual(MIN_TEXT_RATIO); + }); +}); + describe('warn foreground token contrast (WCAG AA text)', () => { it('light theme: warnForeground vs warn >= 4.5:1', () => { // Precomputed ≈ 5.30:1 for #FFFFFF on #956011.