Skip to content

Conversation

etnlbck
Copy link
Contributor

@etnlbck etnlbck commented Oct 1, 2023

Problem solved

Made readability enhancements to simplify reading.

Changes made

Line 39 Prefer using nullish coalescing operator (??) instead of a ternary expression, as it is simpler to read.
Line 47, 100 Use concise character class syntax '\d' instead of '[0-9]'.

@changeset-bot
Copy link

changeset-bot bot commented Oct 1, 2023

🦋 Changeset detected

Latest commit: 51cd30d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@thirdweb-dev/react Patch
@thirdweb-dev/react-core Patch
@thirdweb-dev/react-native Patch
@thirdweb-dev/react-native-compat Patch

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

ref={(e) => (boxEls.current[i] = e)}
key={i}
value={otp[i] === undefined ? "" : otp[i]}
value={otp[i] ?? ""}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Prefer using nullish coalescing operator (??) instead of a ternary expression, as it is simpler to read.

.slice(0, props.digits)
.split("")
.filter((n) => /[0-9]/.test(n))
.filter((n) => /\d/.test(n))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use concise character class syntax '\d' instead of '[0-9]'.

}

if (!/[0-9]/.test(value) && value !== "") {
if (!/\d/.test(value) && value !== "") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use concise character class syntax '\d' instead of '[0-9]'.

@etnlbck etnlbck changed the title Etnlbck otp input refactor(react): OTP Input Readability Enhancements Oct 1, 2023
@etnlbck etnlbck marked this pull request as ready for review October 1, 2023 19:00
@etnlbck etnlbck requested a review from a team as a code owner October 1, 2023 19:00
@etnlbck etnlbck requested a review from a team October 1, 2023 19:00
@MananTank
Copy link
Member

Thanks @etnlbck!

@MananTank MananTank enabled auto-merge October 2, 2023 16:06
@etnlbck
Copy link
Contributor Author

etnlbck commented Oct 4, 2023

@MananTank its strange that it's failing a unit test that has nothing to do with the change I made. I'll check on my side to see if there's anything I can do to resolve. Unless you have a better idea.

@MananTank
Copy link
Member

Hey @etnlbck - Yes, we are aware that the failing tests are unrelated to changes made in this PR, We will merge this PR soon. Thanks!

@joaquim-verges joaquim-verges merged commit fb0f12a into thirdweb-dev:main Oct 13, 2023
@github-actions github-actions bot mentioned this pull request Oct 13, 2023
jnsdls pushed a commit that referenced this pull request Jun 19, 2024
Signed-off-by: Juan Leal <114959779+juandolealt@users.noreply.github.com>
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.

3 participants