mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-17 13:13:35 +08:00
Prevent Settings General checkbox focus/reflow from using off-row sr-only inputs, close stale native preview views when leaving session pages, and avoid Windows notification enable-time smoke side effects. Tested: cd desktop && bun run test -- src/__tests__/generalSettings.test.tsx --reporter verbose Tested: cd desktop && bun run test -- src/components/layout/ContentRouter.test.tsx --reporter verbose Tested: cd desktop && bun run test -- src/lib/desktopNotifications.test.ts --reporter verbose Tested: cd desktop && bun run lint Tested: bun run check:desktop Confidence: medium Scope-risk: narrow
This commit is contained in:
parent
52aa3c5c02
commit
2f243fe920
@ -16,6 +16,7 @@ const MOCK_GET_SETTINGS = vi.fn()
|
||||
const MOCK_UPDATE_SETTINGS = vi.fn()
|
||||
const desktopNotificationsMock = vi.hoisted(() => ({
|
||||
getDesktopNotificationPermission: vi.fn(),
|
||||
getDesktopNotificationPlatform: vi.fn(),
|
||||
notifyDesktop: vi.fn(),
|
||||
requestDesktopNotificationPermission: vi.fn(),
|
||||
openDesktopNotificationSettings: vi.fn(),
|
||||
@ -165,10 +166,12 @@ describe('Settings > General tab', () => {
|
||||
vi.useRealTimers()
|
||||
MOCK_DELETE_PROVIDER.mockReset()
|
||||
desktopNotificationsMock.getDesktopNotificationPermission.mockReset()
|
||||
desktopNotificationsMock.getDesktopNotificationPlatform.mockReset()
|
||||
desktopNotificationsMock.notifyDesktop.mockReset()
|
||||
desktopNotificationsMock.requestDesktopNotificationPermission.mockReset()
|
||||
desktopNotificationsMock.openDesktopNotificationSettings.mockReset()
|
||||
desktopNotificationsMock.getDesktopNotificationPermission.mockResolvedValue('default')
|
||||
desktopNotificationsMock.getDesktopNotificationPlatform.mockReturnValue('darwin')
|
||||
desktopNotificationsMock.notifyDesktop.mockResolvedValue(true)
|
||||
desktopNotificationsMock.requestDesktopNotificationPermission.mockResolvedValue('granted')
|
||||
desktopNotificationsMock.openDesktopNotificationSettings.mockResolvedValue(true)
|
||||
@ -207,6 +210,7 @@ describe('Settings > General tab', () => {
|
||||
autoDreamEnabled: false,
|
||||
skipWebFetchPreflight: true,
|
||||
desktopNotificationsEnabled: true,
|
||||
traceCapture: { enabled: true, storageDir: '/Users/test/.claude/cc-haha/traces' },
|
||||
chatSendBehavior: 'enter',
|
||||
responseLanguage: '',
|
||||
uiZoom: 1,
|
||||
@ -255,6 +259,10 @@ describe('Settings > General tab', () => {
|
||||
setDesktopNotificationsEnabled: vi.fn().mockImplementation(async (enabled: boolean) => {
|
||||
useSettingsStore.setState({ desktopNotificationsEnabled: enabled })
|
||||
}),
|
||||
setTraceCaptureEnabled: vi.fn().mockImplementation(async (enabled: boolean) => {
|
||||
const current = useSettingsStore.getState().traceCapture
|
||||
useSettingsStore.setState({ traceCapture: { ...current, enabled } })
|
||||
}),
|
||||
setChatSendBehavior: vi.fn().mockImplementation(async (chatSendBehavior: ChatSendBehavior) => {
|
||||
useSettingsStore.setState({ chatSendBehavior })
|
||||
}),
|
||||
@ -757,6 +765,43 @@ describe('Settings > General tab', () => {
|
||||
expect(useSettingsStore.getState().setAutoDreamEnabled).toHaveBeenCalledWith(false)
|
||||
})
|
||||
|
||||
it('keeps General checkbox inputs anchored inside their visible rows', () => {
|
||||
render(<Settings />)
|
||||
|
||||
fireEvent.click(screen.getByText('General'))
|
||||
|
||||
for (const label of [
|
||||
'Enable thinking mode',
|
||||
'Enable Auto-dream',
|
||||
'Collect agent traces',
|
||||
'Enable system notifications',
|
||||
'Skip WebFetch domain preflight',
|
||||
]) {
|
||||
const toggle = screen.getByLabelText(label)
|
||||
const row = toggle.closest('label') as HTMLElement | null
|
||||
expect(toggle).toHaveClass('settings-checkbox-input')
|
||||
expect(toggle).not.toHaveClass('sr-only')
|
||||
expect(row).not.toBeNull()
|
||||
expect(row!).toHaveClass('relative')
|
||||
}
|
||||
})
|
||||
|
||||
it('lets the user disable Agent Trace collection without leaving General settings', async () => {
|
||||
render(<Settings />)
|
||||
|
||||
fireEvent.click(screen.getByText('General'))
|
||||
expect(screen.getByLabelText('Collect agent traces')).toBeChecked()
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByLabelText('Collect agent traces'))
|
||||
})
|
||||
|
||||
expect(useSettingsStore.getState().setTraceCaptureEnabled).toHaveBeenCalledWith(false)
|
||||
expect(screen.getByLabelText('Collect agent traces')).not.toBeChecked()
|
||||
expect(screen.getByText('Agent trace')).toBeInTheDocument()
|
||||
expect(screen.getByText('Message Sending')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('uses the shared dropdown for response language', () => {
|
||||
render(<Settings />)
|
||||
|
||||
@ -805,7 +850,25 @@ describe('Settings > General tab', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('opens system settings when enabling notifications finds system denial', async () => {
|
||||
it('does not fire the enable smoke notification on Windows Electron', async () => {
|
||||
useSettingsStore.setState({ desktopNotificationsEnabled: false })
|
||||
desktopNotificationsMock.getDesktopNotificationPlatform.mockReturnValue('win32')
|
||||
render(<Settings />)
|
||||
|
||||
fireEvent.click(screen.getByText('General'))
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByLabelText('Enable system notifications'))
|
||||
})
|
||||
|
||||
expect(useSettingsStore.getState().setDesktopNotificationsEnabled).toHaveBeenCalledWith(true)
|
||||
await vi.waitFor(() => {
|
||||
expect(desktopNotificationsMock.requestDesktopNotificationPermission).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
expect(desktopNotificationsMock.notifyDesktop).not.toHaveBeenCalled()
|
||||
expect(desktopNotificationsMock.openDesktopNotificationSettings).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('shows the system settings action when enabling notifications finds system denial', async () => {
|
||||
useSettingsStore.setState({ desktopNotificationsEnabled: false })
|
||||
desktopNotificationsMock.requestDesktopNotificationPermission.mockResolvedValue('denied')
|
||||
render(<Settings />)
|
||||
@ -816,8 +879,15 @@ describe('Settings > General tab', () => {
|
||||
})
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(desktopNotificationsMock.openDesktopNotificationSettings).toHaveBeenCalledTimes(1)
|
||||
expect(screen.getByText('Permission: Blocked by system settings')).toBeInTheDocument()
|
||||
})
|
||||
expect(desktopNotificationsMock.openDesktopNotificationSettings).not.toHaveBeenCalled()
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Open Settings' }))
|
||||
})
|
||||
|
||||
expect(desktopNotificationsMock.openDesktopNotificationSettings).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('moves H5 access out of General into its own Settings tab', () => {
|
||||
|
||||
@ -1,7 +1,15 @@
|
||||
import { cleanup, fireEvent, render, screen } from '@testing-library/react'
|
||||
import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'
|
||||
import '@testing-library/jest-dom'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { previewBridgeMock } = vi.hoisted(() => ({
|
||||
previewBridgeMock: {
|
||||
close: vi.fn().mockResolvedValue(undefined),
|
||||
},
|
||||
}))
|
||||
|
||||
vi.mock('../../lib/previewBridge', () => ({ previewBridge: previewBridgeMock }))
|
||||
|
||||
vi.mock('../../pages/EmptySession', () => ({
|
||||
EmptySession: () => <div data-testid="empty-session" />,
|
||||
}))
|
||||
@ -40,6 +48,7 @@ import { useTabStore } from '../../stores/tabStore'
|
||||
describe('ContentRouter terminal tabs', () => {
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
previewBridgeMock.close.mockClear()
|
||||
useTabStore.setState({ tabs: [], activeTabId: null })
|
||||
})
|
||||
|
||||
@ -138,4 +147,27 @@ describe('ContentRouter terminal tabs', () => {
|
||||
expect(screen.getByTestId('trace-list')).toBeInTheDocument()
|
||||
expect(screen.queryByTestId('active-session')).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('closes the native preview when switching from a chat session to settings', async () => {
|
||||
useTabStore.setState({
|
||||
tabs: [
|
||||
{ sessionId: 'session-1', title: 'Chat', type: 'session', status: 'idle' },
|
||||
{ sessionId: '__settings__', title: 'Settings', type: 'settings', status: 'idle' },
|
||||
],
|
||||
activeTabId: 'session-1',
|
||||
})
|
||||
|
||||
render(<ContentRouter />)
|
||||
expect(screen.getByTestId('active-session')).toBeInTheDocument()
|
||||
previewBridgeMock.close.mockClear()
|
||||
|
||||
act(() => {
|
||||
useTabStore.setState({ activeTabId: '__settings__' })
|
||||
})
|
||||
|
||||
expect(screen.getByTestId('settings-page')).toBeInTheDocument()
|
||||
await waitFor(() => {
|
||||
expect(previewBridgeMock.close).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@ -1,4 +1,4 @@
|
||||
import type { ReactNode } from 'react'
|
||||
import { useEffect, type ReactNode } from 'react'
|
||||
import { useTabStore } from '../../stores/tabStore'
|
||||
import { EmptySession } from '../../pages/EmptySession'
|
||||
import { ActiveSession } from '../../pages/ActiveSession'
|
||||
@ -7,6 +7,7 @@ import { Settings } from '../../pages/Settings'
|
||||
import { TerminalSettings } from '../../pages/TerminalSettings'
|
||||
import { TraceList } from '../../pages/TraceList'
|
||||
import { TraceSession } from '../../pages/TraceSession'
|
||||
import { previewBridge } from '../../lib/previewBridge'
|
||||
|
||||
export function ContentRouter() {
|
||||
const activeTabId = useTabStore((s) => s.activeTabId)
|
||||
@ -14,6 +15,11 @@ export function ContentRouter() {
|
||||
const activeTabType = tabs.find((t) => t.sessionId === activeTabId)?.type
|
||||
const terminalTabs = tabs.filter((tab) => tab.type === 'terminal')
|
||||
|
||||
useEffect(() => {
|
||||
if (activeTabType === 'session') return
|
||||
void previewBridge.close()
|
||||
}, [activeTabType])
|
||||
|
||||
let page: ReactNode = null
|
||||
if (!activeTabId || !activeTabType) {
|
||||
page = <EmptySession />
|
||||
|
||||
@ -57,6 +57,10 @@ function detectPlatform(): 'darwin' | 'win32' | 'linux' | 'unknown' {
|
||||
return 'unknown'
|
||||
}
|
||||
|
||||
export function getDesktopNotificationPlatform(): 'darwin' | 'win32' | 'linux' | 'unknown' {
|
||||
return detectPlatform()
|
||||
}
|
||||
|
||||
function getNotificationSettingsUrl(): string | null {
|
||||
switch (detectPlatform()) {
|
||||
case 'darwin':
|
||||
|
||||
@ -44,6 +44,7 @@ import { publicAssetPath } from '../lib/publicAsset'
|
||||
import {
|
||||
getDesktopNotificationPermission,
|
||||
notifyDesktop,
|
||||
getDesktopNotificationPlatform,
|
||||
openDesktopNotificationSettings,
|
||||
requestDesktopNotificationPermission,
|
||||
type DesktopNotificationPermission,
|
||||
@ -59,6 +60,7 @@ import { copyTextToClipboard } from '../components/chat/clipboard'
|
||||
const NETWORK_TIMEOUT_MIN_SECONDS = 5
|
||||
const NETWORK_TIMEOUT_MAX_SECONDS = 600
|
||||
const NETWORK_TIMEOUT_STEP_SECONDS = 30
|
||||
const SETTINGS_CHECKBOX_INPUT_CLASS = 'settings-checkbox-input peer'
|
||||
|
||||
function buildH5LaunchUrl(baseUrl: string | null, token: string | null): string | null {
|
||||
if (!baseUrl) return null
|
||||
@ -1652,15 +1654,12 @@ export function GeneralSettings() {
|
||||
try {
|
||||
const permission = await requestDesktopNotificationPermission()
|
||||
setNotificationPermission(permission)
|
||||
if (permission === 'granted') {
|
||||
if (permission === 'granted' && getDesktopNotificationPlatform() !== 'win32') {
|
||||
void notifyDesktop({
|
||||
title: t('settings.general.notificationsTestTitle'),
|
||||
body: t('settings.general.notificationsTestBody'),
|
||||
})
|
||||
}
|
||||
if (permission === 'denied') {
|
||||
await openDesktopNotificationSettings()
|
||||
}
|
||||
} finally {
|
||||
setNotificationActionRunning(false)
|
||||
}
|
||||
@ -2096,13 +2095,13 @@ export function GeneralSettings() {
|
||||
<div className="mt-8">
|
||||
<h2 className="text-base font-semibold text-[var(--color-text-primary)] mb-1">{t('settings.general.thinkingTitle')}</h2>
|
||||
<p className="text-sm text-[var(--color-text-tertiary)] mb-3">{t('settings.general.thinkingDescription')}</p>
|
||||
<label className="flex items-start gap-3 rounded-xl border border-[var(--color-border)] bg-[var(--color-surface-container-low)] px-4 py-3 cursor-pointer hover:border-[var(--color-border-focus)] transition-colors">
|
||||
<label className="relative flex items-start gap-3 rounded-xl border border-[var(--color-border)] bg-[var(--color-surface-container-low)] px-4 py-3 cursor-pointer hover:border-[var(--color-border-focus)] transition-colors">
|
||||
<input
|
||||
type="checkbox"
|
||||
aria-label={t('settings.general.thinkingEnabled')}
|
||||
checked={thinkingEnabled}
|
||||
onChange={(e) => void setThinkingEnabled(e.target.checked)}
|
||||
className="peer sr-only"
|
||||
className={SETTINGS_CHECKBOX_INPUT_CLASS}
|
||||
/>
|
||||
<SettingsCheckboxMark checked={thinkingEnabled} />
|
||||
<div className="min-w-0">
|
||||
@ -2119,13 +2118,13 @@ export function GeneralSettings() {
|
||||
<div className="mt-8">
|
||||
<h2 className="text-base font-semibold text-[var(--color-text-primary)] mb-1">{t('settings.general.autoDreamTitle')}</h2>
|
||||
<p className="text-sm text-[var(--color-text-tertiary)] mb-3">{t('settings.general.autoDreamDescription')}</p>
|
||||
<label className="flex items-start gap-3 rounded-xl border border-[var(--color-border)] bg-[var(--color-surface-container-low)] px-4 py-3 cursor-pointer hover:border-[var(--color-border-focus)] transition-colors">
|
||||
<label className="relative flex items-start gap-3 rounded-xl border border-[var(--color-border)] bg-[var(--color-surface-container-low)] px-4 py-3 cursor-pointer hover:border-[var(--color-border-focus)] transition-colors">
|
||||
<input
|
||||
type="checkbox"
|
||||
aria-label={t('settings.general.autoDreamEnabled')}
|
||||
checked={autoDreamEnabled}
|
||||
onChange={(e) => handleAutoDreamToggle(e.target.checked)}
|
||||
className="peer sr-only"
|
||||
className={SETTINGS_CHECKBOX_INPUT_CLASS}
|
||||
/>
|
||||
<SettingsCheckboxMark checked={autoDreamEnabled} />
|
||||
<div className="min-w-0">
|
||||
@ -2144,13 +2143,13 @@ export function GeneralSettings() {
|
||||
<div className="mt-8">
|
||||
<h2 className="text-base font-semibold text-[var(--color-text-primary)] mb-1">{t('settings.general.traceTitle')}</h2>
|
||||
<p className="text-sm text-[var(--color-text-tertiary)] mb-3">{t('settings.general.traceDescription')}</p>
|
||||
<label className="flex items-start gap-3 rounded-xl border border-[var(--color-border)] bg-[var(--color-surface-container-low)] px-4 py-3 cursor-pointer hover:border-[var(--color-border-focus)] transition-colors">
|
||||
<label className="relative flex items-start gap-3 rounded-xl border border-[var(--color-border)] bg-[var(--color-surface-container-low)] px-4 py-3 cursor-pointer hover:border-[var(--color-border-focus)] transition-colors">
|
||||
<input
|
||||
type="checkbox"
|
||||
aria-label={t('settings.general.traceEnabled')}
|
||||
checked={traceCapture.enabled}
|
||||
onChange={(e) => void setTraceCaptureEnabled(e.target.checked)}
|
||||
className="peer sr-only"
|
||||
className={SETTINGS_CHECKBOX_INPUT_CLASS}
|
||||
/>
|
||||
<SettingsCheckboxMark checked={traceCapture.enabled} />
|
||||
<div className="min-w-0 flex-1">
|
||||
@ -2173,13 +2172,13 @@ export function GeneralSettings() {
|
||||
<h2 className="text-base font-semibold text-[var(--color-text-primary)] mb-1">{t('settings.general.notificationsTitle')}</h2>
|
||||
<p className="text-sm text-[var(--color-text-tertiary)] mb-3">{t('settings.general.notificationsDescription')}</p>
|
||||
<div className="rounded-xl border border-[var(--color-border)] bg-[var(--color-surface-container-low)] px-4 py-3">
|
||||
<label className="flex items-start gap-3 cursor-pointer">
|
||||
<label className="relative flex items-start gap-3 cursor-pointer">
|
||||
<input
|
||||
type="checkbox"
|
||||
aria-label={t('settings.general.notificationsEnabled')}
|
||||
checked={desktopNotificationsEnabled}
|
||||
onChange={(e) => void handleDesktopNotificationsToggle(e.target.checked)}
|
||||
className="peer sr-only"
|
||||
className={SETTINGS_CHECKBOX_INPUT_CLASS}
|
||||
/>
|
||||
<SettingsCheckboxMark checked={desktopNotificationsEnabled} />
|
||||
<div className="min-w-0 flex-1">
|
||||
@ -2396,13 +2395,13 @@ export function GeneralSettings() {
|
||||
<div className="mt-8">
|
||||
<h2 className="text-base font-semibold text-[var(--color-text-primary)] mb-1">{t('settings.general.webFetchPreflightTitle')}</h2>
|
||||
<p className="text-sm text-[var(--color-text-tertiary)] mb-3">{t('settings.general.webFetchPreflightDescription')}</p>
|
||||
<label className="flex items-start gap-3 rounded-xl border border-[var(--color-border)] bg-[var(--color-surface-container-low)] px-4 py-3 cursor-pointer hover:border-[var(--color-border-focus)] transition-colors">
|
||||
<label className="relative flex items-start gap-3 rounded-xl border border-[var(--color-border)] bg-[var(--color-surface-container-low)] px-4 py-3 cursor-pointer hover:border-[var(--color-border-focus)] transition-colors">
|
||||
<input
|
||||
type="checkbox"
|
||||
aria-label={t('settings.general.webFetchPreflightEnabled')}
|
||||
checked={skipWebFetchPreflight}
|
||||
onChange={(e) => void setSkipWebFetchPreflight(e.target.checked)}
|
||||
className="peer sr-only"
|
||||
className={SETTINGS_CHECKBOX_INPUT_CLASS}
|
||||
/>
|
||||
<SettingsCheckboxMark checked={skipWebFetchPreflight} />
|
||||
<div className="min-w-0">
|
||||
|
||||
@ -130,6 +130,21 @@
|
||||
outline: none;
|
||||
}
|
||||
|
||||
.settings-checkbox-input {
|
||||
position: absolute;
|
||||
inset: 0;
|
||||
z-index: 1;
|
||||
width: 100%;
|
||||
height: 100%;
|
||||
margin: 0;
|
||||
cursor: pointer;
|
||||
opacity: 0;
|
||||
}
|
||||
|
||||
.settings-checkbox-input:disabled {
|
||||
cursor: not-allowed;
|
||||
}
|
||||
|
||||
.settings-zoom-kbd {
|
||||
min-width: 18px;
|
||||
height: 18px;
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user