Skip to content
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

feature fix wrong position of the focused input #455

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kkdras
Copy link

@kkdras kkdras commented Oct 2, 2024

  • What does this PR do?

  • Any background context you want to provide?

  • Screenshots and/or Live Demo

Copy link

pantomaxbot bot commented Oct 2, 2024

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

@sashank-zluri
Copy link

/review Please review the PR

@@ -195,6 +198,11 @@ const OTPInput = ({
const changeCodeAtFocus = (value: string) => {
const otp = getOTPValue();
otp[activeInput] = value[0];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that value[0] is not undefined before assigning it to otp[activeInput] to prevent potential runtime errors.

if (value[0] !== undefined) {
    otp[activeInput] = value[0];
}

Copy link

pantomaxbot bot commented Feb 11, 2025

Reviewed up to commit:0db1935faca24c7f5a25bd9e74bc8fe52171dec7

Overall suggestion:

  • Consider adding a cleanup function for the useEffect hook that manages inputRefs.current to prevent potential memory leaks when the component unmounts or numInputs changes.

Few more points:

  • The setActiveInput(activeInput); call is redundant here since activeInput is already the current state. Consider removing it to avoid unnecessary state updates.

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.

2 participants