diff --git a/apps/mobile/src/components/pr-review/pr-review-checks-section.mounted.test.tsx b/apps/mobile/src/components/pr-review/pr-review-checks-section.mounted.test.tsx index 6fec4d266c..f86c6b6b01 100644 --- a/apps/mobile/src/components/pr-review/pr-review-checks-section.mounted.test.tsx +++ b/apps/mobile/src/components/pr-review/pr-review-checks-section.mounted.test.tsx @@ -1,7 +1,10 @@ /* eslint-disable typescript-eslint/no-deprecated -- react-test-renderer is the repository's native-free mounted test tool. */ +// eslint-disable-next-line import/no-nodejs-modules -- vitest-only theme-token guard, runs in node, never bundled into the app +import { readFileSync } from 'node:fs'; + import { createElement } from 'react'; import TestRenderer, { act } from 'react-test-renderer'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { SpinningIcon } from '@/components/ui/spinning-icon'; import { ProviderPrScopeProvider } from '@/lib/pr-review/provider-pr-ref'; @@ -62,6 +65,7 @@ vi.mock('@/components/ui/icons', () => ({ XCircle: 'XCircle', })); vi.mock('@/components/ui/spinning-icon', () => ({ SpinningIcon: 'SpinningIcon' })); +vi.mock('@/components/ui/skeleton', () => ({ Skeleton: 'Skeleton' })); vi.mock('@/components/ui/text', () => ({ Text: 'Text' })); vi.mock('@/components/pr-review/pr-review-reconnect-notice', () => ({ PrReviewReconnectNotice: 'PrReviewReconnectNotice', @@ -204,3 +208,129 @@ describe('PrReviewChecksSection view-on-provider link', () => { restore(); }); }); + +// Spot check e1-nav-mr.png: the CHECKS section rendered as an empty gray +// block — no loading indicator, no empty copy, no error copy. The screen +// was in the loading state, and the state was invisible: the skeleton bars +// carried `bg-muted` inside a `bg-secondary` card, and `--muted` equals +// `--secondary` in BOTH themes (apps/mobile/src/global.css), so the bars +// painted the card's own colour. This test pins the fixed render path: the +// card holds three shared Skeleton bars in `bg-muted-soft` — the one gray +// that differs from the card in both themes — and no bar keeps the +// collision token. The CSS guard below proves the collision is real +// (`--muted` == `--secondary`) and that the token the bars now use is not +// the collision token, so a future theme change that reintroduces the +// collision fails here instead of shipping another empty gray block. +describe('PrReviewChecksSection loading state is visible on the card', () => { + const previous = { isLoading: false, data: query.data }; + + beforeEach(() => { + query.isLoading = true; + }); + + afterEach(() => { + query.isLoading = previous.isLoading; + query.data = previous.data; + }); + + function mountLoading() { + const renderer: { current: TestRenderer.ReactTestRenderer | undefined } = { + current: undefined, + }; + act(() => { + renderer.current = TestRenderer.create( + createElement(PrReviewChecksSection, { + owner: 'group/sub', + repo: 'repo', + number: 12, + headSha: 'head', + }) + ); + }); + const created = renderer.current; + if (!created) { + throw new Error('renderer was not created'); + } + return created; + } + + function findCard(renderer: TestRenderer.ReactTestRenderer) { + const cards = renderer.root.findAll( + node => + String(node.type) === 'View' && + typeof node.props.className === 'string' && + node.props.className.split(/\s+/).includes('bg-secondary') + ); + expect(cards).toHaveLength(1); + const card = cards[0]; + if (!card) { + throw new Error('the checks card did not render'); + } + return card; + } + + it('paints three animated skeleton bars in a colour distinct from the card', () => { + const renderer = mountLoading(); + const card = findCard(renderer); + + // The shared Skeleton component — the app's loading indicator (pulse + + // shimmer), not a static block. + const bars = card.findAll(node => String(node.type) === 'Skeleton'); + expect(bars).toHaveLength(3); + for (const bar of bars) { + const className = String(bar.props.className ?? ''); + expect(className.split(/\s+/)).toContain('bg-muted-soft'); + } + + // The defect itself: no node in the card may keep the bare `bg-muted` + // token — on this card it is the card's own colour, i.e. invisible. + const invisible = card.findAll( + node => + typeof node.props.className === 'string' && + /(^|\s)bg-muted(\s|$)/.test(node.props.className) + ); + expect(invisible).toHaveLength(0); + + // The state is announced, not only shown. + expect(card.props.accessibilityRole).toBe('progressbar'); + expect(card.props.accessibilityLabel).toBe('common.loading'); + + act(() => { + renderer.unmount(); + }); + }); + + it('keeps the CHECKS heading rendered while loading, so the block is labelled', () => { + const renderer = mountLoading(); + const headings = renderer.root.findAll( + node => String(node.type) === 'Text' && node.props.children === 'prReview.checks.title' + ); + expect(headings).toHaveLength(1); + act(() => { + renderer.unmount(); + }); + }); + + it('guards the theme tokens: bg-muted collides with the card, bg-muted-soft does not', () => { + const css = readFileSync(new URL('../../global.css', import.meta.url), 'utf8'); + const values = (name: string) => + [...css.matchAll(new RegExp(`--${name}:\\s*([^;]+);`, 'g'))].map(match => + (match[1] ?? '').trim().toLowerCase() + ); + const secondary = values('secondary'); + const muted = values('muted'); + const mutedSoft = values('muted-soft'); + + // Both theme blocks are present. + expect(secondary.length).toBeGreaterThanOrEqual(2); + expect(muted).toHaveLength(secondary.length); + expect(mutedSoft).toHaveLength(secondary.length); + // The collision that made the old skeleton invisible, pinned so the + // test above stays meaningful. + expect(muted).toEqual(secondary); + // The fix token must contrast with the card in every theme. + for (const [index, soft] of mutedSoft.entries()) { + expect(soft).not.toBe(secondary[index]); + } + }); +}); diff --git a/apps/mobile/src/components/pr-review/pr-review-checks-section.tsx b/apps/mobile/src/components/pr-review/pr-review-checks-section.tsx index 5be63a710f..6f21c1d2bc 100644 --- a/apps/mobile/src/components/pr-review/pr-review-checks-section.tsx +++ b/apps/mobile/src/components/pr-review/pr-review-checks-section.tsx @@ -1,3 +1,4 @@ +/* eslint-disable max-lines -- the section owns every CHECKS state in one file: the card shell, the tone/rollup helpers and the rows share one surface, and splitting the visible-loading fix away from the states it must match scatters it across callers. */ import { useQuery } from '@tanstack/react-query'; import { type inferRouterOutputs, type MobileRouter } from '@kilocode/trpc/mobile'; import { @@ -14,6 +15,7 @@ import { Pressable, View } from 'react-native'; import { PrReviewReconnectNotice } from '@/components/pr-review/pr-review-reconnect-notice'; import { Button } from '@/components/ui/button'; +import { Skeleton } from '@/components/ui/skeleton'; import { SpinningIcon } from '@/components/ui/spinning-icon'; import { Text } from '@/components/ui/text'; import { i18n } from '@/i18n'; @@ -190,16 +192,26 @@ export function PrReviewChecksSection({ // Loading (first time, no cached data): show three skeleton rows in a // card so the section matches the final dimensions once the data lands. + // The bars must NOT be `bg-muted` here: `--muted` and `--secondary` are + // the same colour in both themes (apps/mobile/src/global.css), so a + // `bg-muted` bar inside this `bg-secondary` card paints nothing and the + // section reads as an empty gray block (spot check e1-nav-mr). The shared + // Skeleton gives the pulse + shimmer, and `bg-muted-soft` is the one gray + // that contrasts with the card in both themes. if (checks.isLoading) { return ( {t('prReview.checks.title')} - - - - + + + + );