Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 56 additions & 2 deletions apps/mobile/src/components/agents/code-block.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,19 @@
import * as React from 'react';
import { act, TestRenderer } from '@/test/renderer';

import { describe, expect, it, vi } from 'vitest';
import { beforeEach, describe, expect, it, vi } from 'vitest';

import { CodeBlock } from './code-block';
import { tokenizeCodeLines } from './code-block-model';
import { type MonoScrollTextMode } from './mono-scroll-block-model';
import { tokenColorFor } from '@/lib/pr-review/diff/syntax-colors';
import '@/i18n';

const { useMonoScrollSheetMock } = vi.hoisted(() => ({
const { useMonoScrollSheetMock, useColorSchemeMock } = vi.hoisted(() => ({
useMonoScrollSheetMock: vi.fn<() => { mode: MonoScrollTextMode; track: () => () => void } | null>(
() => null
),
useColorSchemeMock: vi.fn<() => 'dark' | 'light'>(() => 'light'),
}));

// RNGH ships Flow source that the node project cannot parse, so the horizontal
Expand All @@ -22,6 +23,7 @@ vi.mock('react-native', () => ({
Pressable: 'Pressable',
View: 'View',
Text: 'RNText',
useColorScheme: useColorSchemeMock,
}));
vi.mock('react-native-gesture-handler', () => ({
ScrollView: 'ScrollView',
Expand Down Expand Up @@ -165,6 +167,11 @@ function blockElement(props?: Partial<BlockProps>): React.ReactElement {
return React.createElement(CodeBlock, { code: 'const x = 1;', language: 'typescript', ...props });
}

/** The style.color of every colored run, in render order. */
function runColorValues(root: TestRenderer.ReactTestInstance): string[] {
return colorRuns(root).map(run => (propOf(run, 'style') as { color: string }).color);
}

function withSheet(
mode: MonoScrollTextMode,
track: () => () => void,
Expand All @@ -174,6 +181,12 @@ function withSheet(
return block;
}

// The light palette is the baseline for the suites that assert concrete token
// colors; the theme suite overrides it per test.
beforeEach(() => {
useColorSchemeMock.mockReturnValue('light');
});

describe('CodeBlock', () => {
it('renders one nested RNText per token run', async () => {
const renderer = await mount(blockElement());
Expand Down Expand Up @@ -274,6 +287,47 @@ describe('CodeBlock', () => {
});
});

describe('CodeBlock syntax palette follows the color scheme', () => {
// The palette must come from the color scheme, not from comparing a theme
// token (e.g. `background`) to a hex literal: the generated palette is free
// to change and the tokens would silently flip against their surface.
it('picks the dark token palette on a dark color scheme', async () => {
useColorSchemeMock.mockReturnValue('dark');
const renderer = await mount(blockElement());
const colors = runColorValues(renderer.root);

expect(colors).toContain(tokenColorFor('keyword', true));
expect(colors).toContain(tokenColorFor('number', true));
expect(colors).not.toContain(tokenColorFor('keyword', false));
await unmount(renderer);
});

it('picks the light token palette on a light color scheme', async () => {
useColorSchemeMock.mockReturnValue('light');
const renderer = await mount(blockElement());
const colors = runColorValues(renderer.root);

expect(colors).toContain(tokenColorFor('keyword', false));
expect(colors).toContain(tokenColorFor('number', false));
expect(colors).not.toContain(tokenColorFor('keyword', true));
await unmount(renderer);
});

it('reads the scheme again on a later render instead of caching the palette', async () => {
useColorSchemeMock.mockReturnValue('light');
const renderer = await mount(blockElement());
expect(runColorValues(renderer.root)).toContain(tokenColorFor('keyword', false));

useColorSchemeMock.mockReturnValue('dark');
act(() => {
renderer.update(blockElement({ code: 'let y = 2;' }));
});

expect(runColorValues(renderer.root)).toContain(tokenColorFor('keyword', true));
await unmount(renderer);
});
});

describe('CodeBlock copy action', () => {
it('renders no copy affordance without an onCopyCode handler', async () => {
const renderer = await mount(blockElement());
Expand Down
6 changes: 5 additions & 1 deletion apps/mobile/src/components/agents/code-block.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
type LayoutChangeEvent,
Pressable,
Text as RNText,
useColorScheme,
View,
} from 'react-native';
import { ScrollView } from 'react-native-gesture-handler';
Expand Down Expand Up @@ -117,7 +118,10 @@ function CodeBlockImpl({
const effectiveSelectable = selectable ?? textSelectable;
const colors = useThemeColors();
const { t } = useTranslation();
const isDark = colors.background === '#0E0E10';
// Same signal `useThemeColors` reads. Never infer dark mode from a
// background-token equality: the generated palette can change, and the
// tokens would silently flip against their surface.
const isDark = useColorScheme() === 'dark';
const { displayText, isTruncated } = prepareMonoScrollContent(code, maxLength);
const tokenLines = useMemo(
() => tokenizeCodeLines(displayText, language),
Expand Down
2 changes: 2 additions & 0 deletions apps/mobile/src/components/agents/markdown-renderer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ const rnStub = {
ScrollView: 'ScrollView',
TouchableHighlight: 'TouchableHighlight',
Image: 'Image',
// CodeBlock reads the color scheme to pick its syntax palette.
useColorScheme: () => 'light',
StyleSheet: {
create: (styles: Record<string, unknown>) => styles,
hairlineWidth: 1,
Expand Down
2 changes: 2 additions & 0 deletions apps/mobile/src/components/agents/tool-diff-preview.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ vi.mock('react-native', () => ({
View: 'View',
Text: 'RNText',
Pressable: 'Pressable',
// The real DiffLine (mounted below) reads the color scheme for its palette.
useColorScheme: () => 'light',
}));
vi.mock('react-i18next', async importOriginal => {
const actual = await importOriginal<typeof ReactI18next>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,18 @@ import { describe, expect, it, vi } from 'vitest';

import '@/i18n';
import { DiffLine } from './diff-line';
import { MUTED_COLOR, tokenColorFor } from '@/lib/pr-review/diff/syntax-colors';
import { type ParsedDiffLine } from '@/lib/pr-review/diff/parse-patch';

const { useColorSchemeMock } = vi.hoisted(() => ({
useColorSchemeMock: vi.fn<() => 'dark' | 'light'>(() => 'light'),
}));

vi.mock('react-native', () => ({
Pressable: 'Pressable',
Text: 'RNText',
View: 'View',
useColorScheme: useColorSchemeMock,
}));
vi.mock('@/lib/hooks/use-theme-colors', () => ({
useThemeColors: () => ({
Expand Down Expand Up @@ -64,6 +70,41 @@ function findRow(renderer: TestRenderer.ReactTestRenderer): TestRenderer.ReactTe
return row;
}

/** The style.color of every painted run: gutter, marker, and code runs. */
function paintedColors(root: TestRenderer.ReactTestInstance): string[] {
return root
.findAll(node => {
const style = node.props.style as { color?: string } | undefined;
return typeof style?.color === 'string';
})
.map(node => (node.props.style as { color: string }).color);
}

describe('DiffLine syntax palette follows the color scheme', () => {
// The palette must come from the color scheme, not from comparing a theme
// token (e.g. `background`) to a hex literal: the generated palette is free
// to change and the tokens would silently flip against their surface.
it('paints the dark token and muted palette on a dark color scheme', () => {
useColorSchemeMock.mockReturnValue('dark');
const renderer = mountLine({ line: line(), language: 'typescript', keyId: 'line-12' });

const colors = paintedColors(renderer.root);
expect(colors).toContain(tokenColorFor('keyword', true));
expect(colors).toContain(MUTED_COLOR.dark);
expect(colors).not.toContain(tokenColorFor('keyword', false));
});

it('paints the light token and muted palette on a light color scheme', () => {
useColorSchemeMock.mockReturnValue('light');
const renderer = mountLine({ line: line(), language: 'typescript', keyId: 'line-12' });

const colors = paintedColors(renderer.root);
expect(colors).toContain(tokenColorFor('keyword', false));
expect(colors).toContain(MUTED_COLOR.light);
expect(colors).not.toContain(tokenColorFor('keyword', true));
});
});

describe('DiffLine gutter alignment', () => {
// The row is `flex-row items-stretch`, so the gutter View stretches to the
// row's full height. A long code line wraps and makes the row several
Expand Down
12 changes: 10 additions & 2 deletions apps/mobile/src/components/pr-review/diff/diff-line.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,14 @@

import { memo, useMemo } from 'react';
import { useTranslation } from 'react-i18next';
import { Pressable, Text as RNText, type TextStyle, View, type ViewStyle } from 'react-native';
import {
Pressable,
Text as RNText,
type TextStyle,
useColorScheme,
View,
type ViewStyle,
} from 'react-native';

import { useThemeColors } from '@/lib/hooks/use-theme-colors';
import { highlightLine, type HighlightToken } from '@/lib/pr-review/diff/highlight';
Expand Down Expand Up @@ -97,7 +104,8 @@ function markerColorFor(
function DiffLineImpl({ line, language, onTap, isSelected }: Readonly<DiffLineProps>) {
const colors = useThemeColors();
const { t } = useTranslation();
const isDark = colors.background === '#0E0E10';
// Same signal `useThemeColors` reads; never a background-token equality.
const isDark = useColorScheme() === 'dark';
const metrics = useDiffFontMetrics();

const tokens = useMemo<HighlightToken[]>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,18 @@ import { describe, expect, it, vi } from 'vitest';

import '@/i18n';
import { SideBySideRow } from './pr-diff-side-by-side-row';
import { MUTED_COLOR, tokenColorFor } from '@/lib/pr-review/diff/syntax-colors';
import { type ParsedDiffLine } from '@/lib/pr-review/diff/parse-patch';
import { type SideBySideRow as SideBySideRowData } from '@/lib/pr-review/diff/side-by-side';

const { useColorSchemeMock } = vi.hoisted(() => ({
useColorSchemeMock: vi.fn<() => 'dark' | 'light'>(() => 'light'),
}));

vi.mock('react-native', () => ({
Text: 'RNText',
View: 'View',
useColorScheme: useColorSchemeMock,
}));
vi.mock('@/components/ui/text', async () => {
const React = await import('react');
Expand Down Expand Up @@ -38,11 +44,14 @@ function row(overrides: Partial<ParsedDiffLine> = {}): SideBySideRowData {
return { left: { line: line(overrides) }, right: { line: line(overrides) } };
}

function mountRow(data: SideBySideRowData): TestRenderer.ReactTestRenderer {
function mountRow(
data: SideBySideRowData,
language: string | null = null
): TestRenderer.ReactTestRenderer {
const ref: { current: TestRenderer.ReactTestRenderer | null } = { current: null };
act(() => {
ref.current = TestRenderer.create(
createElement(SideBySideRow, { row: data, language: null, rowKeyId: 'row-7' })
createElement(SideBySideRow, { row: data, language, rowKeyId: 'row-7' })
);
});
const created = ref.current;
Expand All @@ -52,6 +61,40 @@ function mountRow(data: SideBySideRowData): TestRenderer.ReactTestRenderer {
return created;
}

/** The style.color of every painted run: gutter and code token runs. */
function paintedColors(root: TestRenderer.ReactTestInstance): string[] {
return root
.findAll(node => {
const style = node.props.style as { color?: string } | undefined;
return typeof style?.color === 'string';
})
.map(node => (node.props.style as { color: string }).color);
}

describe('SideBySideRow syntax palette follows the color scheme', () => {
// Same defect class as the unified DiffLine: the palette must come from the
// color scheme, never from comparing a theme token to a hex literal.
it('paints the dark token and muted palette on a dark color scheme', () => {
useColorSchemeMock.mockReturnValue('dark');
const renderer = mountRow(row(), 'typescript');

const colors = paintedColors(renderer.root);
expect(colors).toContain(tokenColorFor('keyword', true));
expect(colors).toContain(MUTED_COLOR.dark);
expect(colors).not.toContain(tokenColorFor('keyword', false));
});

it('paints the light token and muted palette on a light color scheme', () => {
useColorSchemeMock.mockReturnValue('light');
const renderer = mountRow(row(), 'typescript');

const colors = paintedColors(renderer.root);
expect(colors).toContain(tokenColorFor('keyword', false));
expect(colors).toContain(MUTED_COLOR.light);
expect(colors).not.toContain(tokenColorFor('keyword', true));
});
});

describe('SideBySideRow gutter alignment', () => {
// Same defect class as the unified DiffLine gutter: a wrapped code line
// makes the column several visual lines tall, and a centered number would
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

import { memo, useMemo } from 'react';
import { useTranslation } from 'react-i18next';
import { Text as RNText, type TextStyle, View, type ViewStyle } from 'react-native';
import { Text as RNText, type TextStyle, useColorScheme, View, type ViewStyle } from 'react-native';

import { type TFunction } from 'i18next';
import { Text } from '@/components/ui/text';
Expand Down Expand Up @@ -208,7 +208,8 @@ function SideBySideRowImpl({ row, language, rowKeyId }: Readonly<SideBySideRowPr
const colors = useThemeColors();
const { t } = useTranslation();
const metrics = useDiffFontMetrics();
const isDark = colors.background === '#0E0E10';
// Same signal `useThemeColors` reads; never a background-token equality.
const isDark = useColorScheme() === 'dark';
const leftLine = row.left?.line ?? null;
const rightLine = row.right?.line ?? null;
const rowStyle: ViewStyle = { minHeight: metrics.rowMinHeight };
Expand Down