Skip to content

Commit c957aa2

Browse files
authored
improvement(settings): react-doctor perf & correctness pass (#5327)
* improvement(settings): react-doctor perf & correctness pass - combine multi-pass array iterations into single passes (api-keys, credential-sets, secrets-manager, team-management) - cache/hoist Intl formatters to module scope (billing); hoist pure functions and inline-default constants - stabilize react-query array fallbacks so memos stop recomputing while loading (api-keys, byok, workflow-mcp) - fix create-workflow-mcp modal reset via render-phase prevOpen compare instead of a state-adjusting effect - accessibility: aria-labels on real inputs, aria-hidden on autofill decoys, native <button> for clickable rows - immutable in-place sort where the array is already a fresh copy * fix(settings): render non-clickable inbox rows as div, not disabled button Addresses review: a native disabled <button> can inherit browser disabled styling (dimmed text / lower contrast) on non-navigable task rows. Render an interactive <button> only when the row is clickable; otherwise a plain <div> with identical layout — preserving the semantic-button a11y win without the disabled-state visual regression.
1 parent 78661d2 commit c957aa2

21 files changed

Lines changed: 176 additions & 129 deletions

File tree

apps/sim/app/workspace/[workspaceId]/settings/components/api-keys/api-keys.tsx

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,22 @@ import { CreateApiKeyModal } from './components'
2424

2525
const logger = createLogger('ApiKeys')
2626

27+
/** Stable empty references so memoized derivations don't re-run while data loads. */
28+
const EMPTY_KEYS: ApiKey[] = []
29+
const EMPTY_KEY_NAMES: string[] = []
30+
2731
/** Copies an API key's name and confirms with a toast. */
2832
function copyKeyName(name: string) {
2933
void navigator.clipboard.writeText(name)
3034
toast.success('Copied name to clipboard')
3135
}
3236

37+
/** Formats an API key's last-used timestamp, or "Never" when unused. */
38+
function formatLastUsed(dateString?: string | null): string {
39+
if (!dateString) return 'Never'
40+
return formatDate(new Date(dateString))
41+
}
42+
3343
interface ApiKeyRowMenuProps {
3444
keyName: string
3545
onDelete: () => void
@@ -73,9 +83,9 @@ export function ApiKeys() {
7383
const deleteApiKeyMutation = useDeleteApiKey()
7484
const updateSettingsMutation = useUpdateWorkspaceApiKeySettings()
7585

76-
const workspaceKeys = apiKeysData?.workspaceKeys || []
77-
const personalKeys = apiKeysData?.personalKeys || []
78-
const conflicts = apiKeysData?.conflicts || []
86+
const workspaceKeys = apiKeysData?.workspaceKeys ?? EMPTY_KEYS
87+
const personalKeys = apiKeysData?.personalKeys ?? EMPTY_KEYS
88+
const conflicts = apiKeysData?.conflicts ?? EMPTY_KEY_NAMES
7989
const isLoading = isLoadingKeys || isLoadingSettings
8090

8191
const allowPersonalApiKeys =
@@ -90,21 +100,27 @@ export function ApiKeys() {
90100
const createButtonDisabled = isLoading || (!allowPersonalApiKeys && !canManageWorkspaceKeys)
91101

92102
const filteredWorkspaceKeys = useMemo(() => {
93-
if (!searchTerm.trim()) {
94-
return workspaceKeys.map((key, index) => ({ key, originalIndex: index }))
103+
const term = searchTerm.trim().toLowerCase()
104+
const result: { key: ApiKey; originalIndex: number }[] = []
105+
for (let index = 0; index < workspaceKeys.length; index++) {
106+
const key = workspaceKeys[index]
107+
if (term === '' || key.name.toLowerCase().includes(term)) {
108+
result.push({ key, originalIndex: index })
109+
}
95110
}
96-
return workspaceKeys
97-
.map((key, index) => ({ key, originalIndex: index }))
98-
.filter(({ key }) => key.name.toLowerCase().includes(searchTerm.toLowerCase()))
111+
return result
99112
}, [workspaceKeys, searchTerm])
100113

101114
const filteredPersonalKeys = useMemo(() => {
102-
if (!searchTerm.trim()) {
103-
return personalKeys.map((key, index) => ({ key, originalIndex: index }))
115+
const term = searchTerm.trim().toLowerCase()
116+
const result: { key: ApiKey; originalIndex: number }[] = []
117+
for (let index = 0; index < personalKeys.length; index++) {
118+
const key = personalKeys[index]
119+
if (term === '' || key.name.toLowerCase().includes(term)) {
120+
result.push({ key, originalIndex: index })
121+
}
104122
}
105-
return personalKeys
106-
.map((key, index) => ({ key, originalIndex: index }))
107-
.filter(({ key }) => key.name.toLowerCase().includes(searchTerm.toLowerCase()))
123+
return result
108124
}, [personalKeys, searchTerm])
109125

110126
const handleDeleteKey = async () => {
@@ -128,11 +144,6 @@ export function ApiKeys() {
128144
}
129145
}
130146

131-
const formatLastUsed = (dateString?: string | null) => {
132-
if (!dateString) return 'Never'
133-
return formatDate(new Date(dateString))
134-
}
135-
136147
const actions: SettingsAction[] = [
137148
{
138149
text: 'Create API key',

apps/sim/app/workspace/[workspaceId]/settings/components/api-keys/components/create-api-key-modal/create-api-key-modal.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ export function CreateApiKeyModal({
143143
type='text'
144144
name='fakeusernameremembered'
145145
autoComplete='username'
146+
aria-hidden='true'
146147
style={{
147148
position: 'absolute',
148149
left: '-9999px',

apps/sim/app/workspace/[workspaceId]/settings/components/billing/billing.tsx

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,23 @@ function formatInvoiceDate(createdSeconds: number): string {
9090
})
9191
}
9292

93+
/** Cached currency formatters, keyed by upper-cased ISO currency code. */
94+
const invoiceAmountFormatters = new Map<string, Intl.NumberFormat>()
95+
96+
/** Resolve (and memoize) an `Intl.NumberFormat` for a currency code. */
97+
function getInvoiceAmountFormatter(currency: string): Intl.NumberFormat {
98+
const code = currency.toUpperCase()
99+
let formatter = invoiceAmountFormatters.get(code)
100+
if (!formatter) {
101+
formatter = new Intl.NumberFormat(undefined, { style: 'currency', currency: code })
102+
invoiceAmountFormatters.set(code, formatter)
103+
}
104+
return formatter
105+
}
106+
93107
/** Format a minor-unit (e.g. cents) amount as a localized currency string. */
94108
function formatInvoiceAmount(amountMinor: number, currency: string): string {
95-
return new Intl.NumberFormat(undefined, {
96-
style: 'currency',
97-
currency: currency.toUpperCase(),
98-
}).format(amountMinor / 100)
109+
return getInvoiceAmountFormatter(currency).format(amountMinor / 100)
99110
}
100111

101112
export function Billing() {

apps/sim/app/workspace/[workspaceId]/settings/components/billing/components/usage-limit-field/usage-limit-field.tsx

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,15 @@ import { useDebounce } from '@/hooks/use-debounce'
1313
/** Delay before a usage-limit edit is auto-saved once the user stops typing. */
1414
const AUTOSAVE_DELAY_MS = 1000
1515

16+
/** Static help accessory for the usage-limit header; hoisted so it's a stable reference. */
17+
const USAGE_LIMIT_INFO = (
18+
<Info side='top' className='text-[var(--text-muted)]'>
19+
{
20+
"Max usage to consume per month, set in credits — Sim's usage unit (1,000 credits = $5). By default, it's your plan's included usage, but you can set it beyond."
21+
}
22+
</Info>
23+
)
24+
1625
interface UsageLimitFieldProps {
1726
/** Current monthly usage limit, in dollars. */
1827
currentLimit: number
@@ -111,16 +120,7 @@ export function UsageLimitField({
111120
}, [debouncedDraft, minimumLimit, canEdit, context, organizationId, saveOrgLimit, saveUserLimit])
112121

113122
return (
114-
<SettingsSection
115-
label='Usage limit'
116-
headerAccessory={
117-
<Info side='top' className='text-[var(--text-muted)]'>
118-
{
119-
"Max usage to consume per month, set in credits — Sim's usage unit (1,000 credits = $5). By default, it's your plan's included usage, but you can set it beyond."
120-
}
121-
</Info>
122-
}
123-
>
123+
<SettingsSection label='Usage limit' headerAccessory={USAGE_LIMIT_INFO}>
124124
<ChipInput
125125
type='number'
126126
inputMode='numeric'

apps/sim/app/workspace/[workspaceId]/settings/components/byok/byok-key-manager.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,7 @@ export function BYOKKeyManager(props: BYOKKeyManagerProps) {
299299
strokeWidth={2}
300300
/>
301301
<input
302+
aria-label='Search providers'
302303
placeholder='Search providers...'
303304
value={searchTerm}
304305
onChange={(e) => setSearchTerm(e.target.value)}
@@ -380,6 +381,7 @@ export function BYOKKeyManager(props: BYOKKeyManagerProps) {
380381
type='text'
381382
name='fakeusernameremembered'
382383
autoComplete='username'
384+
aria-hidden='true'
383385
style={{
384386
position: 'absolute',
385387
left: '-9999px',
@@ -391,6 +393,7 @@ export function BYOKKeyManager(props: BYOKKeyManagerProps) {
391393
/>
392394
<div className={CHIP_FIELD_SHELL}>
393395
<input
396+
aria-label='API Key'
394397
type={showApiKey ? 'text' : 'password'}
395398
value={apiKeyInput}
396399
onChange={(e) => {

apps/sim/app/workspace/[workspaceId]/settings/components/byok/byok.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -325,19 +325,18 @@ export function BYOK() {
325325
const workspaceId = (params?.workspaceId as string) || ''
326326

327327
const { data, isLoading } = useBYOKKeys(workspaceId)
328-
const keys = data?.keys ?? []
329328
const upsertKey = useUpsertBYOKKey()
330329
const deleteKey = useDeleteBYOKKey()
331330

332331
const keysByProvider = useMemo(() => {
333332
const grouped = new Map<string, BYOKManagerKey[]>()
334-
for (const key of keys) {
333+
for (const key of data?.keys ?? []) {
335334
const providerKeys = grouped.get(key.providerId) ?? []
336335
providerKeys.push({ id: key.id, name: key.name, maskedKey: key.maskedKey })
337336
grouped.set(key.providerId, providerKeys)
338337
}
339338
return grouped
340-
}, [keys])
339+
}, [data?.keys])
341340

342341
return (
343342
<SettingsPanel>

apps/sim/app/workspace/[workspaceId]/settings/components/copilot/copilot.tsx

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ import {
2727

2828
const logger = createLogger('CopilotSettings')
2929

30+
/** Formats a key's last-used timestamp, falling back to "Never" when unset. */
31+
function formatLastUsed(dateString?: string | null): string {
32+
if (!dateString) return 'Never'
33+
return formatDate(new Date(dateString))
34+
}
35+
3036
/**
3137
* Copilot Keys management component for handling API keys used with the Copilot feature.
3238
* Provides functionality to create, view, and delete copilot API keys.
@@ -95,11 +101,6 @@ export function Copilot() {
95101
}
96102
}
97103

98-
const formatLastUsed = (dateString?: string | null) => {
99-
if (!dateString) return 'Never'
100-
return formatDate(new Date(dateString))
101-
}
102-
103104
const hasKeys = keys.length > 0
104105
const showEmptyState = !hasKeys
105106
const showNoResults = searchTerm.trim() && filteredKeys.length === 0 && keys.length > 0

apps/sim/app/workspace/[workspaceId]/settings/components/credential-sets/credential-sets.tsx

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ import { useSubscriptionData } from '@/hooks/queries/subscription'
5757

5858
const logger = createLogger('EmailPolling')
5959

60+
function getProviderIcon(providerId: string | null) {
61+
if (providerId === 'outlook') return <OutlookIcon className='size-4' />
62+
return <GmailIcon className='size-4' />
63+
}
64+
6065
export function CredentialSets() {
6166
const { data: session } = useSession()
6267
const { data: organizationsData } = useOrganizations()
@@ -240,10 +245,13 @@ export function CredentialSets() {
240245
}
241246
}, [newSetName, newSetDescription, newSetProvider, activeOrganization?.id, createCredentialSet])
242247

243-
const validEmails = useMemo(
244-
() => emailItems.filter((item) => item.isValid).map((item) => item.value),
245-
[emailItems]
246-
)
248+
const validEmails = useMemo(() => {
249+
const result: string[] = []
250+
for (const item of emailItems) {
251+
if (item.isValid) result.push(item.value)
252+
}
253+
return result
254+
}, [emailItems])
247255

248256
const handleInviteMembers = useCallback(async () => {
249257
if (!viewingSet?.id) return
@@ -367,11 +375,6 @@ export function CredentialSets() {
367375
}
368376
}, [deletingSet, activeOrganization?.id, deleteCredentialSet])
369377

370-
const getProviderIcon = (providerId: string | null) => {
371-
if (providerId === 'outlook') return <OutlookIcon className='size-4' />
372-
return <GmailIcon className='size-4' />
373-
}
374-
375378
const activeMemberships = useMemo(
376379
() => memberships.filter((m) => m.status === 'active'),
377380
[memberships]

apps/sim/app/workspace/[workspaceId]/settings/components/general/general.tsx

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import {
1010
ChipModalFooter,
1111
ChipModalHeader,
1212
ChipSelect,
13-
handleKeyboardActivation,
1413
Input,
1514
Label,
1615
Switch,
@@ -293,12 +292,11 @@ export function General() {
293292
<div className='flex flex-col gap-3'>
294293
<div className='flex items-center gap-3'>
295294
<div className='relative'>
296-
<div
297-
role='button'
298-
tabIndex={0}
295+
<button
296+
type='button'
297+
aria-label='Change profile picture'
299298
className={`group relative flex size-9 flex-shrink-0 cursor-pointer items-center justify-center overflow-hidden rounded-full transition-all hover-hover:bg-[var(--bg)] ${!imageUrl ? 'border border-[var(--border)]' : ''}`}
300299
onClick={handleProfilePictureClick}
301-
onKeyDown={(event) => handleKeyboardActivation(event, handleProfilePictureClick)}
302300
>
303301
{(() => {
304302
if (imageUrl) {
@@ -334,7 +332,7 @@ export function General() {
334332
<Camera className='size-4 text-white' />
335333
)}
336334
</div>
337-
</div>
335+
</button>
338336
<Input
339337
type='file'
340338
accept='image/png,image/jpeg,image/jpg'
@@ -357,6 +355,7 @@ export function General() {
357355
</span>
358356
<input
359357
ref={inputRef}
358+
aria-label='Your name'
360359
value={name}
361360
onChange={(e) => setName(e.target.value)}
362361
onKeyDown={handleKeyDown}

apps/sim/app/workspace/[workspaceId]/settings/components/inbox/components/inbox-enable-toggle/inbox-enable-toggle.tsx

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,13 @@ export function InboxEnableToggle() {
2727
const [isDisableOpen, setIsDisableOpen] = useState(false)
2828
const [enableUsername, setEnableUsername] = useState('')
2929

30-
const handleToggle = useCallback(
31-
async (checked: boolean) => {
32-
if (checked) {
33-
setIsEnableOpen(true)
34-
return
35-
}
36-
setIsDisableOpen(true)
37-
},
38-
[workspaceId]
39-
)
30+
const handleToggle = useCallback(async (checked: boolean) => {
31+
if (checked) {
32+
setIsEnableOpen(true)
33+
return
34+
}
35+
setIsDisableOpen(true)
36+
}, [])
4037

4138
const handleDisable = useCallback(async () => {
4239
try {
@@ -45,7 +42,7 @@ export function InboxEnableToggle() {
4542
} catch (error) {
4643
logger.error('Failed to disable inbox', { error })
4744
}
48-
}, [workspaceId])
45+
}, [workspaceId, toggleInbox.mutateAsync])
4946

5047
const handleEnable = useCallback(async () => {
5148
try {
@@ -59,7 +56,7 @@ export function InboxEnableToggle() {
5956
} catch (error) {
6057
logger.error('Failed to enable inbox', { error })
6158
}
62-
}, [workspaceId, enableUsername])
59+
}, [workspaceId, enableUsername, toggleInbox.mutateAsync])
6360

6461
return (
6562
<>

0 commit comments

Comments
 (0)