Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
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
9 changes: 5 additions & 4 deletions apps/mobile/src/components/agents/session-filter-button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,11 @@ type SessionFilterButtonProps = {
* carries `shrink-0`, so the header row cannot squeeze it under the audit's
* 28dp floor.
*
* The sides are spelled out rather than a single uniform number: the row's
* sibling control (`session-list-header-actions.tsx`) caps its facing right
* slop against this control's left slop at the row's 14pt gap, and the test
* that guards that invariant reads the insets.
* The sides are spelled out rather than a single uniform number: the agents
* header row caps this control's two horizontal sides at its `gap-4` gap (the
* row mirrors under RTL while `hitSlop` does not, so the cap cannot sit on one
* physical side) and passes the capped insets in through the prop below. The
* test that guards that invariant reads the insets.
*/
const FILTER_HIT_SLOP = {
top: COMPACT_CONTROL_HIT_SLOP_DP,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,49 +229,65 @@ describe('SessionListHeaderActions new-session control', () => {
});
});

it('meets the filter control at the row gap instead of overlapping its touch region', async () => {
const renderer = await mountHeader(true, noop);
// React Native mirrors the row's `flex-row` order under RTL but does not
// mirror `hitSlop` (`screen-header.tsx:165` spells its own slop per direction
// for the same reason), so the gap-facing pair follows the physical order the
// direction gives the row: the new-session control's right with the filter's
// left in LTR, and its left with the filter's right under RTL.
it.each([false, true])(
'meets the filter control at the row gap instead of overlapping its touch region (RTL=%s)',
async isRTL => {
const renderer = await mountHeader(true, noop);

const newSession = pressesWithLabel(renderer.root, 'New session')[0];
const filter = pressesWithLabel(renderer.root, 'Filter sessions')[0];
if (!newSession || !filter) {
throw new Error('header controls not found');
}

const newSession = pressesWithLabel(renderer.root, 'New session')[0];
const filter = pressesWithLabel(renderer.root, 'Filter sessions')[0];
if (!newSession || !filter) {
throw new Error('header controls not found');
const row = renderer.root.find(
node =>
typeof node.type === 'string' &&
(node.type as string) === 'View' &&
typeof node.props.className === 'string' &&
node.props.className.split(/\s+/).includes('gap-4')
);
const gapDp = await compiledGapDp(row.props.className as string);
// NativeWind v5 fixes 1rem at 14pt, so the row's `gap-4` is 14pt, not 16pt.
expect(gapDp).toBe(14);

// `hitSlopInsets` validates and normalizes either shape; `slopSideDp`
// then reads the facing side: the header caps the filter's two horizontal
// sides, while the new-session control keeps the shared symmetric slop.
const newSessionSlop = hitSlopInsets(newSession.props.hitSlop);
const filterSlop = hitSlopInsets(filter.props.hitSlop);
// The gap has to fit the pair's two facing slops in either direction;
// more than the gap means the two touch regions overlap, and the later
// sibling (the filter) claims the taps inside the overlap. Either control
// may express hitSlop as one number or as per-side insets: `IconButton`
// and this control both pass a per-side object today.
const facingDp = isRTL
? slopSideDp(newSessionSlop, 'left') + slopSideDp(filterSlop, 'right')
: slopSideDp(newSessionSlop, 'right') + slopSideDp(filterSlop, 'left');
expect(facingDp).toBeLessThanOrEqual(gapDp);
// Capping the facing sides must not drop either control below the design
// target: the compact new-session box plus its symmetric slop, and the
// filter's own 36pt box plus the capped slop.
const box = boxDp(newSession.props.className as string);
expect(
box.width + slopSideDp(newSessionSlop, 'left') + slopSideDp(newSessionSlop, 'right')
).toBeGreaterThanOrEqual(TOUCH_TARGET_DP);
const filterBox = boxDp(filter.props.className as string);
expect(filterBox.width).toBeGreaterThanOrEqual(MIN_TAP_TARGET_DP);
expect(
filterBox.width + slopSideDp(filterSlop, 'left') + slopSideDp(filterSlop, 'right')
).toBeGreaterThanOrEqual(TOUCH_TARGET_DP);

act(() => {
renderer.unmount();
});
}

const row = renderer.root.find(
node =>
typeof node.type === 'string' &&
(node.type as string) === 'View' &&
typeof node.props.className === 'string' &&
node.props.className.split(/\s+/).includes('gap-4')
);
const gapDp = await compiledGapDp(row.props.className as string);
// NativeWind v5 fixes 1rem at 14pt, so the row's `gap-4` is 14pt, not 16pt.
expect(gapDp).toBe(14);

// `hitSlopInsets` (inside `slopSideDp`) validates and normalizes either
// shape of React Native's `number | Rect` hitSlop: a control may express one
// dp value for every side or spell out per-side insets. Both header controls
// spell out insets today — the filter writes four equal sides, while the
// new-session control caps its facing (right) side — so the helper has to
// accept either shape rather than assume one. The new-session control sits
// left of the filter, so the gap has to fit both facing slops; more than the
// gap means the two regions overlap.
expect(
slopSideDp(newSession.props.hitSlop, 'right') + slopSideDp(filter.props.hitSlop, 'left')
).toBeLessThanOrEqual(gapDp);
// Capping the right side must not drop the control below the design target.
const box = boxDp(newSession.props.className as string);
expect(
box.width +
slopSideDp(newSession.props.hitSlop, 'left') +
slopSideDp(newSession.props.hitSlop, 'right')
).toBeGreaterThanOrEqual(TOUCH_TARGET_DP);

act(() => {
renderer.unmount();
});
});
);

it('renders no new-session control when showNewSession is false', async () => {
const renderer = await mountHeader(false, noop);
Expand Down
36 changes: 15 additions & 21 deletions apps/mobile/src/components/agents/session-list-header-actions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,20 @@ import { COMPACT_CONTROL_HIT_SLOP_DP } from '@/lib/a11y/tap-target';
import { useThemeColors } from '@/lib/hooks/use-theme-colors';

// The row's `gap-4` compiles to 14pt, not 16pt: NativeWind v5 fixes 1rem at
// 14pt, so `gap-4` (1rem) is 14pt. The filter control's own left slop is 8pt
// (`@/lib/a11y/tap-target`), so the new-session control's right side is capped
// at 14 - 8 = 6 and the two facing slops meet at the row's gap without
// overlapping (6 + 8 = 14). 32 + 8 + 6 = 46pt still clears `DESIGN.md:364`'s
// 44pt. The filter's slop is spelled per side too, so that meeting can be
// checked instead of only the smallest of its four sides.
const NEW_SESSION_HIT_SLOP = {
top: COMPACT_CONTROL_HIT_SLOP_DP,
bottom: COMPACT_CONTROL_HIT_SLOP_DP,
left: COMPACT_CONTROL_HIT_SLOP_DP,
right: 6,
};

// 14pt. React Native mirrors the row's flex order under RTL but does not mirror
// `hitSlop` (`screen-header.tsx:165`), so a cap spelled on one physical side
// would meet the gap in one direction and overlap it in the other: the
// new-session control keeps the shared symmetric 8pt compact slop, and the
// wider filter frame absorbs the row's 2pt shortfall on both horizontal sides
// (14 - 8 = 6). Whichever way the row mirrors, the facing pair sums to exactly
// the 14pt gap, so the two touch regions meet at its boundary instead of one
// claiming the later sibling's taps inside an overlap.
// 36 + 6 + 6 = 48pt still clears `DESIGN.md:364`'s 44pt.
const FILTER_HIT_SLOP = {
Comment thread
iscekic marked this conversation as resolved.
top: COMPACT_CONTROL_HIT_SLOP_DP,
bottom: COMPACT_CONTROL_HIT_SLOP_DP,
left: COMPACT_CONTROL_HIT_SLOP_DP,
right: COMPACT_CONTROL_HIT_SLOP_DP,
left: 6,
right: 6,
};

type SessionListHeaderActionsProps = {
Expand All @@ -50,19 +46,17 @@ export function SessionListHeaderActions({
return (
<View className="flex-row items-center gap-4">
{showNewSession ? (
<IconButton
onPress={onNewSession}
accessibilityLabel={t('common.newSession')}
hitSlop={NEW_SESSION_HIT_SLOP}
>
// No `hitSlop` here: `IconButton`'s default is the symmetric 8pt
// compact slop, the pair's larger half (see `FILTER_HIT_SLOP` above).
<IconButton onPress={onNewSession} accessibilityLabel={t('common.newSession')}>
<Plus size={22} color={colors.foreground} />
</IconButton>
) : null}
<SessionFilterButton
activeCount={activeFilterCount}
hitSlop={FILTER_HIT_SLOP}
onPress={onOpenFilters}
testID="agents-open-filters"
hitSlop={FILTER_HIT_SLOP}
/>
</View>
);
Expand Down
22 changes: 13 additions & 9 deletions apps/mobile/src/components/home/section-header.mounted.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,10 @@ describe('SectionHeader mounted layout', () => {
expect(label.props.maxFontSizeMultiplier).toBeUndefined();
expect(label.props.adjustsFontSizeToFit).not.toBe(true);
expect(label.children).toEqual(['Live now']);
// The tracked class stays for the Latin design; the RTL letter-spacing
// reset applies to RTL-script copy only, so this Latin label keeps its
// tracking (see lib/rtl-text.ts and text.rtl-labels.mounted.test.tsx).
// The tracked class stays for the Latin design; the reset lands on a joined
// script in either direction and on RTL-script copy inside an RTL
// interface, so this Latin label keeps its tracking (see lib/rtl-text.ts and
// text.rtl-labels.mounted.test.tsx).
if (isRTL) {
expect(label.props.style).toContainEqual({ writingDirection: 'rtl' });
expect(label.props.style).not.toContainEqual({ letterSpacing: 0 });
Expand Down Expand Up @@ -116,10 +117,12 @@ describe('SectionHeader mounted layout', () => {
expect(text.children).toEqual(['See all']);
});

// The letter-spacing reset belongs to an RTL interface: in this LTR screen
// the eyebrow label and the action keep `tracking-[1.5px]`, and no inline
// style overrides the class (the RTL case below pins the reset).
it('keeps the tracked letter-spacing on the Arabic label and action in LTR', () => {
// The letter-spacing reset belongs to the script: a joined script takes it in
// either direction, so this LTR screen keeps the tracked class on the element
// and the inline reset draws it inert. The mono family goes with it, because
// JetBrains Mono ships no Arabic glyph (the RTL case below pins the same
// treatment).
it('keeps the tracked class and resets the joined-script label and action in LTR', () => {
const root = mount(
createElement(SectionHeader, {
label: 'الجلسات الجارية الآن',
Expand All @@ -134,8 +137,9 @@ describe('SectionHeader mounted layout', () => {
const actionText = action.find(node => Object.is(node.type, 'Text'));

expect((label.props.className as string).split(' ')).toContain('tracking-[1.5px]');
expect(label.props.style).toBeUndefined();
expect(actionText.props.style).toBeUndefined();
expect(label.props.style).toContainEqual({ letterSpacing: 0 });
expect(actionText.props.style).toContainEqual({ letterSpacing: 0 });
expect((actionText.props.className as string).split(' ')).not.toContain('font-mono-medium');
});

it('renders Arabic labels without the mono family or letter spacing in RTL', () => {
Expand Down
75 changes: 34 additions & 41 deletions apps/mobile/src/components/profile-screen.signout.mounted.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const insets = vi.hoisted(() => ({ top: 0, bottom: 0, left: 0, right: 0 }));

vi.mock('react-native', () => ({
Alert: { alert: alertFn },
Modal: 'Modal',
Platform: {
get OS() {
return platform.os;
Expand Down Expand Up @@ -155,11 +156,6 @@ function isType(node: TestRenderer.ReactTestInstance, type: string): boolean {
return typeof node.type === 'string' && node.type === type;
}

function alertButtons(call: unknown): { style?: string; onPress?: () => void }[] {
const buttons = (call as unknown[] | undefined)?.[2];
return Array.isArray(buttons) ? (buttons as { style?: string; onPress?: () => void }[]) : [];
}

describe('ProfileScreen sign-out confirmation', () => {
beforeEach(() => {
signOutFn.mockReset();
Expand All @@ -175,49 +171,46 @@ describe('ProfileScreen sign-out confirmation', () => {
});
}

// The confirmation is the one shared native alert on both platforms:
// plugins/withAndroidAlertDialogTheme repaints Android's AppCompat dialog with
// the app tokens, and iOS's `UIAlertController` already follows the device
// appearance. No platform mounts an in-app confirmation of its own, so the
// same tap takes the same path whichever value the device reports.
for (const os of ['android', 'ios'] as const) {
it(`opens the shared native alert on ${os}`, async () => {
function dialogButton(
renderer: TestRenderer.ReactTestRenderer,
variant: 'outline' | 'destructive'
) {
return renderer.root.find(node => isType(node, 'Button') && node.props.variant === variant);
}

// The request names no platform, so one destructive confirm serves both: the
// in-app dialog renders on iOS and Android alike, and its sign-out control
// carries the destructive (red) variant. The native alert cannot be the
// shared implementation — Android's `AlertDialog` paints every button with
// the theme accent, ignoring `style: 'destructive'`.
it.each(['android', 'ios'] as const)(
'opens the in-app dialog whose destructive control signs out on %s',
async os => {
platform.os = os;
const { renderer, unmount } = await renderWithProviders(createElement(ProfileScreen));

pressSignOutTile(renderer);

expect(alertFn).toHaveBeenCalledTimes(1);
expect(alertFn.mock.calls[0]?.[0]).toBe('Sign out?');
// Opening the confirmation signs nobody out; only its own choices act.
// One implementation for both platforms: the confirmation is the in-app
// dialog everywhere; no native alert replaces it on either side.
expect(alertFn).not.toHaveBeenCalled();
// Opening the confirmation signs nobody out.
expect(signOutFn).not.toHaveBeenCalled();
const buttons = alertButtons(alertFn.mock.calls[0]);
expect(buttons.find(button => button.style === 'cancel')?.onPress).toBeUndefined();
expect(buttons.some(button => button.style === 'destructive')).toBe(true);
// The alert is the whole confirmation: no in-app dialog renders beside it.
expect(
renderer.root.findAll(
node => isType(node, 'Button') && node.props.variant === 'destructive'
)
).toHaveLength(0);

unmount();
});
}

it("signs out only when the alert's destructive choice is pressed", async () => {
const { renderer, unmount } = await renderWithProviders(createElement(ProfileScreen));

pressSignOutTile(renderer);
// Cancel keeps the user signed in.
act(() => {
(dialogButton(renderer, 'outline').props as { onPress?: () => void }).onPress?.();
});
expect(signOutFn).not.toHaveBeenCalled();

const destructive = alertButtons(alertFn.mock.calls[0]).find(
button => button.style === 'destructive'
);
act(() => {
destructive?.onPress?.();
});
expect(signOutFn).toHaveBeenCalledTimes(1);
// Reopen: only the destructive control signs out.
pressSignOutTile(renderer);
act(() => {
(dialogButton(renderer, 'destructive').props as { onPress?: () => void }).onPress?.();
});
expect(signOutFn).toHaveBeenCalledTimes(1);

unmount();
});
unmount();
}
);
});
Loading
Loading