fix: stabilize React keys in MultiPairInputModal demo#23
Conversation
Rows used `key={\`row-\${index}\`}`, the well-known React anti-pattern
where removing an item from the middle causes React to reuse DOM nodes
across rows. Controlled inputs masked the worst symptom (visible value
stayed in sync because state is rewritten on every render), but
per-DOM-node state — focus, IME composition, autofill, scroll — leaked
between rows after a removal.
Give each row a stable monotonic id (mirroring the React Native demo),
key off `row.id`, and update/remove rows by id rather than by index.
The positional `data-testid` attributes are kept as-is so Appium
selectors unchanged.
Co-authored-by: Cursor <cursoragent@cursor.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM, demo-only change replacing index-based React keys with stable per-row ids — straightforward fix for a well-known anti-pattern.
Extended reasoning...
Overview
This PR touches only two demo files (examples/demo/src/components/modals/MultiPairInputModal.tsx and the identical examples/demo_pods/ copy). It replaces key={\row-${index}`}with stable per-rowidvalues generated by a monotonic counter, and updates the row mutation/removal helpers to match onrow.idinstead of array index.data-testidvalues still use positionalindex` so e2e selectors are unaffected.
Security risks
None. No SDK source is touched, no public API change, no auth/crypto/permission code. The nextRowId counter is module-scoped and only used for React reconciliation — not persisted or exposed.
Level of scrutiny
Low. Demo-app-only change with no production impact, fixing a textbook React anti-pattern. The change pattern is mechanical (index → id) and the two files are kept in sync.
Other factors
The PR mirrors an established approach already used in the React Native demo. The author manually verified focus behavior after row removal, and confirmed data-testid selectors remain stable for existing Appium/e2e tests.
fadi-george
force-pushed
the
main
branch
2 times, most recently
from
bb5f12a to
ae9a23c
Compare
1 participant
Description
One Line Summary
Replace index-based React keys in the demo
MultiPairInputModalwith stable per-row ids so that focus, IME composition, autofill, and other per-DOM-node state stops leaking across rows when a row is removed.Details
Motivation
The demo dialog used
key={\row-${index}`}` for each row, the well-known React anti-pattern. With controlled inputs the visible text stayed correct (state is rewritten on every render), but per-DOM-node state was reused across rows on remove — most visibly: focus, IME composition, autofill suggestions, and scroll position would jump after deleting a middle row, and uncontrolled DOM listeners attached to a removed row's underlying node would now fire on the surviving row that shifted into its slot.This was caught while reviewing the analogous Unity dialog (which has a related but different bug — a name collision in the accessibility bridge). Capacitor doesn't have the Unity collision because
data-testidis recomputed positionally on every render, but the React key issue is a separate latent problem worth cleaning up while the area is fresh.Scope
examples/demo/src/components/modals/MultiPairInputModal.tsxand the identicalexamples/demo_pods/copy.data-testidvalues still use the positionalindex, so existing Appium / e2e selectors targetingmultipair_key_N/multipair_value_Nare unchanged.Notes
Mirrors the React Native demo's approach (
react-native-onesignal/examples/demo/src/components/modals/MultiPairInputModal.tsx), which already uses a monotonicnextId++per row.Testing
Manual testing
data-testidvalues still re-derive from current visible position so e2e selectors are unaffected.Unit testing
n/a — demo-only change.
Affected code checklist
(none — demo apps only, no SDK code touched)
Checklist
Overview
Testing
Final pass
Made with Cursor