diff --git a/apps/web/src/components/settings/AddProviderInstanceDialog.tsx b/apps/web/src/components/settings/AddProviderInstanceDialog.tsx index affa35ff260..8a39b5e80bf 100644 --- a/apps/web/src/components/settings/AddProviderInstanceDialog.tsx +++ b/apps/web/src/components/settings/AddProviderInstanceDialog.tsx @@ -2,7 +2,7 @@ import { CheckIcon } from "lucide-react"; import { Radio as RadioPrimitive } from "@base-ui/react/radio"; -import { useCallback, useEffect, useMemo, useState } from "react"; +import { useCallback, useMemo, useRef, useState } from "react"; import { ProviderInstanceId, ProviderDriverKind, @@ -122,7 +122,7 @@ export function AddProviderInstanceDialog({ open, onOpenChange }: AddProviderIns const [label, setLabel] = useState(""); const [accentColor, setAccentColor] = useState(""); const [instanceId, setInstanceId] = useState(""); - const [instanceIdDirty, setInstanceIdDirty] = useState(false); + const instanceIdDirtyRef = useRef(false); // Driver-specific config drafts keyed by driver so toggling between drivers // during the same dialog session does not lose in-progress input. const [configByDriver, setConfigByDriver] = useState>>({}); @@ -135,26 +135,57 @@ export function AddProviderInstanceDialog({ open, onOpenChange }: AddProviderIns [settings.providerInstances], ); - // Reset the form every time the dialog opens so each creation starts - // from a clean slate. - useEffect(() => { - if (!open) return; + const resetForm = useCallback(() => { setDriver(DEFAULT_DRIVER_KIND); setLabel(""); setAccentColor(""); setInstanceId(""); setWizardStep(0); - setInstanceIdDirty(false); + instanceIdDirtyRef.current = false; setConfigByDriver({}); setHasAttemptedSubmit(false); - }, [open]); + }, []); - // Auto-derive the instance id from driver + label until the user types - // in the Instance ID field directly (after which they own its value). - useEffect(() => { - if (instanceIdDirty) return; - setInstanceId(deriveInstanceId(driver, label)); - }, [driver, label, instanceIdDirty]); + const handleDialogOpenChange = useCallback( + (nextOpen: boolean) => { + if (!nextOpen) { + resetForm(); + } + onOpenChange(nextOpen); + }, + [onOpenChange, resetForm], + ); + + const closeDialog = useCallback(() => { + resetForm(); + onOpenChange(false); + }, [onOpenChange, resetForm]); + + const updateDriver = useCallback( + (value: string) => { + const nextDriver = ProviderDriverKind.make(value); + setDriver(nextDriver); + if (!instanceIdDirtyRef.current) { + setInstanceId(deriveInstanceId(nextDriver, label)); + } + }, + [label], + ); + + const updateLabel = useCallback( + (nextLabel: string) => { + setLabel(nextLabel); + if (!instanceIdDirtyRef.current) { + setInstanceId(deriveInstanceId(driver, nextLabel)); + } + }, + [driver], + ); + + const updateInstanceId = useCallback((nextInstanceId: string) => { + instanceIdDirtyRef.current = true; + setInstanceId(nextInstanceId); + }, []); const driverOption = DRIVER_OPTION_BY_VALUE[driver] ?? DEFAULT_DRIVER_OPTION; const driverSettingsFields = useMemo( @@ -214,7 +245,7 @@ export function AddProviderInstanceDialog({ open, onOpenChange }: AddProviderIns title: "Provider instance added", description: `${driverOption.label} instance '${instanceId}' was added.`, }); - onOpenChange(false); + closeDialog(); } catch (error) { toastManager.add({ type: "error", @@ -230,13 +261,13 @@ export function AddProviderInstanceDialog({ open, onOpenChange }: AddProviderIns instanceIdError, label, accentColor, - onOpenChange, + closeDialog, settings.providerInstances, updateSettings, ]); return ( - +
@@ -301,7 +332,7 @@ export function AddProviderInstanceDialog({ open, onOpenChange }: AddProviderIns setDriver(ProviderDriverKind.make(value))} + onValueChange={updateDriver} aria-labelledby="add-instance-driver-label" className="grid grid-cols-2 gap-2.5" > @@ -365,7 +396,7 @@ export function AddProviderInstanceDialog({ open, onOpenChange }: AddProviderIns className="bg-background" placeholder="e.g. Work" value={label} - onChange={(event) => setLabel(event.target.value)} + onChange={(event) => updateLabel(event.target.value)} /> Shown in the provider list. Optional. @@ -378,10 +409,7 @@ export function AddProviderInstanceDialog({ open, onOpenChange }: AddProviderIns className="bg-background" placeholder={`${driver}_work`} value={instanceId} - onChange={(event) => { - setInstanceIdDirty(true); - setInstanceId(event.target.value); - }} + onChange={(event) => updateInstanceId(event.target.value)} aria-invalid={showInstanceIdError} /> {showInstanceIdError ? ( @@ -466,7 +494,7 @@ export function AddProviderInstanceDialog({ open, onOpenChange }: AddProviderIns size="sm" onClick={() => { if (wizardStep === 0) { - onOpenChange(false); + closeDialog(); return; } setWizardStep((step) => Math.max(0, step - 1)); diff --git a/perf-recordings/react-component-health/README.md b/perf-recordings/react-component-health/README.md new file mode 100644 index 00000000000..aece9a6be0a --- /dev/null +++ b/perf-recordings/react-component-health/README.md @@ -0,0 +1,17 @@ +# React component health recording: provider instance dialog + +This directory contains the react-scan comparison for the provider instance dialog rerender fix. + +- `before-provider-dialog.webm`: recorded from commit `556c42452c5ac94ae8d8698d64fcd372c1680955`. +- `after-provider-dialog.webm`: recorded from commit `be215fb3ea877f46cf7491148b6fdcfa54ab5eae`. + +Capture setup: + +- Vite served the app in hosted-static mode (`VITE_HOSTED_APP_CHANNEL=latest`) so the settings route could load without a backend. +- The page HTML was intercepted by Playwright and injected with `react-scan@0.5.6` before the React bundle. +- The scripted interaction opened Settings > Providers, opened "Add provider instance", advanced to the identity step, typed "Performance Workspace" into the label field, then edited the instance ID. + +Measured during label typing with the React DevTools hook: + +- Before: 34 `AddProviderInstanceDialog` render commits. +- After: 23 `AddProviderInstanceDialog` render commits. diff --git a/perf-recordings/react-component-health/after-provider-dialog.webm b/perf-recordings/react-component-health/after-provider-dialog.webm new file mode 100644 index 00000000000..1d5bf636ac3 Binary files /dev/null and b/perf-recordings/react-component-health/after-provider-dialog.webm differ diff --git a/perf-recordings/react-component-health/before-provider-dialog.webm b/perf-recordings/react-component-health/before-provider-dialog.webm new file mode 100644 index 00000000000..f5310ac9a82 Binary files /dev/null and b/perf-recordings/react-component-health/before-provider-dialog.webm differ