Skip to content

feat: add overrideInputMode prop for screenreader accessibility (#463) - #465

Open
dikshit-n wants to merge 5 commits into
devfolioco:mainfrom
dikshit-n:fix/screenreader-inputmode-accessibility
Open

feat: add overrideInputMode prop for screenreader accessibility (#463)#465
dikshit-n wants to merge 5 commits into
devfolioco:mainfrom
dikshit-n:fix/screenreader-inputmode-accessibility

Conversation

@dikshit-n

Copy link
Copy Markdown

Summary

Add a new overrideInputMode prop to let users control the inputMode HTML attribute independently of inputType, fixing screenreader accessibility issues.

Problem

When inputType is set to number or tel, the component sets inputMode="numeric" on each input. Screenreaders then announce the fields as "stepper" or "telephone" instead of a generic OTP input — see issue #463.

Solution

Introduce overrideInputMode?: "none" | "text" | "numeric" | "tel" | "email" | "url" | "decimal" | "search".

  • Defaults to the current behaviour (driven by inputType) for backward compatibility.
  • Set overrideInputMode="text" with inputType="number" to keep number-only validation while eliminating the screenreader announcement issue.
  • Set overrideInputMode="none" to omit the attribute entirely.

Changes Made

  • src/index.tsx: Added overrideInputMode prop to OTPInputProps, destructured in the component, used to override inputMode in the rendered inputs. Also exported the new AllowedInputMode type.
  • example/src/App.tsx: Added a demo control for overrideInputMode in the example playground.
  • README.md: Documented the new prop in the API table and updated the "Do not override" warning for inputMode.
  • yarn.lock: Updated by yarn install.

Testing

  • TypeScript compiles cleanly with no errors (tsc --noEmit).
  • ESLint passes with no errors.
  • Example app builds with new prop integrated into the config panel.

Checklist

  • Tests pass locally
  • Lint passes
  • Code follows repo style
  • Documentation updated
  • No console.log or debug code left
  • No unrelated changes

@pantoaibot

pantoaibot Bot commented Sep 3, 2026

Copy link
Copy Markdown

Do you want me to review this PR? Please comment /review .

@dikshit-n

Copy link
Copy Markdown
Author

/review

@pantoaibot

pantoaibot Bot commented Sep 3, 2026

Copy link
Copy Markdown

PR Summary:

Add overrideInputMode prop to control the inputMode attribute (fixes screenreader "stepper"/telephone announcements).

Changes:

  • src/index.tsx
    • New AllowedInputMode type and overrideInputMode?: AllowedInputMode on OTPInputProps.
    • inputMode applied to each input is now overrideInputMode ?? (isInputNum ? 'numeric' : 'text').
    • Exported AllowedInputMode type.
    • No change to default behavior: numeric remains default for number/tel unless overridden.
  • README.md
    • Documented overrideInputMode prop in the props table and added guidance to the warning (use the prop instead of overriding inputMode directly).
    • Describes accepted values and rationale (use 'text' to avoid screenreader stepper announcement, 'none' to omit attribute).
  • example/src/App.tsx
    • Example UI updated to include overrideInputMode control, default value set in example state, and prop passed through to OTPInput.

Notes:

  • No breaking changes to existing prop semantics; this is an additive prop for accessibility.
  • Tests were not added in this PR.

Reviewed by Panto AI

Comment thread src/index.tsx Outdated
Comment thread example/src/App.tsx Outdated
@pantoaibot

pantoaibot Bot commented Sep 3, 2026

Copy link
Copy Markdown

Reviewed up to commit:e498c433dc4376e70ed8ad2beab22d668caa2738

Reviewed by Panto AI

@dikshit-n

Copy link
Copy Markdown
Author

Both points addressed in commit 517d936:

Critical bug fixed: The expression now maps overrideInputMode="none" to undefined so React omits the attribute entirely, rather than rendering inputMode="none". Also refactored InputProps.inputMode to use an explicit InputModeValue type that includes undefined.

Nitpick fixed: The example no longer initializes overrideInputMode in state, so it defaults to undefined and demonstrates the input-type-driven default behavior as documented.

@dikshit-n

Copy link
Copy Markdown
Author

/review

Comment thread example/src/App.tsx Outdated
Comment thread src/index.tsx Outdated
Comment thread src/index.tsx
Comment thread src/index.tsx
Comment thread README.md Outdated
Comment thread README.md
@pantoaibot

pantoaibot Bot commented Sep 3, 2026

Copy link
Copy Markdown

Reviewed up to commit:517d936a2c42b1c3bceb09ead929cb9ffeaa57f6

Additional Suggestion
example/src/App.tsx, line:102-109 The new uses `value={overrideInputMode}` while overrideInputMode can be undefined which can trigger controlled/uncontrolled React warnings. Provide an explicit default option representing the "input-type-driven" behaviour (e.g. value="" or value={undefined}) and bind the select with `value={overrideInputMode ?? ''}` (or set a sensible default like 'numeric'). Also consider adding options for all AllowedInputMode values or a "Default (input-type-driven)" option so the playground demonstrates all cases. <div className="side-bar__segment"> <label htmlFor="overrideInputMode">overrideInputMode</label> <select id="overrideInputMode" name="overrideInputMode" value={overrideInputMode ?? ''} onChange={handleChange} > <option value="">input-type-driven (default)</option> <option value="numeric">numeric — shows number pad on mobile</option> <option value="text">text — avoids screenreader stepper announcement</option> <option value="none">none — disables inputMode attribute</option> <option value="tel">tel</option> <option value="email">email</option> <option value="url">url</option> <option value="decimal">decimal</option> <option value="search">search</option> </select> </div> example/src/App.tsx, line:127 You forward `overrideInputMode={overrideInputMode}` to . Ensure the playground's initial value and select include an explicit 'default' state for the input-type-driven behavior (see previous comment) so users can revert to the original default behavior from the UI. // inside initial state overrideInputMode: undefined, // select binding <select id="overrideInputMode" name="overrideInputMode" value={overrideInputMode ?? ''} onChange={handleChange} > <option value="">input-type-driven (default)</option> {/* other options */} </select> // forwarding to OTPInput remains the same <OTPInput // ...other props overrideInputMode={overrideInputMode} renderInput={(props) => <input {...props} />} shouldAutoFocus /> src/index.tsx, line:269-271 The expression `overrideInputMode === 'none' ? undefined : overrideInputMode ?? (isInputNum ? 'numeric' : 'text')` correctly omits the attribute when 'none' is chosen. Consider making the intent clearer by extracting to a small helper (e.g. computeInputMode()) or adding a brief inline comment: `// 'none' -> omit attribute, undefined -> use inputType-driven default` to improve readability. const computeInputMode = ( overrideInputMode: AllowedInputMode | undefined, isInputNum: boolean ): InputModeValue => { // 'none' -> omit attribute, undefined -> use inputType-driven default if (overrideInputMode === 'none') return undefined; if (overrideInputMode) return overrideInputMode; return isInputNum ? 'numeric' : 'text'; }; // usage inputMode: computeInputMode(overrideInputMode, isInputNum), example/src/App.tsx, line:14 The initializer `overrideInputMode: undefined as unknown as string` is a type hack and confusing. Replace it with a properly typed value (e.g. `overrideInputMode: undefined as AllowedInputMode | undefined`) or include an explicit default (see next comment) so the intent is clear and TypeScript types are correct. const [{ otp, numInputs, separator, minLength, maxLength, placeholder, inputType, overrideInputMode }, setConfig] = React.useState<PlaygroundConfig>({ otp: '', numInputs: 4, separator: '-', minLength: 0, maxLength: 40, placeholder: '', inputType: 'text', overrideInputMode: undefined, }); Reviewed by Panto AI

…ps type, typed PlaygroundConfig, README examples
@dikshit-n

Copy link
Copy Markdown
Author

All remaining review comments addressed in commit 911d7ae:

Round 2 changes:

  • App.tsx (typed PlaygroundConfig): Replaced the ad-hoc undefined as unknown as string cast with a properly typed PlaygroundConfig interface. Also imported AllowedInputMode from react-otp-input and fixed the controlled/uncontrolled select issue by using overrideInputMode ?? "" for the value and converting back to undefined on empty-string selection.

  • src/index.tsx (backward-compatible InputProps): Restored the original Required<Pick<...>> structure for the public InputProps type. The inputMode field is typed as React.InputHTMLAttributes[HTMLInputElement]["inputMode"] (which includes undefined) so the internal overrideInputMode === "none" ? undefined : ... expression is type-safe without casts. InputModeValue type alias removed (was unused).

  • README.md (Type column): Changed the Type column for overrideInputMode from generic string to the explicit list of accepted values. Added a usage example snippet showing overrideInputMode="text" with inputType="number" to fix the screenreader issue.

  • README.md (usage example): Added a concrete code example after the "Do not override" warning showing the recommended pattern.

@dikshit-n

Copy link
Copy Markdown
Author

/review

Comment thread README.md
@pantoaibot

pantoaibot Bot commented Sep 3, 2026

Copy link
Copy Markdown

Reviewed up to commit:911d7ae1df5f4765edbac39e027458b00f3961ca

Additional Suggestion
example/src/App.tsx, line:32-35 handleChange uses a generic `[name]: value` assignment which is not type-safe for the strongly-typed PlaygroundConfig state. Prefer narrowing `name` to `keyof PlaygroundConfig` (or create separate handlers per field) so TypeScript can validate assignments. For example, type the event handler as `React.ChangeEvent & { target: HTMLInputElement & { name: keyof PlaygroundConfig } }` or cast `name as keyof PlaygroundConfig` when updating state.
const handleChange = (
  event: React.ChangeEvent<HTMLInputElement | HTMLSelectElement> & {
    target: (EventTarget & HTMLInputElement) & { name: keyof PlaygroundConfig };
  }
) => {
  const { name, value } = event.target;
  setConfig((prevConfig) => ({ ...prevConfig, [name]: value }));
};

Reviewed by Panto AI

@dikshit-n

Copy link
Copy Markdown
Author

Addressed in commit aac652d:

  • App.tsx handleChange type safety: Cast name as keyof PlaygroundConfig to give TypeScript enough information to validate the state update — eliminates the unsafe [name]: value pattern.
  • App.tsx select options: Added all AllowedInputMode values (tel, email, url, decimal, search) alongside the existing options so the playground fully demonstrates the prop surface.
  • README overrideInputMode="none" note: Added a clarifying sentence explaining that "none" intentionally omits the attribute (React renders inputMode="none" as a text input on some browsers, which is not the intended behaviour).

@dikshit-n

Copy link
Copy Markdown
Author

/review

Comment thread example/src/App.tsx Outdated
Comment thread example/src/App.tsx Outdated
Comment thread src/index.tsx
Comment thread src/index.tsx Outdated
Comment thread README.md
@pantoaibot

pantoaibot Bot commented Sep 3, 2026

Copy link
Copy Markdown

Reviewed up to commit:aac652d033e0704098f8cf46b336816471807c0b

Additional Suggestion
src/index.tsx, line:249-281 The mapping for inputMode uses overrideInputMode === 'none' ? undefined : overrideInputMode ?? (isInputNum ? 'numeric' : 'text'), which is correct. Consider moving this logic into a small helper (getInputMode) to keep renderInput call site concise and easier to test. Example: const getInputMode = (override?) => override === 'none' ? undefined : override ?? (isInputNum ? 'numeric' : 'text'); then pass inputMode: getInputMode(overrideInputMode).
const getInputMode = (
  overrideInputMode: AllowedInputMode | undefined,
  isInputNum: boolean
): React.InputHTMLAttributes<HTMLInputElement>['inputMode'] => {
  if (overrideInputMode === 'none') return undefined;
  if (overrideInputMode != null) return overrideInputMode;
  return isInputNum ? 'numeric' : 'text';
};

// usage inside map:
inputMode: getInputMode(overrideInputMode, isInputNum),

Reviewed by Panto AI

…leChange numeric coercion, type-only import, README default clarity
@dikshit-n

Copy link
Copy Markdown
Author

All addressed in commit bdfbe47:

  • App.tsx — type-only import: Replaced import OTPInput, { AllowedInputMode } with import OTPInput + import type { AllowedInputMode } — avoids emitting a runtime import for a type-only symbol.

  • App.tsx — handleChange numeric coercion: Added an explicit name === "numInputs" branch that parses Number(value) before calling setConfig. Other string fields (otp, separator, placeholder, inputType, overrideInputMode) fall through to the generic assignment.

  • src/index.tsx — inputMode optional: Changed inputMode from required to optional (inputMode?:) in InputProps. The type is React.InputHTMLAttributes[HTMLInputElement]["inputMode"] which includes undefined, so the internal overrideInputMode === "none" ? undefined : ... expression is still type-safe.

  • src/index.tsx — grouped export consistency: Removed the inline export from InputProps and added it to the grouped export type { ... } statement at the bottom.

  • README — default column: Changed the Default cell for overrideInputMode from "input-type-driven" to "derived from inputType (\"numeric\" for \"number\"/\"tel\", otherwise \"text\"); pass \"none\" to omit the attribute" so readers immediately understand the default value and the none behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant