-
Notifications
You must be signed in to change notification settings - Fork 21
Enhance OTP field functionality with length and numeric-only options #66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| // Copyright 2026 The ThunderID Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| import {render, screen, cleanup, fireEvent} from '@testing-library/react'; | ||
| import {createTheme} from '@thunderid/browser'; | ||
| import {ReactElement} from 'react'; | ||
| import {afterEach, describe, expect, it, vi} from 'vitest'; | ||
| import ThemeContext, {ThemeContextValue} from '../../../../contexts/Theme/ThemeContext'; | ||
| import OtpField from '../OtpField'; | ||
|
|
||
| const themeContextValue: ThemeContextValue = { | ||
| colorScheme: 'light', | ||
| direction: 'ltr', | ||
| theme: createTheme(), | ||
| toggleTheme: vi.fn(), | ||
| }; | ||
|
|
||
| const withTheme = (ui: ReactElement): ReactElement => ( | ||
| <ThemeContext.Provider value={themeContextValue}>{ui}</ThemeContext.Provider> | ||
| ); | ||
|
|
||
| const boxes = (): HTMLInputElement[] => screen.getAllByRole('textbox'); | ||
|
|
||
| const paste = (target: HTMLElement, text: string): void => { | ||
| const event: Event = new Event('paste', {bubbles: true, cancelable: true}); | ||
| Object.defineProperty(event, 'clipboardData', {value: {getData: () => text}}); | ||
| fireEvent(target, event); | ||
| }; | ||
|
|
||
| describe('OtpField', () => { | ||
| afterEach(() => { | ||
| cleanup(); | ||
| }); | ||
|
|
||
| it('renders six boxes by default', () => { | ||
| render(withTheme(<OtpField />)); | ||
| expect(boxes()).toHaveLength(6); | ||
| }); | ||
|
|
||
| it('renders the requested number of boxes', () => { | ||
| render(withTheme(<OtpField length={8} />)); | ||
| expect(boxes()).toHaveLength(8); | ||
| }); | ||
|
|
||
| it('rejects a letter when the field is numeric', () => { | ||
| const onChange = vi.fn(); | ||
| render(withTheme(<OtpField type="number" onChange={onChange} />)); | ||
|
|
||
| fireEvent.change(boxes()[0], {target: {value: 'a'}}); | ||
|
|
||
| expect(onChange).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('accepts a digit when the field is numeric', () => { | ||
| const onChange = vi.fn(); | ||
| render(withTheme(<OtpField type="number" onChange={onChange} />)); | ||
|
|
||
| fireEvent.change(boxes()[0], {target: {value: '7'}}); | ||
|
|
||
| expect(onChange).toHaveBeenCalledWith({target: {value: '7'}}); | ||
| }); | ||
|
|
||
| it('marks a numeric field with the numeric input mode', () => { | ||
| render(withTheme(<OtpField type="number" />)); | ||
| expect(boxes()[0]).toHaveAttribute('inputmode', 'numeric'); | ||
| }); | ||
|
|
||
| it('accepts a letter when the field is alphanumeric', () => { | ||
| const onChange = vi.fn(); | ||
| render(withTheme(<OtpField onChange={onChange} />)); | ||
|
|
||
| fireEvent.change(boxes()[0], {target: {value: 'K'}}); | ||
|
|
||
| expect(onChange).toHaveBeenCalledWith({target: {value: 'K'}}); | ||
| }); | ||
|
|
||
| it('upper-cases entered characters when asked to', () => { | ||
| const onChange = vi.fn(); | ||
| render(withTheme(<OtpField uppercase onChange={onChange} />)); | ||
|
|
||
| fireEvent.change(boxes()[0], {target: {value: 'k'}}); | ||
|
|
||
| expect(onChange).toHaveBeenCalledWith({target: {value: 'K'}}); | ||
| }); | ||
|
|
||
| it('upper-cases a pasted code when asked to', () => { | ||
| const onChange = vi.fn(); | ||
| render(withTheme(<OtpField uppercase onChange={onChange} />)); | ||
|
|
||
| paste(boxes()[0], 'k7gx2m'); | ||
|
|
||
| expect(onChange).toHaveBeenCalledWith({target: {value: 'K7GX2M'}}); | ||
| }); | ||
|
|
||
| it('keeps a pasted code intact when surrounded by other text', () => { | ||
| const onChange = vi.fn(); | ||
| render(withTheme(<OtpField type="number" onChange={onChange} />)); | ||
|
|
||
| paste(boxes()[0], 'Your code is 123456'); | ||
|
|
||
| expect(onChange).toHaveBeenCalledWith({target: {value: '123456'}}); | ||
| }); | ||
|
|
||
| it('calls onComplete once every box is filled', () => { | ||
| const onComplete = vi.fn(); | ||
| render(withTheme(<OtpField type="number" onComplete={onComplete} />)); | ||
|
|
||
| paste(boxes()[0], '123456'); | ||
|
|
||
| expect(onComplete).toHaveBeenCalledWith('123456'); | ||
| }); | ||
|
Comment on lines
+104
to
+111
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
file=$(fd -i -t f 'OtpField.test.tsx' . | head -n 1)
printf '%s\n' "TEST_FILE=$file"
wc -l "$file"
sed -n '1,180p' "$file"
printf '%s\n' '--- OtpField candidates ---'
fd -i -t f 'OtpField' packages/react packages 2>/dev/null | head -n 30
printf '%s\n' '--- completion references ---'
rg -n -C 4 'onComplete|complete|paste' packages/react/src/components/primitives/OtpField packages/react/src 2>/dev/null | head -n 240Repository: thunder-id/javascript-sdks Length of output: 25262 🏁 Script executed: #!/bin/bash
set -eu
file=packages/react/src/components/primitives/OtpField/OtpField.tsx
wc -l "$file"
sed -n '120,230p' "$file"
printf '%s\n' '--- test and implementation event-handler declarations ---'
rg -n 'const handle(Change|KeyDown|Paste)|onPaste|onChange=|onKeyDown=|onComplete' \
packages/react/src/components/primitives/OtpField/OtpField.tsx \
packages/react/src/components/primitives/OtpField/__tests__/OtpField.test.tsxRepository: thunder-id/javascript-sdks Length of output: 6990 🏁 Script executed: #!/bin/bash
set -eu
file=packages/react/src/components/primitives/OtpField/OtpField.tsx
python3 - <<'PY'
from pathlib import Path
import re
path = Path("packages/react/src/components/primitives/OtpField/OtpField.tsx")
source = path.read_text()
paste = re.search(
r"const handlePaste.*?(?=\n\s*return \()",
source,
re.S,
)
if not paste:
raise SystemExit("handlePaste implementation not found")
body = paste.group(0)
calls = re.findall(r"\bonComplete\s*\(", body)
print(f"handlePaste onComplete call sites: {len(calls)}")
print("single paste path invokes onComplete at most once:", len(calls) == 1)
test = Path(
"packages/react/src/components/primitives/OtpField/__tests__/OtpField.test.tsx"
).read_text()
section = re.search(
r"it\('calls onComplete once every box is filled'.*?\n\s*\}\);",
test,
re.S,
)
if not section:
raise SystemExit("target test not found")
assertion = section.group(0)
print("exact call-count assertion present:",
bool(re.search(r"toHaveBeenCalledTimes\s*\(\s*1\s*\)", assertion)))
PYRepository: thunder-id/javascript-sdks Length of output: 299 Assert the single-completion contract. Add 🤖 Prompt for AI Agents |
||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,9 @@ export interface FieldConfig { | |
| disabled?: boolean; | ||
| error?: string; | ||
| label: string; | ||
| length?: number; | ||
| name: string; | ||
| numericOnly?: boolean; | ||
|
Comment on lines
+21
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Expose and forward the OTP options from
Forward the props through the component wrapper interface FieldFactorySetupProps {
className?: string;
disabled: boolean;
error?: string;
label: string;
+ length?: number;
name: string;
+ numericOnly: boolean;
options: SelectOption[];
placeholder?: string;
required: boolean;
touched: boolean;
type: FieldType;
value: string;
}
props: {
className: {default: undefined, type: String},
disabled: {default: false, type: Boolean},
error: {default: undefined, type: String},
label: {required: true, type: String},
+ length: {default: undefined, type: Number},
name: {required: true, type: String},
+ numericOnly: {default: true, type: Boolean},
options: {default: () => [], type: Array as PropType<SelectOption[]>},
placeholder: {default: undefined, type: String},
required: {default: false, type: Boolean},
touched: {default: false, type: Boolean},
type: {required: true, type: String as PropType<FieldType>},
value: {default: '', type: String},
},
createField({
className: props.className,
disabled: props.disabled,
error: props.error,
label: props.label,
+ length: props.length,
name: props.name,
+ numericOnly: props.numericOnly,
onBlur: () => emit('blur'),This follows the PR objective that Vue field configuration passes OTP length and numeric-only settings into the OTP field. Also applies to: 98-99, 159-160 🤖 Prompt for AI Agents |
||
| onBlur?: () => void; | ||
| onChange: (value: string) => void; | ||
| options?: SelectOption[]; | ||
|
|
@@ -93,6 +95,8 @@ export const createField = (config: FieldConfig): VNode => { | |
| options = [], | ||
| touched = false, | ||
| placeholder, | ||
| length, | ||
| numericOnly = true, | ||
| } = config; | ||
|
|
||
| const validationError: string | null | undefined = error || validateFieldValue(value, type, required, touched); | ||
|
|
@@ -152,6 +156,8 @@ export const createField = (config: FieldConfig): VNode => { | |
| case FieldType.Otp: | ||
| return h(OtpField, { | ||
| ...commonProps, | ||
| ...(length ? {length} : {}), | ||
| numericOnly, | ||
| 'onUpdate:modelValue': onChange, | ||
| } as Record<string, unknown>); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Set a maximum OTP length.
Line 349 accepts every positive integer. An
otpLengthof1000000causesOtpFieldto allocate state and render one million inputs. This can lock the authentication UI.Reject values above the backend protocol maximum before calling
createField.🤖 Prompt for AI Agents