[otp field] Fix vertical arrow slot navigation

[atomiks]

Pressing ArrowUp or ArrowDown in an OTP slot could collapse the slot selection, so a later typed character could be blocked by the one-character input length.

Root cause

Vertical arrows fell through to native text-input caret handling instead of being handled as OTP field navigation.

Changes

• Map ArrowUp to the first slot, matching Home.
• Map ArrowDown to the last filled slot when the current slot has a character, matching End.
• Keep focus in place when pressing ArrowDown on an empty slot.
• Only advance same-character typing when another slot exists.
• Add regression coverage for vertical arrow navigation and final-slot same-character handling.

[pkg-pr-new]

• base-ui-tanstack-start

• vite-css-base-ui-example

  pnpm add https://pkg.pr.new/mui/base-ui/@base-ui/react@4844
  

  pnpm add https://pkg.pr.new/mui/base-ui/@base-ui/utils@4844
  

commit: 36299db

[code-infra-dashboard]

Bundle size

Bundle	Parsed size	Gzip size
@base-ui/react	🔺+47B(+0.01%)	🔺+21B(+0.01%)

Details of bundle changes

Performance

Total duration: 934.37 ms ▼-352.64 ms^(-27.4%) | Renders: 50 ⁽⁺⁰⁾ | Paint: 1,391.57 ms ▼-570.29 ms^(-29.1%)

Test	Duration	Renders
Tabs mount (200 instances)	183.22 ms ▼-88.56 ms(-32.6%)	4 (+0)
Menu mount (300 instances)	103.68 ms ▼-49.42 ms(-32.3%)	2 (+0)
Select mount (200 instances)	110.43 ms ▼-40.48 ms(-26.8%)	3 (+0)
Scroll Area mount (300 instances)	70.97 ms ▼-36.56 ms(-34.0%)	3 (+0)
Menu open (500 items)	51.91 ms ▼-22.74 ms(-30.5%)	12 (+0)

…and 3 more (+4 within noise) — details

---

Check out the code infra dashboard for more information about this PR.

[netlify]

✅ Deploy Preview for base-ui ready!

Name	Link
🔨 Latest commit	36299db
🔍 Latest deploy log	https://app.netlify.com/projects/base-ui/deploys/6a0c083ee30f0e0008c83ef8
😎 Deploy Preview	https://deploy-preview-4844--base-ui.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[flaviendelangle]

PR Review Summary — #4844 [otp field] Fix vertical arrow slot navigation

Author: atomiks • Base: master → Head: codex/fix-otp-field-selection • Diff: +63 / −6 across 2 files

The PR maps ArrowUp/ArrowDown in OTPField.Input to Home/End-like slot navigation (with ArrowDown keeping focus in place on an empty slot), and fixes a latent re-.select() bug when the same character is retyped into the final slot.

Critical Issues

None blocking. Both review agents converged — no correctness, security, or merge-blocker concerns.

Important Issues

• [tests] Lost Home/End readonly coverage — packages/react/src/otp-field/input/OTPFieldInput.test.tsx:306-312. The existing readonly navigation test was modified to swap Home/End assertions for ArrowUp/ArrowDown, removing readonly coverage for the original keys. Either restore the Home/End assertions alongside the new ones, or add a dedicated arrow-key readonly test. A future regression breaking readonly Home/End would now go unnoticed.

• [tests] No ArrowDown boundary case (already on last filled slot) — packages/react/src/otp-field/input/OTPFieldInput.test.tsx:199-211. The parametrized test only exercises ArrowDown from a non-boundary index (inputs[1] of defaultValue="1234", lastFilledIndex=3). The case where current index already equals lastFilledIndex is exactly where the new slotValue !== '' guard interacts — focus should stay, keyDown should still return false. Without it, a refactor that flipped the guard polarity could pass silently.

Suggestions

• [code-review] lastFilledIndex is a misleading name — packages/react/src/otp-field/input/OTPFieldInput.tsx:199. Math.max(value.length - 1, 0) returns 0 when value === '', which isn't actually a "filled" index. Current callers happen to be safe (focusing slot 0 when empty is correct; the new slotValue !== '' guard on ArrowDown short-circuits before it matters), but the name implies a postcondition the value doesn't guarantee. Consider endTargetIndex.

• [code-review] ArrowDown on an empty slot still calls stopEvent — packages/react/src/otp-field/input/OTPFieldInput.tsx:218-223. Functionally fine for type=text|password (browsers have no default ArrowDown behavior to suppress), but if the input type ever changes, suppressing it silently could mask issues. Either move stopEvent inside the conditional, or leave a one-line WHY.

• [code-review] Modifier-semantics asymmetry vs. ArrowLeft/Right — packages/react/src/otp-field/input/OTPFieldInput.tsx:200-212 vs 218-223. ArrowLeft/Right only jump to boundary with Ctrl/Meta; ArrowUp/Down jump unconditionally. Not wrong (WAI-ARIA APG doesn't standardize an OTP keyboard model), but worth confirming with the design system that the divergence is intentional — some macOS users expect Cmd+Arrow semantics across all four directions.

• [tests] No coverage for ArrowUp/ArrowDown in disabled mode — the early-return at OTPFieldInput.tsx:194 now gates the new keys too. A defaultValue="12" disabled + ArrowUp test would harden the gate; no existing test covers disabled keyboard behavior.

• [tests] stopPropagation not directly asserted — OTPFieldInput.test.tsx:215-220 asserts preventDefault via fireEvent.keyDown(...) === false. stopEvent also calls stopPropagation, which the return value doesn't capture. Minor — both calls live in the same helper.

Strengths

• The re-.select() fix is the standout. Replacing focusInput(Math.min(length - 1, index + 1)) with if (index < length - 1) focusInput(index + 1) at packages/react/src/otp-field/input/OTPFieldInput.tsx:258-262 correctly avoids the spurious .select() call (root invokes target.select() unconditionally) when the user retypes the same character on the last slot. The test at OTPFieldInput.test.tsx:222-241 uses vi.spyOn(lastInput, 'select') — a precise behavioral assertion that would catch any regression.

• ArrowDown-on-empty-slot semantics match the user mental model. When the focus is already past the filled region, the natural "end" is the current position. Captured cleanly by the test at OTPFieldInput.test.tsx:213-221.

• Tests follow project conventions. Uses fireEvent.keyDown return-value to assert preventDefault, Vitest APIs only (vi.spyOn, vi.fn — no Chai/Sinon chains), screen.getAllByRole with explicit <HTMLInputElement> typing, and act-wrapped focus changes. Aligns with AGENTS.md testing guidelines.

• No CLAUDE.md / AGENTS.md violations — no as any, no new hooks (so useTimeout/useStableCallback/useIsoLayoutEffect N/A), no document/window lookups added.

• Small, focused diff that fixes two real bugs (missing ArrowUp/Down navigation + re-.select() on the last slot) without scope creep.

Recommended Action

1. Address before merge (low-effort):

   • Restore Home/End readonly coverage at OTPFieldInput.test.tsx:306-312 (or add a sibling test for ArrowUp/Down in readonly).
   • Add the ArrowDown-on-last-filled-slot boundary test case.

2. Consider (cheap polish):

   • Rename lastFilledIndex → endTargetIndex at OTPFieldInput.tsx:199.
   • Confirm with design system that ArrowUp/Down unconditional boundary-jump (vs ArrowLeft/Right requiring Ctrl/Meta) is intentional.

3. Follow-up (optional):

   • Disabled-mode coverage for the new keys.