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.
This commit is contained in:
Waleed
2026-07-01 12:06:54 -07:00
committed by GitHub
parent 78661d25ff
commit c957aa2b94
21 changed files with 176 additions and 129 deletions
@@ -24,12 +24,22 @@ import { CreateApiKeyModal } from './components'
const logger = createLogger('ApiKeys')
/** Stable empty references so memoized derivations don't re-run while data loads. */
const EMPTY_KEYS: ApiKey[] = []
const EMPTY_KEY_NAMES: string[] = []
/** Copies an API key's name and confirms with a toast. */
function copyKeyName(name: string) {
void navigator.clipboard.writeText(name)
toast.success('Copied name to clipboard')
}
/** Formats an API key's last-used timestamp, or "Never" when unused. */
function formatLastUsed(dateString?: string | null): string {
if (!dateString) return 'Never'
return formatDate(new Date(dateString))
}
interface ApiKeyRowMenuProps {
keyName: string
onDelete: () => void
@@ -73,9 +83,9 @@ export function ApiKeys() {
const deleteApiKeyMutation = useDeleteApiKey()
const updateSettingsMutation = useUpdateWorkspaceApiKeySettings()
const workspaceKeys = apiKeysData?.workspaceKeys || []
const personalKeys = apiKeysData?.personalKeys || []
const conflicts = apiKeysData?.conflicts || []
const workspaceKeys = apiKeysData?.workspaceKeys ?? EMPTY_KEYS
const personalKeys = apiKeysData?.personalKeys ?? EMPTY_KEYS
const conflicts = apiKeysData?.conflicts ?? EMPTY_KEY_NAMES
const isLoading = isLoadingKeys || isLoadingSettings
const allowPersonalApiKeys =
@@ -90,21 +100,27 @@ export function ApiKeys() {
const createButtonDisabled = isLoading || (!allowPersonalApiKeys && !canManageWorkspaceKeys)
const filteredWorkspaceKeys = useMemo(() => {
if (!searchTerm.trim()) {
return workspaceKeys.map((key, index) => ({ key, originalIndex: index }))
const term = searchTerm.trim().toLowerCase()
const result: { key: ApiKey; originalIndex: number }[] = []
for (let index = 0; index < workspaceKeys.length; index++) {
const key = workspaceKeys[index]
if (term === '' || key.name.toLowerCase().includes(term)) {
result.push({ key, originalIndex: index })
}
}
return workspaceKeys
.map((key, index) => ({ key, originalIndex: index }))
.filter(({ key }) => key.name.toLowerCase().includes(searchTerm.toLowerCase()))
return result
}, [workspaceKeys, searchTerm])
const filteredPersonalKeys = useMemo(() => {
if (!searchTerm.trim()) {
return personalKeys.map((key, index) => ({ key, originalIndex: index }))
const term = searchTerm.trim().toLowerCase()
const result: { key: ApiKey; originalIndex: number }[] = []
for (let index = 0; index < personalKeys.length; index++) {
const key = personalKeys[index]
if (term === '' || key.name.toLowerCase().includes(term)) {
result.push({ key, originalIndex: index })
}
}
return personalKeys
.map((key, index) => ({ key, originalIndex: index }))
.filter(({ key }) => key.name.toLowerCase().includes(searchTerm.toLowerCase()))
return result
}, [personalKeys, searchTerm])
const handleDeleteKey = async () => {
@@ -128,11 +144,6 @@ export function ApiKeys() {
}
}
const formatLastUsed = (dateString?: string | null) => {
if (!dateString) return 'Never'
return formatDate(new Date(dateString))
}
const actions: SettingsAction[] = [
{
text: 'Create API key',
@@ -143,6 +143,7 @@ export function CreateApiKeyModal({
type='text'
name='fakeusernameremembered'
autoComplete='username'
aria-hidden='true'
style={{
position: 'absolute',
left: '-9999px',
@@ -90,12 +90,23 @@ function formatInvoiceDate(createdSeconds: number): string {
})
}
/** Cached currency formatters, keyed by upper-cased ISO currency code. */
const invoiceAmountFormatters = new Map<string, Intl.NumberFormat>()
/** Resolve (and memoize) an `Intl.NumberFormat` for a currency code. */
function getInvoiceAmountFormatter(currency: string): Intl.NumberFormat {
const code = currency.toUpperCase()
let formatter = invoiceAmountFormatters.get(code)
if (!formatter) {
formatter = new Intl.NumberFormat(undefined, { style: 'currency', currency: code })
invoiceAmountFormatters.set(code, formatter)
}
return formatter
}
/** Format a minor-unit (e.g. cents) amount as a localized currency string. */
function formatInvoiceAmount(amountMinor: number, currency: string): string {
return new Intl.NumberFormat(undefined, {
style: 'currency',
currency: currency.toUpperCase(),
}).format(amountMinor / 100)
return getInvoiceAmountFormatter(currency).format(amountMinor / 100)
}
export function Billing() {
@@ -13,6 +13,15 @@ import { useDebounce } from '@/hooks/use-debounce'
/** Delay before a usage-limit edit is auto-saved once the user stops typing. */
const AUTOSAVE_DELAY_MS = 1000
/** Static help accessory for the usage-limit header; hoisted so it's a stable reference. */
const USAGE_LIMIT_INFO = (
<Info side='top' className='text-[var(--text-muted)]'>
{
"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."
}
</Info>
)
interface UsageLimitFieldProps {
/** Current monthly usage limit, in dollars. */
currentLimit: number
@@ -111,16 +120,7 @@ export function UsageLimitField({
}, [debouncedDraft, minimumLimit, canEdit, context, organizationId, saveOrgLimit, saveUserLimit])
return (
<SettingsSection
label='Usage limit'
headerAccessory={
<Info side='top' className='text-[var(--text-muted)]'>
{
"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."
}
</Info>
}
>
<SettingsSection label='Usage limit' headerAccessory={USAGE_LIMIT_INFO}>
<ChipInput
type='number'
inputMode='numeric'
@@ -299,6 +299,7 @@ export function BYOKKeyManager(props: BYOKKeyManagerProps) {
strokeWidth={2}
/>
<input
aria-label='Search providers'
placeholder='Search providers...'
value={searchTerm}
onChange={(e) => setSearchTerm(e.target.value)}
@@ -380,6 +381,7 @@ export function BYOKKeyManager(props: BYOKKeyManagerProps) {
type='text'
name='fakeusernameremembered'
autoComplete='username'
aria-hidden='true'
style={{
position: 'absolute',
left: '-9999px',
@@ -391,6 +393,7 @@ export function BYOKKeyManager(props: BYOKKeyManagerProps) {
/>
<div className={CHIP_FIELD_SHELL}>
<input
aria-label='API Key'
type={showApiKey ? 'text' : 'password'}
value={apiKeyInput}
onChange={(e) => {
@@ -325,19 +325,18 @@ export function BYOK() {
const workspaceId = (params?.workspaceId as string) || ''
const { data, isLoading } = useBYOKKeys(workspaceId)
const keys = data?.keys ?? []
const upsertKey = useUpsertBYOKKey()
const deleteKey = useDeleteBYOKKey()
const keysByProvider = useMemo(() => {
const grouped = new Map<string, BYOKManagerKey[]>()
for (const key of keys) {
for (const key of data?.keys ?? []) {
const providerKeys = grouped.get(key.providerId) ?? []
providerKeys.push({ id: key.id, name: key.name, maskedKey: key.maskedKey })
grouped.set(key.providerId, providerKeys)
}
return grouped
}, [keys])
}, [data?.keys])
return (
<SettingsPanel>
@@ -27,6 +27,12 @@ import {
const logger = createLogger('CopilotSettings')
/** Formats a key's last-used timestamp, falling back to "Never" when unset. */
function formatLastUsed(dateString?: string | null): string {
if (!dateString) return 'Never'
return formatDate(new Date(dateString))
}
/**
* Copilot Keys management component for handling API keys used with the Copilot feature.
* Provides functionality to create, view, and delete copilot API keys.
@@ -95,11 +101,6 @@ export function Copilot() {
}
}
const formatLastUsed = (dateString?: string | null) => {
if (!dateString) return 'Never'
return formatDate(new Date(dateString))
}
const hasKeys = keys.length > 0
const showEmptyState = !hasKeys
const showNoResults = searchTerm.trim() && filteredKeys.length === 0 && keys.length > 0
@@ -57,6 +57,11 @@ import { useSubscriptionData } from '@/hooks/queries/subscription'
const logger = createLogger('EmailPolling')
function getProviderIcon(providerId: string | null) {
if (providerId === 'outlook') return <OutlookIcon className='size-4' />
return <GmailIcon className='size-4' />
}
export function CredentialSets() {
const { data: session } = useSession()
const { data: organizationsData } = useOrganizations()
@@ -240,10 +245,13 @@ export function CredentialSets() {
}
}, [newSetName, newSetDescription, newSetProvider, activeOrganization?.id, createCredentialSet])
const validEmails = useMemo(
() => emailItems.filter((item) => item.isValid).map((item) => item.value),
[emailItems]
)
const validEmails = useMemo(() => {
const result: string[] = []
for (const item of emailItems) {
if (item.isValid) result.push(item.value)
}
return result
}, [emailItems])
const handleInviteMembers = useCallback(async () => {
if (!viewingSet?.id) return
@@ -367,11 +375,6 @@ export function CredentialSets() {
}
}, [deletingSet, activeOrganization?.id, deleteCredentialSet])
const getProviderIcon = (providerId: string | null) => {
if (providerId === 'outlook') return <OutlookIcon className='size-4' />
return <GmailIcon className='size-4' />
}
const activeMemberships = useMemo(
() => memberships.filter((m) => m.status === 'active'),
[memberships]
@@ -10,7 +10,6 @@ import {
ChipModalFooter,
ChipModalHeader,
ChipSelect,
handleKeyboardActivation,
Input,
Label,
Switch,
@@ -293,12 +292,11 @@ export function General() {
<div className='flex flex-col gap-3'>
<div className='flex items-center gap-3'>
<div className='relative'>
<div
role='button'
tabIndex={0}
<button
type='button'
aria-label='Change profile picture'
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)]' : ''}`}
onClick={handleProfilePictureClick}
onKeyDown={(event) => handleKeyboardActivation(event, handleProfilePictureClick)}
>
{(() => {
if (imageUrl) {
@@ -334,7 +332,7 @@ export function General() {
<Camera className='size-4 text-white' />
)}
</div>
</div>
</button>
<Input
type='file'
accept='image/png,image/jpeg,image/jpg'
@@ -357,6 +355,7 @@ export function General() {
</span>
<input
ref={inputRef}
aria-label='Your name'
value={name}
onChange={(e) => setName(e.target.value)}
onKeyDown={handleKeyDown}
@@ -27,16 +27,13 @@ export function InboxEnableToggle() {
const [isDisableOpen, setIsDisableOpen] = useState(false)
const [enableUsername, setEnableUsername] = useState('')
const handleToggle = useCallback(
async (checked: boolean) => {
if (checked) {
setIsEnableOpen(true)
return
}
setIsDisableOpen(true)
},
[workspaceId]
)
const handleToggle = useCallback(async (checked: boolean) => {
if (checked) {
setIsEnableOpen(true)
return
}
setIsDisableOpen(true)
}, [])
const handleDisable = useCallback(async () => {
try {
@@ -45,7 +42,7 @@ export function InboxEnableToggle() {
} catch (error) {
logger.error('Failed to disable inbox', { error })
}
}, [workspaceId])
}, [workspaceId, toggleInbox.mutateAsync])
const handleEnable = useCallback(async () => {
try {
@@ -59,7 +56,7 @@ export function InboxEnableToggle() {
} catch (error) {
logger.error('Failed to enable inbox', { error })
}
}, [workspaceId, enableUsername])
}, [workspaceId, enableUsername, toggleInbox.mutateAsync])
return (
<>
@@ -65,7 +65,7 @@ export function InboxSettingsTab() {
} catch (error) {
setEditAddressError(getErrorMessage(error, 'Failed to update address'))
}
}, [workspaceId, newUsername])
}, [workspaceId, newUsername, updateAddress.mutateAsync])
const handleAddSender = useCallback(async () => {
if (!newSenderEmail.trim()) return
@@ -82,7 +82,7 @@ export function InboxSettingsTab() {
} catch (error) {
setAddSenderError(getErrorMessage(error, 'Failed to add sender'))
}
}, [workspaceId, newSenderEmail, newSenderLabel])
}, [workspaceId, newSenderEmail, newSenderLabel, addSender.mutateAsync])
const handleRemoveSender = useCallback(
async (senderId: string) => {
@@ -93,7 +93,7 @@ export function InboxSettingsTab() {
setRemoveSenderError(getErrorMessage(error, 'Failed to remove sender'))
}
},
[workspaceId]
[workspaceId, removeSender.mutateAsync]
)
return (
@@ -133,25 +133,13 @@ export function InboxTaskList() {
const statusBadge = STATUS_BADGES[task.status] || STATUS_BADGES.received
const isClickable =
task.chatId && (task.status === 'completed' || task.status === 'failed')
return (
<div
key={task.id}
className={`flex items-center gap-2.5 rounded-lg p-2 text-left transition-colors ${
isClickable
? 'cursor-pointer hover-hover:bg-[var(--surface-active)]'
: 'cursor-default'
}`}
role='button'
aria-disabled={!isClickable}
tabIndex={isClickable ? 0 : undefined}
onClick={() => handleTaskClick(task)}
onKeyDown={(e) => {
if (isClickable && (e.key === 'Enter' || e.key === ' ')) {
e.preventDefault()
handleTaskClick(task)
}
}}
>
const rowClassName = `flex w-full items-center gap-2.5 rounded-lg p-2 text-left transition-colors ${
isClickable
? 'cursor-pointer hover-hover:bg-[var(--surface-active)]'
: 'cursor-default'
}`
const rowContent = (
<>
<div className='flex min-w-0 flex-1 flex-col'>
<div className='flex min-w-0 items-center gap-1.5'>
<span className='truncate text-[14px] text-[var(--text-body)]'>
@@ -202,6 +190,21 @@ export function InboxTaskList() {
<ArrowRight className='size-4 flex-shrink-0 text-[var(--text-icon)]' />
)}
</div>
</>
)
return isClickable ? (
<button
key={task.id}
type='button'
className={rowClassName}
onClick={() => handleTaskClick(task)}
>
{rowContent}
</button>
) : (
<div key={task.id} className={rowClassName}>
{rowContent}
</div>
)
})}
@@ -672,6 +672,7 @@ export function McpServerFormModal({
type='text'
name='fakeusernameremembered'
autoComplete='username'
aria-hidden='true'
style={{ position: 'absolute', left: '-9999px', opacity: 0, pointerEvents: 'none' }}
tabIndex={-1}
readOnly
@@ -680,6 +681,7 @@ export function McpServerFormModal({
type='password'
name='fakepasswordremembered'
autoComplete='current-password'
aria-hidden='true'
style={{ position: 'absolute', left: '-9999px', opacity: 0, pointerEvents: 'none' }}
tabIndex={-1}
readOnly
@@ -303,7 +303,7 @@ export function RecentlyDeleted() {
}
const col = (activeSort ?? DEFAULT_SORT).column
const dir = (activeSort ?? DEFAULT_SORT).direction
items = [...items].sort((a, b) => {
items.sort((a, b) => {
let cmp = 0
switch (col) {
case 'name':
@@ -173,10 +173,14 @@ function parseEnvVarLine(line: string): UIEnvironmentVariable | null {
/** Parses an array of raw text lines, returning only valid non-empty KEY=VALUE entries. */
function parseValidEnvVars(lines: string[]): UIEnvironmentVariable[] {
return lines
.map(parseEnvVarLine)
.filter((parsed): parsed is UIEnvironmentVariable => parsed !== null)
.filter(({ key, value }) => key && value)
const result: UIEnvironmentVariable[] = []
for (const line of lines) {
const parsed = parseEnvVarLine(line)
if (parsed?.key && parsed.value) {
result.push(parsed)
}
}
return result
}
interface WorkspaceVariableRowProps {
@@ -771,9 +775,10 @@ export function SecretsManager() {
}
const personalChanged = (() => {
const initialMap = new Map(
initialVarsRef.current.filter((v) => v.key && v.value).map((v) => [v.key, v.value])
)
const initialMap = new Map<string, string>()
for (const v of initialVarsRef.current) {
if (v.key && v.value) initialMap.set(v.key, v.value)
}
const currentKeys = Object.keys(validVariables)
if (initialMap.size !== currentKeys.length) return true
for (const [key, value] of Object.entries(validVariables)) {
@@ -912,13 +917,14 @@ export function SecretsManager() {
return (
<>
<div className='hidden'>
<div className='hidden' aria-hidden='true'>
<input
type='text'
name='fakeusernameremembered'
autoComplete='username'
tabIndex={-1}
readOnly
aria-hidden='true'
/>
<input
type='password'
@@ -926,6 +932,7 @@ export function SecretsManager() {
autoComplete='current-password'
tabIndex={-1}
readOnly
aria-hidden='true'
/>
<input
type='email'
@@ -933,6 +940,7 @@ export function SecretsManager() {
autoComplete='email'
tabIndex={-1}
readOnly
aria-hidden='true'
/>
</div>
@@ -65,6 +65,8 @@ export function NoOrganizationView({
style={{ position: 'absolute', left: '-9999px', opacity: 0, pointerEvents: 'none' }}
tabIndex={-1}
readOnly
aria-hidden='true'
aria-label='Ignore this field'
/>
<div>
<Label htmlFor='team-name-field' className='font-medium text-small'>
@@ -137,6 +139,8 @@ export function NoOrganizationView({
style={{ position: 'absolute', left: '-9999px', opacity: 0, pointerEvents: 'none' }}
tabIndex={-1}
readOnly
aria-hidden='true'
aria-label='Ignore this field'
/>
<ChipModalField
type='input'
@@ -25,6 +25,8 @@ const ROLE_OPTIONS = [
{ value: 'read', label: 'Read' },
] as const
const EMPTY_EMAILS: string[] = []
interface OrganizationInviteModalProps {
open: boolean
onOpenChange: (open: boolean) => void
@@ -54,8 +56,8 @@ export function OrganizationInviteModal({
onOpenChange,
organizationId,
workspaces,
externalEmails = [],
pendingEmails = [],
externalEmails = EMPTY_EMAILS,
pendingEmails = EMPTY_EMAILS,
}: OrganizationInviteModalProps) {
const [emails, setEmails] = useState<string[]>([])
const [selectedWorkspaceIds, setSelectedWorkspaceIds] = useState<string[]>([])
@@ -66,6 +66,10 @@ function copyToClipboard(text: string) {
void navigator.clipboard.writeText(text)
}
function buildActionsMenu(actions: RowAction[]) {
return <RowActionsMenu label='Member actions' actions={actions} />
}
interface OrganizationMemberListsProps {
organizationId: string
roster: OrganizationRoster | null | undefined
@@ -109,10 +113,6 @@ export function OrganizationMemberLists({
const isActiveSearch = q.length > 0
const buildActionsMenu = (actions: RowAction[]) => (
<RowActionsMenu label='Member actions' actions={actions} />
)
const renderOrgMemberRow = (member: RosterMember) => {
const isSelf = member.userId === currentUserId
const isOwner = member.role === 'owner'
@@ -96,13 +96,13 @@ export function TeamManagement() {
}
: null
const externalEmails = useMemo(
() =>
(roster?.members ?? [])
.filter((member) => member.role === 'external')
.map((member) => member.email),
[roster]
)
const externalEmails = useMemo(() => {
const emails: string[] = []
for (const member of roster?.members ?? []) {
if (member.role === 'external') emails.push(member.email)
}
return emails
}, [roster])
/**
* Pending invitations for emails that already belong to a member are
@@ -111,14 +111,15 @@ export function TeamManagement() {
* blocked in the invite modal.
*/
const pendingEmails = useMemo(() => {
const memberEmailSet = new Set(
(roster?.members ?? [])
.filter((member) => member.role !== 'external')
.map((member) => member.email.toLowerCase())
)
return (roster?.pendingInvitations ?? [])
.map((invitation) => invitation.email)
.filter((email) => !memberEmailSet.has(email.toLowerCase()))
const memberEmailSet = new Set<string>()
for (const member of roster?.members ?? []) {
if (member.role !== 'external') memberEmailSet.add(member.email.toLowerCase())
}
const emails: string[] = []
for (const invitation of roster?.pendingInvitations ?? []) {
if (!memberEmailSet.has(invitation.email.toLowerCase())) emails.push(invitation.email)
}
return emails
}, [roster])
useEffect(() => {
@@ -150,7 +151,7 @@ export function TeamManagement() {
} catch (error) {
logger.error('Failed to create organization', error)
}
}, [orgName, orgSlug, createOrgMutation])
}, [orgName, orgSlug, createOrgMutation, session?.user])
const handleRemoveMember = useCallback(
async (member: Member) => {
@@ -1,6 +1,6 @@
'use client'
import { useCallback, useEffect, useState } from 'react'
import { useCallback, useState } from 'react'
import {
ButtonGroup,
ButtonGroupItem,
@@ -44,12 +44,14 @@ export function CreateWorkflowMcpServerModal({
const isFormValid = formData.name.trim().length > 0
useEffect(() => {
if (open) {
setFormData({ ...INITIAL_FORM_DATA })
setSelectedWorkflowIds([])
}
}, [open])
const [prevOpen, setPrevOpen] = useState(false)
if (open && !prevOpen) {
setFormData({ ...INITIAL_FORM_DATA })
setSelectedWorkflowIds([])
}
if (open !== prevOpen) {
setPrevOpen(open)
}
const handleCreateServer = useCallback(async () => {
if (!formData.name.trim()) return
@@ -66,7 +68,7 @@ export function CreateWorkflowMcpServerModal({
} catch (err) {
logger.error('Failed to create server:', err)
}
}, [formData, selectedWorkflowIds, workspaceId, onOpenChange])
}, [formData, selectedWorkflowIds, workspaceId, onOpenChange, createServerMutation.mutateAsync])
const showWorkflows = workflowOptions !== undefined
@@ -217,7 +217,7 @@ function ServerDetailView({ workspaceId, serverId, onBack }: ServerDetailViewPro
return !descriptionChanged && !paramDescriptionsChanged
})()
const tools = data?.tools ?? []
const tools = useMemo(() => data?.tools ?? [], [data?.tools])
const availableWorkflows = useMemo(() => {
const existingWorkflowIds = new Set(tools.map((t) => t.workflowId))