From 61419094f9a7cdbf6509cc5d83a436fddcbfe59e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Sun, 1 Nov 2026 09:02:04 +0000 Subject: [PATCH 1/5] fix(mobile): drop repeated section headers from personal Security Agent notifications The personal notification settings screen put a section heading over each of its two switches: "New-finding notification" above "Email me about new findings" and "SLA warning notification" above "Email me about SLA warnings". Each heading restated the only row under it. Render both headings only for the organization scope, where the two switches share the title "Email organization owners" and the heading is the only thing that tells them apart, including in the composed accessible name. Personal rows name themselves; grouping and spacing are unchanged. --- .../notification-settings-screen.tsx | 16 +- ...ication-settings-sections.mounted.test.tsx | 156 ++++++++++++++++++ 2 files changed, 166 insertions(+), 6 deletions(-) create mode 100644 apps/mobile/src/components/security-agent/notification-settings-sections.mounted.test.tsx diff --git a/apps/mobile/src/components/security-agent/notification-settings-screen.tsx b/apps/mobile/src/components/security-agent/notification-settings-screen.tsx index 3777c6fa235..ada7bee3b2c 100644 --- a/apps/mobile/src/components/security-agent/notification-settings-screen.tsx +++ b/apps/mobile/src/components/security-agent/notification-settings-screen.tsx @@ -211,9 +211,11 @@ export function NotificationSettingsScreen({ scope }: Readonly<{ scope: string } )} - - {t('securityAgent.notifications.newFindingSection')} - + {!personal && ( + + {t('securityAgent.notifications.newFindingSection')} + + )} - - {t('securityAgent.notifications.slaSection')} - + {!personal && ( + + {t('securityAgent.notifications.slaSection')} + + )} ({ randomUUID: () => '00000000-0000-4000-8000-000000000000' })); +vi.mock('expo-haptics', () => ({ selectionAsync: vi.fn() })); +vi.mock('react-native', () => ({ + View: 'View', + ScrollView: 'ScrollView', + TextInput: 'TextInput', + Pressable: 'Pressable', + Switch: 'Switch', + Platform: { OS: 'android' }, + useColorScheme: () => 'light', + Alert: { alert: vi.fn() }, + I18nManager: { isRTL: false }, +})); +vi.mock('expo-router', () => ({ + useRouter: () => ({ back: vi.fn(), replace: vi.fn(), push: vi.fn() }), + useNavigation: () => ({ dispatch: vi.fn(), goBack: vi.fn() }), + usePreventRemove: () => undefined, +})); +vi.mock('sonner-native', () => ({ toast: { success: vi.fn(), error: vi.fn() } })); +vi.mock('@/components/ui/icons', () => ({ Check: 'Check' })); +vi.mock('@/lib/hooks/use-theme-colors', () => ({ + useThemeColors: () => ({ + mutedForeground: '#000', + primaryForeground: '#fff', + primary: '#00f', + }), +})); +vi.mock('@/lib/hooks/use-security-agent', () => ({ + useSecurityAgentCapability: () => ({ + canManage: true, + status: 'allowed', + isLoading: false, + isFetching: false, + isError: false, + refetch: vi.fn(), + }), + useSecurityAgentConfig: () => ({ + data: { + isEnabled: true, + newFindingNotificationsEnabled: true, + newFindingNotificationMinSeverity: 'high', + slaNotificationsEnabled: true, + slaNotificationMinSeverity: 'medium', + slaNotificationWarningDays: 7, + }, + isLoading: false, + isError: false, + refetch: vi.fn(), + }), + useSaveSecurityAgentConfig: () => ({ mutateAsync: vi.fn(), isPending: false }), + useTrackSecurityAgentInteraction: () => ({ mutate: vi.fn() }), +})); +vi.mock('@rn-primitives/dialog', () => ({ + Action: 'AlertDialog.Action', + Cancel: 'AlertDialog.Cancel', + Content: 'AlertDialog.Content', + Description: 'AlertDialog.Description', + Overlay: 'AlertDialog.Overlay', + Portal: 'AlertDialog.Portal', + Root: 'AlertDialog.Root', + Title: 'AlertDialog.Title', +})); +vi.mock('@/components/destructive-confirm-dialog', () => ({ + DestructiveConfirmDialog: () => null, +})); +vi.mock('@/components/ui/input', () => ({ Input: 'Input' })); +vi.mock('@/components/ui/skeleton', () => ({ Skeleton: () => null })); +vi.mock('@/components/ui/text', () => ({ Text: 'Text' })); +vi.mock('@/components/ui/radio-group', () => ({ + RadioGroup: (props: { children?: unknown }) => props.children ?? null, + radioItemA11y: ({ + label, + checked, + disabled, + }: { + label: string; + checked: boolean; + disabled: boolean; + }) => ({ + accessibilityRole: 'radio', + accessibilityLabel: label, + accessibilityState: { checked, disabled, busy: false }, + }), +})); +vi.mock('@/components/screen-header', () => ({ ScreenHeader: () => null })); +vi.mock('@/components/platform-error-screen', () => ({ + PlatformErrorScreen: 'PlatformErrorScreen', +})); +vi.mock('@/components/security-agent/settings-save-button', () => ({ + SettingsSaveButton: () => null, +})); +vi.mock('@/components/tab-screen', () => ({ + TabScreenScrollView: (props: { children?: unknown }) => props.children ?? null, +})); + +const PERSONAL_SCOPE = 'personal'; +const ORGANIZATION_SCOPE = '00000000-0000-4000-8000-000000000001'; + +let renderer: TestRenderer.ReactTestRenderer | undefined = undefined; + +afterEach(() => { + act(() => { + renderer?.unmount(); + }); + renderer = undefined; +}); + +function mount(scope: string): TestRenderer.ReactTestRenderer { + act(() => { + renderer = TestRenderer.create(createElement(NotificationSettingsScreen, { scope })); + }); + if (!renderer) { + throw new Error('renderer was not created'); + } + return renderer; +} + +describe('personal scope', () => { + it('titles each switch after its notification and shows no section heading', () => { + const copy = mount(PERSONAL_SCOPE) + .root.findAllByType('Text') + .map(node => node.props.children); + + expect(copy).toContain(i18n.t('securityAgent.notifications.newFindingPersonalTitle')); + expect(copy).toContain(i18n.t('securityAgent.notifications.slaPersonalTitle')); + expect(copy).not.toContain(i18n.t('securityAgent.notifications.newFindingSection')); + expect(copy).not.toContain(i18n.t('securityAgent.notifications.slaSection')); + }); +}); + +describe('organization scope', () => { + it('heads each switch with its notification section, because both read the same recipient', () => { + const names = mount(ORGANIZATION_SCOPE) + .root.findAllByType('Switch') + .map(node => node.props.accessibilityLabel); + + expect(names).toHaveLength(2); + expect(names[0]).toContain(i18n.t('securityAgent.notifications.newFindingSection')); + expect(names[1]).toContain(i18n.t('securityAgent.notifications.slaSection')); + expect(names[1]).not.toContain(i18n.t('securityAgent.notifications.newFindingSection')); + }); +}); From bbb621b888acfa14e73cab91e7085387654fe44a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Sun, 1 Nov 2026 11:50:33 +0000 Subject: [PATCH 2/5] fix(mobile): space the personal notification groups apart without their headings With the single-row headings gone, the personal scope's two switch groups were separated only by the standard 21dp container gap, while the gaps inside a group are 7dp, so the two notifications read as one list. The personal scope now uses a 42dp container gap, close to the separation the organization scope gets from its headings, which stay there. The permission note that sits above the groups never renders in the personal scope (it can always manage), so its spacing is unchanged. --- .../notification-settings-screen.tsx | 7 ++++++- ...ication-settings-sections.mounted.test.tsx | 20 ++++++++++++++++--- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/apps/mobile/src/components/security-agent/notification-settings-screen.tsx b/apps/mobile/src/components/security-agent/notification-settings-screen.tsx index ada7bee3b2c..7fa59216b8d 100644 --- a/apps/mobile/src/components/security-agent/notification-settings-screen.tsx +++ b/apps/mobile/src/components/security-agent/notification-settings-screen.tsx @@ -203,7 +203,12 @@ export function NotificationSettingsScreen({ scope }: Readonly<{ scope: string } // iOS IME inset and focused-field auto-reveal (AGENTS.md: input screens). automaticallyAdjustKeyboardInsets className="flex-1" - contentContainerClassName="px-6 gap-6 pt-4" + // The personal scope has no section heading above its two switches, so + // the gap between the groups carries the break there. The organization + // scope keeps the standard gap, next to its headings. The permission + // note above the groups only renders for a scope that cannot manage, + // which the personal scope never is. + contentContainerClassName={cn('px-6 pt-4', personal ? 'gap-12' : 'gap-6')} > {!canManage && ( diff --git a/apps/mobile/src/components/security-agent/notification-settings-sections.mounted.test.tsx b/apps/mobile/src/components/security-agent/notification-settings-sections.mounted.test.tsx index e2028c6adfa..5b14d5821f2 100644 --- a/apps/mobile/src/components/security-agent/notification-settings-sections.mounted.test.tsx +++ b/apps/mobile/src/components/security-agent/notification-settings-sections.mounted.test.tsx @@ -103,9 +103,7 @@ vi.mock('@/components/platform-error-screen', () => ({ vi.mock('@/components/security-agent/settings-save-button', () => ({ SettingsSaveButton: () => null, })); -vi.mock('@/components/tab-screen', () => ({ - TabScreenScrollView: (props: { children?: unknown }) => props.children ?? null, -})); +vi.mock('@/components/tab-screen', () => ({ TabScreenScrollView: 'TabScreenScrollView' })); const PERSONAL_SCOPE = 'personal'; const ORGANIZATION_SCOPE = '00000000-0000-4000-8000-000000000001'; @@ -140,6 +138,14 @@ describe('personal scope', () => { expect(copy).not.toContain(i18n.t('securityAgent.notifications.newFindingSection')); expect(copy).not.toContain(i18n.t('securityAgent.notifications.slaSection')); }); + + it('spaces the two switch groups wider, because no heading separates them here', () => { + const screen = mount(PERSONAL_SCOPE); + + expect(screen.root.findByType('TabScreenScrollView').props.contentContainerClassName).toBe( + 'px-6 pt-4 gap-12' + ); + }); }); describe('organization scope', () => { @@ -153,4 +159,12 @@ describe('organization scope', () => { expect(names[1]).toContain(i18n.t('securityAgent.notifications.slaSection')); expect(names[1]).not.toContain(i18n.t('securityAgent.notifications.newFindingSection')); }); + + it('keeps the standard group gap, next to its headings', () => { + const screen = mount(ORGANIZATION_SCOPE); + + expect(screen.root.findByType('TabScreenScrollView').props.contentContainerClassName).toBe( + 'px-6 pt-4 gap-6' + ); + }); }); From 672d755fb843fcc15600b243e851ce3b805035fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Sun, 1 Nov 2026 13:23:29 +0000 Subject: [PATCH 3/5] fix(mobile): space the personal notification groups like the space above them With the section headings gone from the personal scope, a wider gap between its two switch groups left the space above the first group at 14dp and the space between the groups at 42dp, so the larger break read as a heading that had gone missing. The personal scope now uses the same 14dp gap between the groups; the organization scope keeps its standard 21dp gap next to its headings. --- .../security-agent/notification-settings-screen.tsx | 13 +++++++------ .../notification-settings-sections.mounted.test.tsx | 4 ++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/apps/mobile/src/components/security-agent/notification-settings-screen.tsx b/apps/mobile/src/components/security-agent/notification-settings-screen.tsx index 7fa59216b8d..26696207259 100644 --- a/apps/mobile/src/components/security-agent/notification-settings-screen.tsx +++ b/apps/mobile/src/components/security-agent/notification-settings-screen.tsx @@ -203,12 +203,13 @@ export function NotificationSettingsScreen({ scope }: Readonly<{ scope: string } // iOS IME inset and focused-field auto-reveal (AGENTS.md: input screens). automaticallyAdjustKeyboardInsets className="flex-1" - // The personal scope has no section heading above its two switches, so - // the gap between the groups carries the break there. The organization - // scope keeps the standard gap, next to its headings. The permission - // note above the groups only renders for a scope that cannot manage, - // which the personal scope never is. - contentContainerClassName={cn('px-6 pt-4', personal ? 'gap-12' : 'gap-6')} + // The personal scope has no section heading between its two switches, so + // the gap between the groups has to match the space above the first one, + // or the larger break reads as a heading that went missing. The + // organization scope keeps the standard gap, next to its headings. The + // permission note above the groups only renders for a scope that cannot + // manage, which the personal scope never is. + contentContainerClassName={cn('px-6 pt-4', personal ? 'gap-4' : 'gap-6')} > {!canManage && ( diff --git a/apps/mobile/src/components/security-agent/notification-settings-sections.mounted.test.tsx b/apps/mobile/src/components/security-agent/notification-settings-sections.mounted.test.tsx index 5b14d5821f2..f4d675c74e5 100644 --- a/apps/mobile/src/components/security-agent/notification-settings-sections.mounted.test.tsx +++ b/apps/mobile/src/components/security-agent/notification-settings-sections.mounted.test.tsx @@ -139,11 +139,11 @@ describe('personal scope', () => { expect(copy).not.toContain(i18n.t('securityAgent.notifications.slaSection')); }); - it('spaces the two switch groups wider, because no heading separates them here', () => { + it('spaces the two switch groups like the space above the first one', () => { const screen = mount(PERSONAL_SCOPE); expect(screen.root.findByType('TabScreenScrollView').props.contentContainerClassName).toBe( - 'px-6 pt-4 gap-12' + 'px-6 pt-4 gap-4' ); }); }); From 58c23747fc0a050baa9061503f776a79b6535936 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Sun, 1 Nov 2026 15:37:11 +0000 Subject: [PATCH 4/5] fix(mobile): separate the personal notification groups with a wider break --- .../security-agent/notification-settings-screen.tsx | 13 +++++++------ .../notification-settings-sections.mounted.test.tsx | 4 ++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/apps/mobile/src/components/security-agent/notification-settings-screen.tsx b/apps/mobile/src/components/security-agent/notification-settings-screen.tsx index 26696207259..d5c9fdbe4ea 100644 --- a/apps/mobile/src/components/security-agent/notification-settings-screen.tsx +++ b/apps/mobile/src/components/security-agent/notification-settings-screen.tsx @@ -204,12 +204,13 @@ export function NotificationSettingsScreen({ scope }: Readonly<{ scope: string } automaticallyAdjustKeyboardInsets className="flex-1" // The personal scope has no section heading between its two switches, so - // the gap between the groups has to match the space above the first one, - // or the larger break reads as a heading that went missing. The - // organization scope keeps the standard gap, next to its headings. The - // permission note above the groups only renders for a scope that cannot - // manage, which the personal scope never is. - contentContainerClassName={cn('px-6 pt-4', personal ? 'gap-4' : 'gap-6')} + // the break between the two groups carries their separation: it has to + // read as a group break, not as the 7dp and 5.25dp steps inside a group, + // and not as the void a removed heading would leave. The organization + // scope keeps the standard gap, next to its headings. The permission + // note above the groups only renders for a scope that cannot manage, + // which the personal scope never is. + contentContainerClassName={cn('px-6 pt-4', personal ? 'gap-7' : 'gap-6')} > {!canManage && ( diff --git a/apps/mobile/src/components/security-agent/notification-settings-sections.mounted.test.tsx b/apps/mobile/src/components/security-agent/notification-settings-sections.mounted.test.tsx index f4d675c74e5..29b9927018e 100644 --- a/apps/mobile/src/components/security-agent/notification-settings-sections.mounted.test.tsx +++ b/apps/mobile/src/components/security-agent/notification-settings-sections.mounted.test.tsx @@ -139,11 +139,11 @@ describe('personal scope', () => { expect(copy).not.toContain(i18n.t('securityAgent.notifications.slaSection')); }); - it('spaces the two switch groups like the space above the first one', () => { + it('separates the two switch groups by a break wider than the steps inside one', () => { const screen = mount(PERSONAL_SCOPE); expect(screen.root.findByType('TabScreenScrollView').props.contentContainerClassName).toBe( - 'px-6 pt-4 gap-4' + 'px-6 pt-4 gap-7' ); }); }); From 4ca24445fb5a9fed09f78d945f6a22213b6a8f0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Sun, 1 Nov 2026 18:23:59 +0000 Subject: [PATCH 5/5] test(mobile): assert the organization notification headings render --- ...ication-settings-sections.mounted.test.tsx | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/apps/mobile/src/components/security-agent/notification-settings-sections.mounted.test.tsx b/apps/mobile/src/components/security-agent/notification-settings-sections.mounted.test.tsx index 29b9927018e..ce28dab1cbd 100644 --- a/apps/mobile/src/components/security-agent/notification-settings-sections.mounted.test.tsx +++ b/apps/mobile/src/components/security-agent/notification-settings-sections.mounted.test.tsx @@ -127,16 +127,30 @@ function mount(scope: string): TestRenderer.ReactTestRenderer { return renderer; } +/** + * The section headings the tree paints, in order. A switch's accessible label + * is composed from the same strings on separate lines, so it cannot stand in + * for the heading itself. + */ +function sectionHeadings(screen: TestRenderer.ReactTestRenderer): string[] { + const sectionKeys = new Set([ + i18n.t('securityAgent.notifications.newFindingSection'), + i18n.t('securityAgent.notifications.slaSection'), + ]); + return screen.root + .findAllByType('Text') + .map(node => node.props.children) + .filter((child): child is string => typeof child === 'string' && sectionKeys.has(child)); +} + describe('personal scope', () => { it('titles each switch after its notification and shows no section heading', () => { - const copy = mount(PERSONAL_SCOPE) - .root.findAllByType('Text') - .map(node => node.props.children); + const screen = mount(PERSONAL_SCOPE); + const copy = screen.root.findAllByType('Text').map(node => node.props.children); expect(copy).toContain(i18n.t('securityAgent.notifications.newFindingPersonalTitle')); expect(copy).toContain(i18n.t('securityAgent.notifications.slaPersonalTitle')); - expect(copy).not.toContain(i18n.t('securityAgent.notifications.newFindingSection')); - expect(copy).not.toContain(i18n.t('securityAgent.notifications.slaSection')); + expect(sectionHeadings(screen)).toEqual([]); }); it('separates the two switch groups by a break wider than the steps inside one', () => { @@ -150,14 +164,17 @@ describe('personal scope', () => { describe('organization scope', () => { it('heads each switch with its notification section, because both read the same recipient', () => { - const names = mount(ORGANIZATION_SCOPE) - .root.findAllByType('Switch') - .map(node => node.props.accessibilityLabel); + const screen = mount(ORGANIZATION_SCOPE); + const names = screen.root.findAllByType('Switch').map(node => node.props.accessibilityLabel); expect(names).toHaveLength(2); expect(names[0]).toContain(i18n.t('securityAgent.notifications.newFindingSection')); expect(names[1]).toContain(i18n.t('securityAgent.notifications.slaSection')); expect(names[1]).not.toContain(i18n.t('securityAgent.notifications.newFindingSection')); + expect(sectionHeadings(screen)).toEqual([ + i18n.t('securityAgent.notifications.newFindingSection'), + i18n.t('securityAgent.notifications.slaSection'), + ]); }); it('keeps the standard group gap, next to its headings', () => {