feat(headless): add headless OTP input primitive#9079
Conversation
🦋 Changeset detectedLatest commit: d340dce The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds a new headless OTP primitive, with its core components, public exports, build and docs wiring, and a test suite covering input, navigation, form, and accessibility behavior. ChangesHeadless OTP Primitive
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.changeset/headless-otp.md (1)
1-3: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a non-empty changeset entry.
This PR adds a new OTP primitive, but the changeset is still just delimiters. If
packages/headlessis released through Changesets, this needs a summary/bump entry so the new API shows up in the release notes. Based on learnings, empty changesets are only acceptable for docs-only PRs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.changeset/headless-otp.md around lines 1 - 3, The changeset file is empty and needs a real release note entry for the new OTP primitive. Update the changeset content so it includes a non-empty summary and the appropriate bump for the headless package, making sure the new API is captured by Changesets and appears in release notes.Source: Learnings
🧹 Nitpick comments (3)
packages/headless/src/primitives/otp/otp.test.tsx (2)
44-370: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing coverage for
maskandDeletekey, both called out as supported features.The PR summary states the primitive supports masking and Backspace/Delete, but this suite only exercises Backspace (Line 139, 144) and never sets
mask. Consider adding:
- A test asserting
OTP.Inputrenders withtype="password"(or equivalent) whenmaskis set.- A
{Delete}keyboard test alongside the existing Backspace coverage (Lines 129-147).🧪 Suggested additional tests
it('masks input values when mask is set', () => { render(<Harness length={4} mask />); for (const input of inputs()) { expect(input).toHaveAttribute('type', 'password'); } }); it('deletes the current character on Delete without moving focus', async () => { const user = userEvent.setup(); render(<Harness length={4} defaultValue='12' />); await user.click(inputs()[0]); await user.keyboard('{Delete}'); expect(inputs()[0]).toHaveValue(''); expect(inputs()[0]).toHaveFocus(); });As per coding guidelines, "Unit tests are required for all new functionality" and "Include tests for all new features."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/headless/src/primitives/otp/otp.test.tsx` around lines 44 - 370, The OTP test suite is missing coverage for two supported features: masking and the Delete key behavior. Add a test around OTP.Root/OTP.Input to verify that enabling mask causes the rendered inputs to use the masked password type, and extend the existing keyboard navigation coverage in the Backspace/Delete section to assert that {Delete} clears the current slot without shifting focus. Use the existing Harness, inputs(), and keyboard navigation tests as the place to add these cases.Source: Coding guidelines
46-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer RTL
screenqueries over rawdocument.querySelectorfor consistency.Several assertions query via
document.querySelector('[data-cl-slot="..."]')while other tests in the same file correctly usescreen.getByRole/getByTestId(Lines 319, 321, 352). Since thesedata-cl-slotvalues aren't naturally targetable by role/label, consider exposingdata-testidor usingscreen.getByRole('group')/container.querySelectorscoped tocontainerfor consistency, rather than globaldocument.querySelector.As per coding guidelines, "Use proper test queries in React Testing Library tests."
Also applies to: 53-53, 266-266, 282-282, 289-289
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/headless/src/primitives/otp/otp.test.tsx` at line 46, The OTP tests are using global document.querySelector for several slot assertions instead of React Testing Library queries. Update the affected assertions in the otp test cases to use RTL-friendly queries consistently, such as screen.getByTestId or a scoped container query, and if needed expose stable test ids for the slot elements so they can be targeted without querying the global document. Keep the existing patterns in the file aligned with the other screen-based assertions.Source: Coding guidelines
packages/headless/src/primitives/otp/otp-input.tsx (1)
85-168: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate insertion logic between
onChangeandonPaste.Both handlers compute
sanitize→replaceAt→sanitize→setValue→queueFocusidentically. Consider extracting a shared helper to avoid drift between the two paths.♻️ Proposed refactor
+ const applyInsertion = useCallback( + (rawInsert: string) => { + const inserted = sanitize(rawInsert, pattern, length); + if (inserted === '') { + return false; + } + const next = sanitize(replaceAt(value, index, inserted), pattern, length); + setValue(next); + queueFocus(Math.min(index + inserted.length, length - 1), next); + return true; + }, + [pattern, length, value, index, setValue, queueFocus], + ); + onChange: (event: React.ChangeEvent<HTMLInputElement>) => { if (disabled) { return; } const raw = event.currentTarget.value; if (raw === '') { setValue(removeAt(value, index)); return; } - const inserted = sanitize(raw, pattern, length); - if (inserted === '') { - event.currentTarget.value = char; - return; - } - - const next = sanitize(replaceAt(value, index, inserted), pattern, length); - setValue(next); - queueFocus(Math.min(index + inserted.length, length - 1), next); + if (!applyInsertion(raw)) { + event.currentTarget.value = char; + } }, ... onPaste: (event: React.ClipboardEvent<HTMLInputElement>) => { if (disabled) { return; } event.preventDefault(); - const inserted = sanitize(event.clipboardData?.getData('text') ?? '', pattern, length); - if (inserted === '') { - return; - } - const next = sanitize(replaceAt(value, index, inserted), pattern, length); - setValue(next); - queueFocus(Math.min(index + inserted.length, length - 1), next); + applyInsertion(event.clipboardData?.getData('text') ?? ''); },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/headless/src/primitives/otp/otp-input.tsx` around lines 85 - 168, The OTP input handlers in `OtpInput` duplicate the same insert flow in both `onChange` and `onPaste` (`sanitize` → `replaceAt` → `sanitize` → `setValue` → `queueFocus`), so extract that shared logic into a helper used by both paths. Keep the helper responsible for applying inserted text at the current `index`, updating state, and moving focus consistently so future changes to insertion behavior only need to be made in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/headless/src/primitives/otp/otp-input.tsx`:
- Around line 15-44: Wrap OTPInput with React.forwardRef so the consumer ref is
received correctly in React 18 instead of being read from props; update the
OTPInput component signature to accept the forwarded ref, then merge that ref
with the internal registerInput-based setRef logic inside OTPInput while keeping
the existing useOTPContext behavior unchanged.
In `@packages/swingset/src/stories/otp.stories.tsx`:
- Around line 1-38: The OTP story is not matching the shared docs-story contract
and also triggers react/jsx-pascal-case because the imported component name is
capitalized as OTP. Alias the import in the OTP story module to a
PascalCase-safe name like Otp, then update the story export so Default accepts
the standard props signature used in other .stories.tsx files, including props:
Record<string, unknown> and knobsAsProps. Keep the existing OTP.Root and Slots
usage intact, but wire the story through the shared props pattern so the docs
build and knob plumbing remain consistent.
---
Outside diff comments:
In @.changeset/headless-otp.md:
- Around line 1-3: The changeset file is empty and needs a real release note
entry for the new OTP primitive. Update the changeset content so it includes a
non-empty summary and the appropriate bump for the headless package, making sure
the new API is captured by Changesets and appears in release notes.
---
Nitpick comments:
In `@packages/headless/src/primitives/otp/otp-input.tsx`:
- Around line 85-168: The OTP input handlers in `OtpInput` duplicate the same
insert flow in both `onChange` and `onPaste` (`sanitize` → `replaceAt` →
`sanitize` → `setValue` → `queueFocus`), so extract that shared logic into a
helper used by both paths. Keep the helper responsible for applying inserted
text at the current `index`, updating state, and moving focus consistently so
future changes to insertion behavior only need to be made in one place.
In `@packages/headless/src/primitives/otp/otp.test.tsx`:
- Around line 44-370: The OTP test suite is missing coverage for two supported
features: masking and the Delete key behavior. Add a test around
OTP.Root/OTP.Input to verify that enabling mask causes the rendered inputs to
use the masked password type, and extend the existing keyboard navigation
coverage in the Backspace/Delete section to assert that {Delete} clears the
current slot without shifting focus. Use the existing Harness, inputs(), and
keyboard navigation tests as the place to add these cases.
- Line 46: The OTP tests are using global document.querySelector for several
slot assertions instead of React Testing Library queries. Update the affected
assertions in the otp test cases to use RTL-friendly queries consistently, such
as screen.getByTestId or a scoped container query, and if needed expose stable
test ids for the slot elements so they can be targeted without querying the
global document. Keep the existing patterns in the file aligned with the other
screen-based assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: c99c4ca8-4420-441f-b328-139166867351
📒 Files selected for processing (16)
.changeset/headless-otp.mdpackages/headless/README.mdpackages/headless/package.jsonpackages/headless/src/primitives/otp/README.mdpackages/headless/src/primitives/otp/index.tspackages/headless/src/primitives/otp/otp-context.tspackages/headless/src/primitives/otp/otp-input.tsxpackages/headless/src/primitives/otp/otp-root.tsxpackages/headless/src/primitives/otp/otp-utils.tspackages/headless/src/primitives/otp/otp.test.tsxpackages/headless/src/primitives/otp/parts.tspackages/headless/vite.config.tspackages/swingset/src/components/DocsViewer.tsxpackages/swingset/src/lib/registry.tspackages/swingset/src/stories/otp.mdxpackages/swingset/src/stories/otp.stories.tsx
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
Description
Adds a new headless one-time-password / PIN primitive at
@clerk/headless/otp, following the existing headless patterns (compoundOTP.Root+OTP.Inputparts, auseOTPhook,renderElement/mergeProps,useControllableState, anddata-cl-*state attributes with zero styles). It uses one real<input>per character (base-uiOTPFieldinspired) so each slot is individually styleable, and handles typing/focus advance, Backspace/Delete, arrow/Home/End navigation, paste distribution, apatterncharacter filter, masking,disabled,onComplete, and optional<form>submission vianame. Also adds Swingset docs (story + MDX overview, wired into the registry and docs viewer) and a primitive README. Test it viapnpm --filter @clerk/headless test(19 OTP tests incl. axe) or interactively in Swingset (pnpm dev:swingset).Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
@clerk/headless/otp.