From 411271b4545f4d80513b3d4e7038fed30bdf473b Mon Sep 17 00:00:00 2001 From: PathGao <42336971+PathGao@users.noreply.github.com> Date: Wed, 29 Jul 2026 05:03:13 +0800 Subject: [PATCH] refactor(ui): share one onNumber handler for numeric setting inputs (#6127) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(ui): share one onNumber handler for numeric setting inputs The Number(v) || 0 idiom in InputNumber onChange handlers is the root pattern behind the cleared-port bug (#6121): AntD reports a cleared field as null, and || 0 turns that into a stored zero or a min-clamp. The port fields got an inline null-guard; the other sixteen numeric settings kept the idiom, so every new field is a chance to reintroduce the bug. Extract the guard into onNumber(apply): null, empty and NaN change events are ignored so a cleared field snaps back to its stored value on blur, and numeric events pass through unchanged. Convert all sixteen sites in the settings and xray pages. Two sites keep their deliberate different semantics: smtpPort falls back to 587 on clear, and the Telegram notify interval clamps through Math.max. For the non-port fields this changes clearing from storing 0 to keeping the stored value; zero remains reachable by typing it. Co-Authored-By: Claude Fable 5 * refactor(ui): fold the remaining hand-rolled numeric guards into onNumber From review: ObservatorySettingsTab's sampling field hand-rolled the same ignore-null semantic and smtpPort kept a fallback-to-587 on clear that nothing documents as intentional and that silently overwrites a configured non-standard port — both now go through the shared helper, leaving the Telegram interval clamp as the one deliberate exception. Also from review: narrow the helper to numbers only (no stringMode input exists in the repo, and the string branch codified a guarantee the number-typed callback cannot honour), soften the docblock to describe behavior rather than promise prevention, add a GeneralTab component test covering the clear-vs-typed-zero semantics, and assert the blur snap-back in both settings tests so a display/state desync cannot ship unnoticed. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 --- frontend/src/pages/settings/EmailTab.tsx | 3 +- frontend/src/pages/settings/GeneralTab.tsx | 19 ++++----- .../pages/settings/SubscriptionFormatsTab.tsx | 5 ++- .../pages/settings/SubscriptionGeneralTab.tsx | 5 ++- .../xray/balancers/ObservatorySettingsTab.tsx | 3 +- frontend/src/pages/xray/dns/DnsTab.tsx | 3 +- frontend/src/pages/xray/dns/useDnsColumns.tsx | 4 +- .../src/pages/xray/outbounds/OutboundsTab.tsx | 5 ++- frontend/src/test/general-tab.test.tsx | 40 +++++++++++++++++++ frontend/src/test/on-number.test.ts | 23 +++++++++++ .../test/subscription-general-tab.test.tsx | 1 + frontend/src/utils/onNumber.ts | 16 ++++++++ 12 files changed, 108 insertions(+), 19 deletions(-) create mode 100644 frontend/src/test/general-tab.test.tsx create mode 100644 frontend/src/test/on-number.test.ts create mode 100644 frontend/src/utils/onNumber.ts diff --git a/frontend/src/pages/settings/EmailTab.tsx b/frontend/src/pages/settings/EmailTab.tsx index 250a89009..e5e6a2780 100644 --- a/frontend/src/pages/settings/EmailTab.tsx +++ b/frontend/src/pages/settings/EmailTab.tsx @@ -3,6 +3,7 @@ import { useTranslation } from 'react-i18next'; import { Alert, Button, Input, InputNumber, Select, Space, Switch, Tabs } from 'antd'; import { MailOutlined, SendOutlined, SettingOutlined } from '@ant-design/icons'; import { HttpUtil } from '@/utils'; +import { onNumber } from '@/utils/onNumber'; import type { AllSetting } from '@/models/setting'; import { SettingListItem } from '@/components/ui'; import { EmailNotifications } from '@/components/ui/notifications/EmailNotifications'; @@ -64,7 +65,7 @@ export default function EmailTab({ allSetting, updateSetting }: EmailTabProps) { updateSetting({ smtpPort: Number(v) || 587 })} /> + onChange={onNumber((v) => updateSetting({ smtpPort: v }))} /> diff --git a/frontend/src/pages/settings/GeneralTab.tsx b/frontend/src/pages/settings/GeneralTab.tsx index 2c1ae840d..f98129a55 100644 --- a/frontend/src/pages/settings/GeneralTab.tsx +++ b/frontend/src/pages/settings/GeneralTab.tsx @@ -17,6 +17,7 @@ import { } from '@ant-design/icons'; import type { AllSetting } from '@/models/setting'; import { HttpUtil, LanguageManager } from '@/utils'; +import { onNumber } from '@/utils/onNumber'; import { SettingListItem } from '@/components/ui'; import { useMediaQuery } from '@/hooks/useMediaQuery'; import { catTabLabel } from './catTabLabel'; @@ -170,7 +171,7 @@ export default function GeneralTab({ allSetting, updateSetting }: GeneralTabProp { if (v != null) updateSetting({ webPort: v }); }} /> + onChange={onNumber((v) => updateSetting({ webPort: v }))} /> @@ -179,7 +180,7 @@ export default function GeneralTab({ allSetting, updateSetting }: GeneralTabProp updateSetting({ sessionMaxAge: Number(v) || 0 })} /> + onChange={onNumber((v) => updateSetting({ sessionMaxAge: v }))} /> updateSetting({ pageSize: Number(v) || 0 })} /> + onChange={onNumber((v) => updateSetting({ pageSize: v }))} /> @@ -234,11 +235,11 @@ export default function GeneralTab({ allSetting, updateSetting }: GeneralTabProp <> updateSetting({ expireDiff: Number(v) || 0 })} /> + onChange={onNumber((v) => updateSetting({ expireDiff: v }))} /> updateSetting({ trafficDiff: Number(v) || 0 })} /> + onChange={onNumber((v) => updateSetting({ trafficDiff: v }))} /> ), @@ -308,7 +309,7 @@ export default function GeneralTab({ allSetting, updateSetting }: GeneralTabProp { if (v != null) updateSetting({ ldapPort: v }); }} /> + onChange={onNumber((v) => updateSetting({ ldapPort: v }))} /> updateSetting({ ldapUseTLS: v })} /> @@ -387,15 +388,15 @@ export default function GeneralTab({ allSetting, updateSetting }: GeneralTabProp updateSetting({ ldapDefaultTotalGB: Number(v) || 0 })} /> + onChange={onNumber((v) => updateSetting({ ldapDefaultTotalGB: v }))} /> updateSetting({ ldapDefaultExpiryDays: Number(v) || 0 })} /> + onChange={onNumber((v) => updateSetting({ ldapDefaultExpiryDays: v }))} /> updateSetting({ ldapDefaultLimitIP: Number(v) || 0 })} /> + onChange={onNumber((v) => updateSetting({ ldapDefaultLimitIP: v }))} /> ), diff --git a/frontend/src/pages/settings/SubscriptionFormatsTab.tsx b/frontend/src/pages/settings/SubscriptionFormatsTab.tsx index 403a91227..eebb759d5 100644 --- a/frontend/src/pages/settings/SubscriptionFormatsTab.tsx +++ b/frontend/src/pages/settings/SubscriptionFormatsTab.tsx @@ -17,6 +17,7 @@ import { SettingOutlined, } from '@ant-design/icons'; import type { AllSetting } from '@/models/setting'; +import { onNumber } from '@/utils/onNumber'; import { SettingListItem } from '@/components/ui'; import { GoRegexInput } from '@/components/form'; import { useMediaQuery } from '@/hooks/useMediaQuery'; @@ -279,11 +280,11 @@ export default function SubscriptionFormatsTab({ allSetting, updateSetting }: Su
setMuxField('concurrency', Number(v) || 0)} /> + onChange={onNumber((v) => setMuxField('concurrency', v))} /> setMuxField('xudpConcurrency', Number(v) || 0)} /> + onChange={onNumber((v) => setMuxField('xudpConcurrency', v))} /> updateSetting({ subUpdates: Number(v) || 0 })} /> + onChange={onNumber((v) => updateSetting({ subUpdates: v }))} /> ), diff --git a/frontend/src/pages/xray/balancers/ObservatorySettingsTab.tsx b/frontend/src/pages/xray/balancers/ObservatorySettingsTab.tsx index ceb29f2f7..c1704138b 100644 --- a/frontend/src/pages/xray/balancers/ObservatorySettingsTab.tsx +++ b/frontend/src/pages/xray/balancers/ObservatorySettingsTab.tsx @@ -2,6 +2,7 @@ import { useMemo } from 'react'; import { useTranslation } from 'react-i18next'; import { Alert, Empty, Input, InputNumber, Select, Space, Switch, Tag } from 'antd'; +import { onNumber } from '@/utils/onNumber'; import { SettingListItem } from '@/components/ui'; import { BurstObservatorySchema, @@ -195,7 +196,7 @@ export default function ObservatorySettingsTab({ patchPingConfig({ sampling: typeof v === 'number' ? v : burst.pingConfig.sampling })} + onChange={onNumber((v) => patchPingConfig({ sampling: v }))} style={{ width: '100%' }} /> diff --git a/frontend/src/pages/xray/dns/DnsTab.tsx b/frontend/src/pages/xray/dns/DnsTab.tsx index 5d5cc3ebc..0eb8c1dbe 100644 --- a/frontend/src/pages/xray/dns/DnsTab.tsx +++ b/frontend/src/pages/xray/dns/DnsTab.tsx @@ -11,6 +11,7 @@ import { SettingOutlined, } from '@ant-design/icons'; +import { onNumber } from '@/utils/onNumber'; import { SettingListItem } from '@/components/ui'; import { useMediaQuery } from '@/hooks/useMediaQuery'; import { catTabLabel } from '@/pages/settings/catTabLabel'; @@ -311,7 +312,7 @@ export default function DnsTab({ templateSettings, setTemplateSettings }: DnsTab min={0} step={60} style={{ width: '100%' }} - onChange={(v) => setDnsField('serveExpiredTTL', Number(v) || 0)} + onChange={onNumber((v) => setDnsField('serveExpiredTTL', v))} /> } /> diff --git a/frontend/src/pages/xray/dns/useDnsColumns.tsx b/frontend/src/pages/xray/dns/useDnsColumns.tsx index b523bc2a6..8244010a0 100644 --- a/frontend/src/pages/xray/dns/useDnsColumns.tsx +++ b/frontend/src/pages/xray/dns/useDnsColumns.tsx @@ -4,6 +4,8 @@ import { Button, Dropdown, Input, InputNumber, Space } from 'antd'; import { MoreOutlined, EditOutlined, DeleteOutlined } from '@ant-design/icons'; import type { ColumnsType } from 'antd/es/table'; +import { onNumber } from '@/utils/onNumber'; + import { addrFor, domainsFor, expectedIPsFor } from './helpers'; import type { DnsServerValue } from './DnsServerModal'; @@ -113,7 +115,7 @@ export function useFakednsColumns({ aria-label={t('pages.xray.fakedns.poolSize')} min={1} size="small" - onChange={(v) => updateFakednsField(index, 'poolSize', Number(v) || 0)} + onChange={onNumber((v) => updateFakednsField(index, 'poolSize', v))} /> ), }, diff --git a/frontend/src/pages/xray/outbounds/OutboundsTab.tsx b/frontend/src/pages/xray/outbounds/OutboundsTab.tsx index f4e15bcb0..b29d05e9f 100644 --- a/frontend/src/pages/xray/outbounds/OutboundsTab.tsx +++ b/frontend/src/pages/xray/outbounds/OutboundsTab.tsx @@ -38,6 +38,7 @@ import { } from '@ant-design/icons'; import { HttpUtil } from '@/utils'; +import { onNumber } from '@/utils/onNumber'; import PromptModal from '@/components/feedback/PromptModal'; import TextModal from '@/components/feedback/TextModal'; @@ -626,14 +627,14 @@ export default function OutboundsTab({ setIntervalHM(Number(v) || 0, intervalMinutes)} + onChange={onNumber((v) => setIntervalHM(v, intervalMinutes))} style={{ width: 80 }} /> {t('pages.xray.outboundSub.hours')} setIntervalHM(intervalHours, Number(v) || 0)} + onChange={onNumber((v) => setIntervalHM(intervalHours, v))} style={{ width: 80 }} /> {t('pages.xray.outboundSub.minutes')} diff --git a/frontend/src/test/general-tab.test.tsx b/frontend/src/test/general-tab.test.tsx new file mode 100644 index 000000000..d2895be19 --- /dev/null +++ b/frontend/src/test/general-tab.test.tsx @@ -0,0 +1,40 @@ +import { fireEvent, screen } from '@testing-library/react'; +import { MemoryRouter } from 'react-router'; +import { describe, expect, it, vi } from 'vitest'; + +import { AllSetting } from '@/models/setting'; +import GeneralTab from '@/pages/settings/GeneralTab'; +import { renderWithProviders } from './test-utils'; + +describe('GeneralTab', () => { + it('keeps the stored page size when the field is cleared', () => { + const updateSetting = vi.fn(); + + renderWithProviders( + + + , + ); + + const pageSizeInput = screen.getByDisplayValue('25'); + fireEvent.change(pageSizeInput, { target: { value: '' } }); + fireEvent.blur(pageSizeInput); + + expect(updateSetting).not.toHaveBeenCalled(); + expect((pageSizeInput as HTMLInputElement).value).toBe('25'); + }); + + it('forwards typed page sizes unchanged, zero included', () => { + const updateSetting = vi.fn(); + + renderWithProviders( + + + , + ); + + fireEvent.change(screen.getByDisplayValue('25'), { target: { value: '0' } }); + + expect(updateSetting).toHaveBeenCalledWith({ pageSize: 0 }); + }); +}); diff --git a/frontend/src/test/on-number.test.ts b/frontend/src/test/on-number.test.ts new file mode 100644 index 000000000..a0d32219d --- /dev/null +++ b/frontend/src/test/on-number.test.ts @@ -0,0 +1,23 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { onNumber } from '@/utils/onNumber'; + +describe('onNumber', () => { + it('forwards numeric values, including zero and negatives', () => { + const apply = vi.fn(); + const handler = onNumber(apply); + handler(8443); + handler(0); + handler(-1); + expect(apply.mock.calls).toEqual([[8443], [0], [-1]]); + }); + + it('ignores cleared events instead of writing a synthetic value', () => { + const apply = vi.fn(); + const handler = onNumber(apply); + handler(null); + handler(undefined); + handler(NaN); + expect(apply).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/test/subscription-general-tab.test.tsx b/frontend/src/test/subscription-general-tab.test.tsx index fbc4914a5..769cdae00 100644 --- a/frontend/src/test/subscription-general-tab.test.tsx +++ b/frontend/src/test/subscription-general-tab.test.tsx @@ -26,6 +26,7 @@ describe('SubscriptionGeneralTab', () => { fireEvent.blur(portInput); expect(updateSetting).not.toHaveBeenCalled(); + expect((portInput as HTMLInputElement).value).toBe('2096'); }); it('forwards typed subscription ports unchanged', () => { diff --git a/frontend/src/utils/onNumber.ts b/frontend/src/utils/onNumber.ts new file mode 100644 index 000000000..77e1c4b6e --- /dev/null +++ b/frontend/src/utils/onNumber.ts @@ -0,0 +1,16 @@ +/** + * Wraps an Ant Design InputNumber change handler with the shared + * cleared-field semantic: null and undefined change events (a cleared or + * unparsable field) are ignored, leaving the stored value in place — the + * input snaps back on blur — while real numbers pass through unchanged. + * Number-only by design: not for `stringMode` inputs, whose whole point is + * to avoid the IEEE-754 round trip this signature would force. + */ +export function onNumber( + apply: (value: number) => void, +): (value: number | null | undefined) => void { + return (value) => { + if (typeof value !== 'number' || !Number.isFinite(value)) return; + apply(value); + }; +}