From 51a46d3b87f0bb398dbb62fd35ee36312e710dcc Mon Sep 17 00:00:00 2001 From: sudo-vaibhav <53619134+sudo-vaibhav@users.noreply.github.com> Date: Tue, 23 Jun 2026 14:19:21 +0530 Subject: [PATCH] fix(frontend): preserve input focus across reruns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The renderer keyed every component by its value (`key={props.tree.value}` in TreeRender), so any committed value change unmounted and remounted the widget — dropping DOM focus, the text caret, and selection mid-interaction on every rerun. This was visible in Backroad-based apps (e.g. code-bot) where editing one field or nudging a slider yanked focus away. Key components by their stable `id` instead (the parent already keys the subtree by `path`, so the value-key only ever forced remounts). To keep server-driven value updates visible without a remount, components now sync the server value in place via a new `useSyncedState` hook (React's "adjust state on prop change" pattern, guarded so it neither loops nor clobbers in-progress local edits). The two react-select widgets move from uncontrolled `defaultValue` to a controlled, synced value for the same reason. Adds an e2e regression guard: a slider must keep keyboard focus across each commit-driven rerun (repeated arrow nudges only register if focus survives the remount). Verified it fails on the old value-keyed renderer. Co-Authored-By: Claude Opus 4.8 --- e2e/focus-retention.spec.ts | 61 +++++++++++++++++++ .../src/lib/components/checkbox.tsx | 4 +- .../src/lib/components/color_picker.tsx | 3 +- .../src/lib/components/date_input.tsx | 4 +- .../src/lib/components/multiselect.tsx | 24 ++++---- .../src/lib/components/number_input.tsx | 4 +- .../src/lib/components/radio.tsx | 4 +- .../src/lib/components/select.tsx | 16 ++++- .../src/lib/components/slider.tsx | 4 +- .../src/lib/components/text_area.tsx | 4 +- .../src/lib/components/text_input.tsx | 4 +- .../src/lib/components/time_input.tsx | 4 +- .../src/lib/components/toggle.tsx | 4 +- .../src/lib/hooks/use-synced-state.ts | 35 +++++++++++ libs/backroad-components/src/lib/tree.tsx | 9 ++- 15 files changed, 150 insertions(+), 34 deletions(-) create mode 100644 e2e/focus-retention.spec.ts create mode 100644 libs/backroad-components/src/lib/hooks/use-synced-state.ts diff --git a/e2e/focus-retention.spec.ts b/e2e/focus-retention.spec.ts new file mode 100644 index 0000000..d64cbb3 --- /dev/null +++ b/e2e/focus-retention.spec.ts @@ -0,0 +1,61 @@ +import { expect, test } from '@playwright/test'; + +// Runs under the `chromium` project (already authenticated via the saved +// storage state). +// +// Regression guard for focus survival across a rerun. The renderer used to key +// every component by its value, so any committed value change unmounted and +// remounted the widget — dropping DOM focus, the text caret, and selection +// mid-interaction. Components are now keyed by their stable `id` and sync the +// server value in place (useSyncedState), so the node survives a value change. +// +// The Volume slider is the sharpest probe: a keyboard nudge commits a new value +// and reruns the script. If that rerun remounts the slider, it loses focus and +// the NEXT arrow key goes nowhere. So we focus once and then drive every nudge +// through `page.keyboard` (which targets whatever is currently focused, and does +// NOT re-focus the element the way `locator.press` would — re-focusing would +// mask the very bug under test). Each subsequent increment landing proves focus +// was retained across the previous rerun. + +test.describe('focus survives a rerun', () => { + test.beforeEach(async ({ page }) => { + await page.goto('/widgets'); + await expect(page.getByRole('heading', { name: /^Widgets$/ })).toBeVisible({ + timeout: 10_000, + }); + }); + + test('a slider keeps keyboard focus across each commit-driven rerun', async ({ + page, + }) => { + const slider = page.getByRole('slider', { name: 'Volume' }); + await expect(page.getByText('Volume is 30')).toBeVisible(); + + await slider.focus(); + await expect(slider).toBeFocused(); + + // First nudge: commit + rerun. Under the old value-keyed renderer the slider + // remounts here and focus is lost. + await page.keyboard.press('ArrowRight'); + await expect(page.getByText('Volume is 31')).toBeVisible({ + timeout: 10_000, + }); + // The node must still be the focused one for the rest of the test to work. + await expect(slider).toBeFocused(); + + // These two only register if focus survived the reruns above — each waits + // for the script echo, so the prior rerun has fully landed (and, under the + // old behaviour, would already have remounted) before the next key. + await page.keyboard.press('ArrowRight'); + await expect(page.getByText('Volume is 32')).toBeVisible({ + timeout: 10_000, + }); + + await page.keyboard.press('ArrowRight'); + await expect(page.getByText('Volume is 33')).toBeVisible({ + timeout: 10_000, + }); + + await expect(slider).toBeFocused(); + }); +}); diff --git a/libs/backroad-components/src/lib/components/checkbox.tsx b/libs/backroad-components/src/lib/components/checkbox.tsx index e0623b0..f65dc03 100644 --- a/libs/backroad-components/src/lib/components/checkbox.tsx +++ b/libs/backroad-components/src/lib/components/checkbox.tsx @@ -1,10 +1,10 @@ -import { useState } from 'react'; +import { useSyncedState } from '../hooks/use-synced-state'; import { BackroadComponentRenderer } from '../types/components'; import { setBackroadValue } from '../socket'; import { Checkbox as UICheckbox, Label } from 'backroad-ui'; export const Checkbox: BackroadComponentRenderer<'checkbox'> = (props) => { - const [value, setValue] = useState(props.value); + const [value, setValue] = useSyncedState(props.value); return (
= ( props ) => { const popover = useRef(null); - const [value, setValue] = useState(props.value || undefined); + const [value, setValue] = useSyncedState(props.value || undefined); const [isOpen, setIsOpen] = useState(false); const close = useCallback(() => setIsOpen(false), []); diff --git a/libs/backroad-components/src/lib/components/date_input.tsx b/libs/backroad-components/src/lib/components/date_input.tsx index c08e0ce..474bfb2 100644 --- a/libs/backroad-components/src/lib/components/date_input.tsx +++ b/libs/backroad-components/src/lib/components/date_input.tsx @@ -1,4 +1,4 @@ -import { useState } from 'react'; +import { useSyncedState } from '../hooks/use-synced-state'; import { BackroadComponentRenderer } from '../types/components'; import { setBackroadValue } from '../socket'; import { Input, Label } from 'backroad-ui'; @@ -6,7 +6,7 @@ import { Input, Label } from 'backroad-ui'; // Native date picker. `change` fires once per selection (not per keystroke), // so committing directly is cheap. Value is an ISO `YYYY-MM-DD` string. export const DateInput: BackroadComponentRenderer<'date_input'> = (props) => { - const [value, setValue] = useState(props.value); + const [value, setValue] = useSyncedState(props.value); return (
diff --git a/libs/backroad-components/src/lib/components/multiselect.tsx b/libs/backroad-components/src/lib/components/multiselect.tsx index 20a6b7a..cd8f150 100644 --- a/libs/backroad-components/src/lib/components/multiselect.tsx +++ b/libs/backroad-components/src/lib/components/multiselect.tsx @@ -2,17 +2,21 @@ import ReactSelect from 'react-select'; import { getFlattenedOptions, reactSelectClassNames } from '../helpers/select'; import { setBackroadValue } from '../socket'; import { BackroadComponentRenderer } from '../types/components'; -import { useState } from 'react'; -import { SelectOptionType } from '@backroad/core'; +import { useSyncedState } from '../hooks/use-synced-state'; import { Label } from 'backroad-ui'; export const Multiselect: BackroadComponentRenderer<'multiselect'> = ( props ) => { const flattenedOptions = getFlattenedOptions(props.args.options); + // Controlled (was uncontrolled `defaultValue`): the renderer no longer + // remounts this on a value change. Track the selected VALUES — which keep a + // stable identity across local re-renders — and derive the option objects + // react-select wants from them each render. Syncing on the freshly-filtered + // option array instead would make useSyncedState re-fire every render. + const [selectedValues, setSelectedValues] = useSyncedState(props.value); const valueOptions = flattenedOptions.filter((option) => - props.value?.includes(option.value) - ) as Readonly; - const [value, setValue] = useState(valueOptions); + selectedValues?.includes(option.value) + ); return (
@@ -21,14 +25,12 @@ export const Multiselect: BackroadComponentRenderer<'multiselect'> = ( inputId={props.id} unstyled classNames={reactSelectClassNames} - defaultValue={value} + value={valueOptions} isMulti onChange={(newValue) => { - setValue(newValue); - setBackroadValue({ - id: props.id, - value: newValue?.map((option) => option.value), - }); + const values = newValue.map((option) => option.value); + setSelectedValues(values); + setBackroadValue({ id: props.id, value: values }); }} />
diff --git a/libs/backroad-components/src/lib/components/number_input.tsx b/libs/backroad-components/src/lib/components/number_input.tsx index 85f4106..c733440 100644 --- a/libs/backroad-components/src/lib/components/number_input.tsx +++ b/libs/backroad-components/src/lib/components/number_input.tsx @@ -1,12 +1,12 @@ import { BackroadComponent } from '@backroad/core'; -import { useState } from 'react'; +import { useSyncedState } from '../hooks/use-synced-state'; import { setBackroadValue } from '../socket'; import { handleKeyUpBlur } from '../helpers/handleKeyUp'; import { Minus, Plus } from 'lucide-react'; import { Input, Label, Button } from 'backroad-ui'; export const NumberInput = (props: BackroadComponent<'number_input', true>) => { - const [inputValue, setInputValue] = useState(props.value); + const [inputValue, setInputValue] = useSyncedState(props.value); const stepValue = props.args.step || 1; const precisionValue = props.args.precision || 0; diff --git a/libs/backroad-components/src/lib/components/radio.tsx b/libs/backroad-components/src/lib/components/radio.tsx index 448ea0c..2c36ce6 100644 --- a/libs/backroad-components/src/lib/components/radio.tsx +++ b/libs/backroad-components/src/lib/components/radio.tsx @@ -1,10 +1,10 @@ -import { useState } from 'react'; +import { useSyncedState } from '../hooks/use-synced-state'; import { BackroadComponentRenderer } from '../types/components'; import { setBackroadValue } from '../socket'; import { RadioGroup, RadioGroupItem, Label } from 'backroad-ui'; export const Radio: BackroadComponentRenderer<'radio'> = (props) => { - const [value, setValue] = useState(props.value); + const [value, setValue] = useSyncedState(props.value); return (
{props.args.label} diff --git a/libs/backroad-components/src/lib/components/select.tsx b/libs/backroad-components/src/lib/components/select.tsx index 91bff7f..556fb83 100644 --- a/libs/backroad-components/src/lib/components/select.tsx +++ b/libs/backroad-components/src/lib/components/select.tsx @@ -2,9 +2,20 @@ import ReactSelect from 'react-select'; import { getFlattenedOptions, reactSelectClassNames } from '../helpers/select'; import { setBackroadValue } from '../socket'; import { BackroadComponentRenderer } from '../types/components'; +import { useSyncedState } from '../hooks/use-synced-state'; import { Label } from 'backroad-ui'; export const Select: BackroadComponentRenderer<'select'> = (props) => { + // Controlled (was uncontrolled `defaultValue`): the renderer no longer + // remounts this on a value change, so the selection has to be driven by + // state. Seed + resync from the server value via useSyncedState, and update + // optimistically on change so the menu reflects the pick before the rerun + // round-trips. + const [value, setValue] = useSyncedState( + getFlattenedOptions(props.args.options).find( + (option) => option.value === props.value + ) ?? null + ); return (
@@ -13,10 +24,9 @@ export const Select: BackroadComponentRenderer<'select'> = (props) => { inputId={props.id} unstyled classNames={reactSelectClassNames} - defaultValue={getFlattenedOptions(props.args.options).find( - (option) => option.value === props.value - )} + value={value} onChange={(newValue) => { + setValue(newValue ?? null); setBackroadValue({ id: props.id, value: newValue?.value }); }} /> diff --git a/libs/backroad-components/src/lib/components/slider.tsx b/libs/backroad-components/src/lib/components/slider.tsx index 459c81f..818d0e1 100644 --- a/libs/backroad-components/src/lib/components/slider.tsx +++ b/libs/backroad-components/src/lib/components/slider.tsx @@ -1,4 +1,4 @@ -import { useState } from 'react'; +import { useSyncedState } from '../hooks/use-synced-state'; import { BackroadComponentRenderer } from '../types/components'; import { setBackroadValue } from '../socket'; import { Label, Slider as SliderPrimitive } from 'backroad-ui'; @@ -10,7 +10,7 @@ import { Label, Slider as SliderPrimitive } from 'backroad-ui'; // the thumb for sighted users. export const Slider: BackroadComponentRenderer<'slider'> = (props) => { const { label, min = 0, max = 100, step = 1 } = props.args; - const [value, setValue] = useState(props.value); + const [value, setValue] = useSyncedState(props.value); return (
diff --git a/libs/backroad-components/src/lib/components/text_area.tsx b/libs/backroad-components/src/lib/components/text_area.tsx index ca8751b..51885d4 100644 --- a/libs/backroad-components/src/lib/components/text_area.tsx +++ b/libs/backroad-components/src/lib/components/text_area.tsx @@ -1,4 +1,4 @@ -import { useState } from 'react'; +import { useSyncedState } from '../hooks/use-synced-state'; import { BackroadComponentRenderer } from '../types/components'; import { setBackroadValue } from '../socket'; import { Label, Textarea } from 'backroad-ui'; @@ -6,7 +6,7 @@ import { Label, Textarea } from 'backroad-ui'; // Unlike text_input, Enter must insert a newline rather than commit, so there's // no handleKeyUpBlur here — the value commits on blur only. export const TextArea: BackroadComponentRenderer<'text_area'> = (props) => { - const [value, setValue] = useState(props.value); + const [value, setValue] = useSyncedState(props.value); return (
diff --git a/libs/backroad-components/src/lib/components/text_input.tsx b/libs/backroad-components/src/lib/components/text_input.tsx index f9bf4a7..732b0e2 100644 --- a/libs/backroad-components/src/lib/components/text_input.tsx +++ b/libs/backroad-components/src/lib/components/text_input.tsx @@ -1,11 +1,11 @@ -import { useState } from 'react'; +import { useSyncedState } from '../hooks/use-synced-state'; import { BackroadComponentRenderer } from '../types/components'; import { handleKeyUpBlur } from '../helpers/handleKeyUp'; import { setBackroadValue } from '../socket'; import { Input, Label } from 'backroad-ui'; export const TextInput: BackroadComponentRenderer<'text_input'> = (props) => { - const [value, setValue] = useState(props.value); + const [value, setValue] = useSyncedState(props.value); return (
diff --git a/libs/backroad-components/src/lib/components/time_input.tsx b/libs/backroad-components/src/lib/components/time_input.tsx index 270087f..eb9670d 100644 --- a/libs/backroad-components/src/lib/components/time_input.tsx +++ b/libs/backroad-components/src/lib/components/time_input.tsx @@ -1,4 +1,4 @@ -import { useState } from 'react'; +import { useSyncedState } from '../hooks/use-synced-state'; import { BackroadComponentRenderer } from '../types/components'; import { setBackroadValue } from '../socket'; import { Input, Label } from 'backroad-ui'; @@ -6,7 +6,7 @@ import { Input, Label } from 'backroad-ui'; // Native time picker. Value is a 24-hour `HH:mm` string. `step` (seconds) // switches the picker to second-level granularity when provided. export const TimeInput: BackroadComponentRenderer<'time_input'> = (props) => { - const [value, setValue] = useState(props.value); + const [value, setValue] = useSyncedState(props.value); return (
diff --git a/libs/backroad-components/src/lib/components/toggle.tsx b/libs/backroad-components/src/lib/components/toggle.tsx index 965ca3c..7cc7fff 100644 --- a/libs/backroad-components/src/lib/components/toggle.tsx +++ b/libs/backroad-components/src/lib/components/toggle.tsx @@ -1,10 +1,10 @@ -import { useState } from 'react'; +import { useSyncedState } from '../hooks/use-synced-state'; import { BackroadComponentRenderer } from '../types/components'; import { setBackroadValue } from '../socket'; import { Switch, Label } from 'backroad-ui'; export const Toggle: BackroadComponentRenderer<'toggle'> = (props) => { - const [value, setValue] = useState(props.value); + const [value, setValue] = useSyncedState(props.value); return (
( + serverValue: T +): [T, Dispatch>] { + const [value, setValue] = useState(serverValue); + const lastServerValue = useRef(serverValue); + // `Object.is` guard: `serverValue` only gets a new identity when a render + // patch replaces this node (i.e. a genuine server-side change), not on the + // re-renders triggered by local `setValue`, so this neither loops nor clobbers + // local edits. + if (!Object.is(serverValue, lastServerValue.current)) { + lastServerValue.current = serverValue; + setValue(serverValue); + } + return [value, setValue]; +} diff --git a/libs/backroad-components/src/lib/tree.tsx b/libs/backroad-components/src/lib/tree.tsx index e792241..cfdcd15 100644 --- a/libs/backroad-components/src/lib/tree.tsx +++ b/libs/backroad-components/src/lib/tree.tsx @@ -7,10 +7,17 @@ export const TreeRender = (props: { tree: BackroadNode }) => { if (isBackroadComponent(props.tree, true)) { const ComponentRenderer = backroadClientComponents[props.tree.type]; // Suspense boundary for lazy renderers (e.g. `table`); inert for the rest. + // Key by the component's stable `id`, NOT its value: the parent already + // keys this whole subtree by `path`, so keying on value here only served to + // force a remount whenever the value changed — which dropped focus, the text + // caret, and selection mid-edit on every rerun. Value-driven UI updates are + // handled inside the components via `useSyncedState` instead, so the DOM + // node now survives a value change and only remounts when the component's + // identity actually changes. return ( {/* @ts-expect-error there are sufficient checks to ensure this is correct */} - + ); } else {