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

chore(clerk-js): Allow single-character usernames in <UserProfile /> validation #4243

Conversation

nikospapcom
Copy link
Member

@nikospapcom nikospapcom commented Sep 30, 2024

Description

In this pr we are allowing single-character usernames in <UserProfile /> validation

Checklist

  • npm test runs as expected.
  • npm run build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

@nikospapcom nikospapcom self-assigned this Sep 30, 2024
Copy link

changeset-bot bot commented Sep 30, 2024

🦋 Changeset detected

Latest commit: 772f2a4

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

This PR includes changesets to release 3 packages
Name Type
@clerk/clerk-js Patch
@clerk/chrome-extension Patch
@clerk/clerk-expo 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

@@ -26,8 +26,7 @@ export const UsernameForm = withCardStateProvider((props: UsernameFormProps) =>

const isUsernameRequired = userSettings.attributes.username.required;

const canSubmit =
(isUsernameRequired ? usernameField.value.length > 1 : true) && user.username !== usernameField.value;
const canSubmit = user.username !== usernameField.value;
Copy link
Member

Choose a reason for hiding this comment

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

❓ It seems like we have the userSettings available and thus the username min and max length. Is it possible to use them on the client side validation in place of the hardcoded value?

Copy link
Member Author

Choose a reason for hiding this comment

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

We have it but we rely only on the backend validation if we have username as missing field during sign-up.
I removed the client side validation in order to have consistency and have the same behaviour in both cases

ss from missing fields screen

Screenshot 2024-09-30 at 12 02 48 PM

Copy link
Member

Choose a reason for hiding this comment

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

Great thanks!

Copy link
Member

Choose a reason for hiding this comment

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

@nikospapcom I don't see why this change improves things.

Currently we want to disable the button, and don't let you submit if you don't have a value in the field or the value that you matches your current username.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe only changing usernameField.value.length > 0 is enough ?

@@ -26,8 +26,7 @@ export const UsernameForm = withCardStateProvider((props: UsernameFormProps) =>

const isUsernameRequired = userSettings.attributes.username.required;

const canSubmit =
(isUsernameRequired ? usernameField.value.length > 1 : true) && user.username !== usernameField.value;
const canSubmit = user.username !== usernameField.value;
Copy link
Member

Choose a reason for hiding this comment

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

@nikospapcom I don't see why this change improves things.

Currently we want to disable the button, and don't let you submit if you don't have a value in the field or the value that you matches your current username.

@nikospapcom nikospapcom force-pushed the nikospap/user-718-userprofile-doesnt-respect-username-settings branch from 670d0c5 to 772f2a4 Compare September 30, 2024 11:07
@nikospapcom nikospapcom changed the title chore(clerk-js): Remove client-side validation for <UserProfile /> username field chore(clerk-js): Allow single-character usernames in <UserProfile /> validation Sep 30, 2024
@nikospapcom nikospapcom merged commit c49eaa7 into main Sep 30, 2024
22 checks passed
@nikospapcom nikospapcom deleted the nikospap/user-718-userprofile-doesnt-respect-username-settings branch September 30, 2024 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants