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
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,27 @@ describe('SessionFilterModal', () => {
);
});

it('omits an empty platform section instead of rendering an orphaned label', async () => {
// A live list whose rows all have an unknown origin offers no platform
// option (`buildLiveFilterOptions` only offers the origins that are
// running), so the section must be omitted exactly as PROJECT already is
// when it has no rows — otherwise the sheet shows a bare PLATFORM label
// with nothing under it (spot defect e4-filter).
const { renderer } = await renderModal({
platformOptions: [],
selectedPlatforms: [],
projectOptions: [firstProject],
});
const labels = renderer.root.findAllByType(Text).map(text => text.props.children);
expect(labels).not.toContain(i18n.t('common.platform'));
expect(labels).toContain(i18n.t('agentChat.sessionFilter.project'));
expect(
renderer.root
.findAllByProps({ accessibilityRole: 'checkbox' })
.map(row => row.findByType(Text).props.children)
).toEqual([firstProject.displayName]);
});

it('commits both draft arrays only on Apply and preserves unavailable selections', async () => {
const selectedPlatforms = ['cli', 'future-platform'];
const selectedProjects = [firstProject.gitUrl, unavailable];
Expand Down
38 changes: 23 additions & 15 deletions apps/mobile/src/components/agents/platform-filter-modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -212,21 +212,29 @@ export function SessionFilterModal({
</Text>
<ScrollView className="shrink" showsVerticalScrollIndicator={false}>
<View className="gap-4">
<View className="gap-1">
<Text variant="eyebrow" className="px-3">
{t('common.platform')}
</Text>
{platforms.map(platform => (
<FilterCheckboxRow
key={platform}
label={platformFilterLabel(platform)}
isChecked={draftPlatforms.includes(platform)}
onPress={() => {
togglePlatform(platform);
}}
/>
))}
</View>
{/* The rows are derived from the live sessions, so a list whose
rows all have an unknown origin (`liveSessionPlatformBucket`
returns null) offers no platform at all and nothing is
selected. Rendering the label then leaves an orphaned section
header above PROJECT (spot defect e4-filter); the section is
omitted exactly as PROJECT already is when it has no rows. */}
{platforms.length > 0 && (
<View className="gap-1">
<Text variant="eyebrow" className="px-3">
{t('common.platform')}
</Text>
{platforms.map(platform => (
<FilterCheckboxRow
key={platform}
label={platformFilterLabel(platform)}
isChecked={draftPlatforms.includes(platform)}
onPress={() => {
togglePlatform(platform);
}}
/>
))}
</View>
)}
{projectGroups.size > 0 && (
<View className="gap-1">
<Text variant="eyebrow" className="px-3">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { describe, expect, it, vi } from 'vitest';

import { CommentRow } from './comment-row';
import { CommentModerationProvider } from './comment-moderation';
import { MIN_AUDITED_CONTROL_FRAME_DP } from '@/lib/a11y/touch-target';
import { MIN_TAP_TARGET_DP } from '@/lib/a11y/tap-target';
import {
COMMENT_ACTIONS_FRAME_DP,
COMMENT_ACTIONS_HIT_SLOP,
Expand Down Expand Up @@ -121,7 +121,7 @@ describe('CommentRow overflow touch target', () => {
expect(overflow.props.className).toContain('items-center');
expect(overflow.props.className).toContain('justify-center');
expect(overflow.props.hitSlop).toEqual(COMMENT_ACTIONS_HIT_SLOP);
expect(COMMENT_ACTIONS_FRAME_DP).toBeGreaterThanOrEqual(MIN_AUDITED_CONTROL_FRAME_DP);
expect(COMMENT_ACTIONS_FRAME_DP).toBeGreaterThanOrEqual(MIN_TAP_TARGET_DP);
expect(
COMMENT_ACTIONS_FRAME_DP + COMMENT_ACTIONS_HIT_SLOP.top + COMMENT_ACTIONS_HIT_SLOP.bottom
).toBeGreaterThanOrEqual(44);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { act, TestRenderer } from '@/test/renderer';
import { describe, expect, it, vi } from 'vitest';

import { DiscussionThread } from './discussion-thread';
import { COMPACT_CONTROL_FRAME_DP, COMPACT_CONTROL_HIT_SLOP_DP } from '@/lib/a11y/touch-target';
import { COMPACT_H11_FRAME_DP, COMPACT_H11_HIT_SLOP_DP } from '@/lib/a11y/tap-target';
import { type ReviewThread } from '@/lib/pr-review/discussion/review-discussion-types';

vi.mock('react-native', () => ({
Expand Down Expand Up @@ -102,8 +102,8 @@ describe('DiscussionThread resolve toggle touch target', () => {
expect(resolveButton.props.className).toContain('h-11 w-11');
expect(resolveButton.props.className).toContain('items-center');
expect(resolveButton.props.className).toContain('justify-center');
expect(resolveButton.props.hitSlop).toBe(COMPACT_CONTROL_HIT_SLOP_DP);
expect(COMPACT_CONTROL_FRAME_DP + 2 * COMPACT_CONTROL_HIT_SLOP_DP).toBeGreaterThanOrEqual(44);
expect(resolveButton.props.hitSlop).toBe(COMPACT_H11_HIT_SLOP_DP);
expect(COMPACT_H11_FRAME_DP + 2 * COMPACT_H11_HIT_SLOP_DP).toBeGreaterThanOrEqual(44);

// The visible circle keeps its compact 28pt size inside the frame.
const circle = resolveButton.find(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import { ReplyInput } from '@/components/pr-review/discussion/reply-input';
import { ThreadDiffSnippet } from '@/components/pr-review/discussion/thread-diff-snippet';
import { Text } from '@/components/ui/text';
import { i18n } from '@/i18n';
import { COMPACT_CONTROL_HIT_SLOP_DP } from '@/lib/a11y/touch-target';
import { COMPACT_H11_HIT_SLOP_DP } from '@/lib/a11y/tap-target';
import { formatNumber } from '@/lib/format';
import {
type ReviewComment,
Expand Down Expand Up @@ -359,7 +359,7 @@ function ResolveToggle({ resolved, disabled, onPress }: Readonly<ResolveTogglePr
// The frame is the tap target the size audit measures (38.5pt on
// device) and the header row grows to hold it, so the whole frame is
// hittable; the 3pt slop reaches the 44pt minimum.
hitSlop={COMPACT_CONTROL_HIT_SLOP_DP}
hitSlop={COMPACT_H11_HIT_SLOP_DP}
className="h-11 w-11 items-center justify-center active:opacity-70"
>
{/* The visible circle stays compact (explicit px, because NativeWind's
Expand Down
14 changes: 14 additions & 0 deletions apps/mobile/src/lib/a11y/tap-target.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import type * as NativeCSSCompiler from 'react-native-css/compiler';
import {
COMPACT_CONTROL_BOX_CLASS,
COMPACT_CONTROL_HIT_SLOP_DP,
COMPACT_H11_FRAME_DP,
COMPACT_H11_HIT_SLOP_DP,
compactControlTargetDp,
INLINE_LINK_BOX_CLASS,
INLINE_LINK_CONNECTOR_CLASS,
INLINE_LINK_FACING_HIT_SLOP_DP,
Expand Down Expand Up @@ -83,6 +86,17 @@ describe('shared tap-target geometry', () => {
);
});

it('keeps the measured h-11 frame at or above the audit floor and reaches the design target', () => {
// The regression this guards: a control sized to its glyph (or to a
// rem-scaled `h-7`, which measures 24.5dp) is reported as too small to tap.
expect(COMPACT_H11_FRAME_DP).toBeGreaterThanOrEqual(MIN_TAP_TARGET_DP);
expect(COMPACT_H11_HIT_SLOP_DP).toBeGreaterThan(0);
expect(compactControlTargetDp()).toBe(
tapTargetReachDp(COMPACT_H11_FRAME_DP, COMPACT_H11_HIT_SLOP_DP)
);
expect(compactControlTargetDp()).toBeGreaterThanOrEqual(TOUCH_TARGET_DP);
});

it('carries an inline link from the audit floor to the design target with its slop', () => {
expect(INLINE_LINK_BOX_CLASS).toContain('min-h-[28px]');
expect(INLINE_LINK_BOX_CLASS).toContain('min-w-[28px]');
Expand Down
26 changes: 26 additions & 0 deletions apps/mobile/src/lib/a11y/tap-target.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,32 @@ export const COMPACT_CONTROL_BOX_CLASS = 'h-[32px] w-[32px] items-center justify
*/
export const COMPACT_CONTROL_HIT_SLOP_DP = 8;

/**
* The frame a compact `h-11` control renders, in pt. NativeWind's rem is 14pt
* here, so `h-11` (2.75rem) measures 38.5pt on device: above the
* {@link MIN_TAP_TARGET_DP} floor a control-size audit accepts, and
* {@link COMPACT_H11_HIT_SLOP_DP} per side carries it to
* {@link TOUCH_TARGET_DP}. This is the second compact geometry beside
* {@link COMPACT_CONTROL_BOX_CLASS}: an `h-11` frame is set by its
* surroundings' layout (`discussion-thread.tsx`, `comment-row.tsx`), not by the
* icon-button box that 32pt defines. The frames stay literal class strings in
* the components (NativeWind reads them at build time); these numbers keep the
* arithmetic and its checks in one place. `comment-trailing-controls.ts` holds
* the one control whose neighbour bounds its tap area.
*/
export const COMPACT_H11_FRAME_DP = 38.5;

/**
* Per-side `hitSlop` that lifts {@link COMPACT_H11_FRAME_DP} to
* `DESIGN.md:364`'s 44pt minimum: 38.5 + 3 + 3 = 44.5pt.
*/
export const COMPACT_H11_HIT_SLOP_DP = 3;

/** Width and height of an `h-11` compact control's tap area, in pt. */
export function compactControlTargetDp(): number {
return tapTargetReachDp(COMPACT_H11_FRAME_DP, COMPACT_H11_HIT_SLOP_DP);
}

/**
* The box an inline text link renders. The sign-in legal links sit inside a
* sentence, so their box is the control-size audit's {@link MIN_TAP_TARGET_DP}
Expand Down
21 changes: 0 additions & 21 deletions apps/mobile/src/lib/a11y/touch-target.test.ts

This file was deleted.

28 changes: 0 additions & 28 deletions apps/mobile/src/lib/a11y/touch-target.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, expect, it } from 'vitest';
import { describe, expect, it, vi } from 'vitest';

import { MIN_AUDITED_CONTROL_FRAME_DP } from '@/lib/a11y/touch-target';
import { MIN_TAP_TARGET_DP } from '@/lib/a11y/tap-target';
import {
COMMENT_ACTIONS_FRAME_DP,
COMMENT_ACTIONS_HIT_SLOP,
Expand All @@ -11,6 +11,8 @@ import {
FIX_WITH_KILO_VISUAL_DP,
} from '@/lib/pr-review/comment-trailing-controls';

vi.mock('react-native', () => ({ I18nManager: { isRTL: false } }));

describe('comment trailing controls tap areas', () => {
it('keeps the pill and the overflow hit areas apart', () => {
// The regression this guards: the overflow's 8pt left hitleed used to
Expand All @@ -22,7 +24,7 @@ describe('comment trailing controls tap areas', () => {
it('keeps the overflow frame at or above the 28dp the size audit measures', () => {
// h-7 measured 24.5dp on device (NativeWind's rem is 14pt) and was
// reported as too small to tap.
expect(COMMENT_ACTIONS_FRAME_DP).toBeGreaterThanOrEqual(MIN_AUDITED_CONTROL_FRAME_DP);
expect(COMMENT_ACTIONS_FRAME_DP).toBeGreaterThanOrEqual(MIN_TAP_TARGET_DP);
expect(COMMENT_ACTIONS_VISUAL_DP).toBeLessThan(COMMENT_ACTIONS_FRAME_DP);
});

Expand Down
12 changes: 6 additions & 6 deletions apps/mobile/src/lib/pr-review/comment-trailing-controls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// session, never the moderation sheet. Keeping the arithmetic in one place lets
// the row's gap and the two tap areas be checked together (vr1, 2026-09-16).

import { COMPACT_CONTROL_FRAME_DP, COMPACT_CONTROL_HIT_SLOP_DP } from '@/lib/a11y/touch-target';
import { COMPACT_H11_FRAME_DP, COMPACT_H11_HIT_SLOP_DP } from '@/lib/a11y/tap-target';

/**
* The pill's visual height in pt, read off its classes in
Expand All @@ -33,17 +33,17 @@ export const FIX_WITH_KILO_VISUAL_DP = 23;
export const FIX_WITH_KILO_HIT_SLOP = { top: 11, bottom: 11, left: 2, right: 2 } as const;

/** The overflow's frame: `h-11 w-11` measured on device. */
export const COMMENT_ACTIONS_FRAME_DP = COMPACT_CONTROL_FRAME_DP;
export const COMMENT_ACTIONS_FRAME_DP = COMPACT_H11_FRAME_DP;

/** The overflow's visible circle, in explicit dp so rem cannot shrink it. */
export const COMMENT_ACTIONS_VISUAL_DP = 28;

/** The overflow's hit slop, which lifts its frame to the 44pt minimum. */
export const COMMENT_ACTIONS_HIT_SLOP = {
top: COMPACT_CONTROL_HIT_SLOP_DP,
bottom: COMPACT_CONTROL_HIT_SLOP_DP,
left: COMPACT_CONTROL_HIT_SLOP_DP,
right: COMPACT_CONTROL_HIT_SLOP_DP,
top: COMPACT_H11_HIT_SLOP_DP,
bottom: COMPACT_H11_HIT_SLOP_DP,
left: COMPACT_H11_HIT_SLOP_DP,
right: COMPACT_H11_HIT_SLOP_DP,
} as const;

/** The trailing group's `gap-3` class, in dp: 0.75rem at NativeWind's 14pt rem. */
Expand Down
Loading