-
Notifications
You must be signed in to change notification settings - Fork 242
Add username unavailability indicator and validation to Edit Profile … #580
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
base: dev
Are you sure you want to change the base?
Conversation
|
🎉 Welcome @4555jan!
We appreciate your contribution! 🚀 |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR implements a username availability checking feature in the EditProfileScreen by adding reactive state tracking in the controller and integrating debounced validation logic with real-time UI feedback in the screen, including availability indicators and error messaging. Changes
Sequence DiagramsequenceDiagram
actor User
participant Screen as EditProfileScreen
participant Debouncer
participant Validator
participant API as Availability<br/>Check
participant Controller
User->>Screen: Types username
Screen->>Debouncer: Queue availability check
Debouncer-->>Debouncer: Wait for input pause
Debouncer->>Validator: Validate format & length
alt Invalid Format/Length
Validator-->>Screen: Show error snackbar
Screen->>Screen: Hide verification icon
else Valid Format
Validator-->>Screen: Format OK
Screen->>Controller: Set usernameAvailableChecking = true
Controller-->>Screen: Update UI (show "checking")
Screen->>API: Check username availability
API-->>Screen: Available/Unavailable
alt Username Available
Screen->>Screen: Show green icon
Screen->>Controller: Set usernameAvailableChecking = false
else Username Taken
Screen->>Screen: Show error snackbar
Screen->>Controller: Set usernameAvailableChecking = false
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@M4dhav i added a debouncer of 800ms delay because During testing, I noticed that the username validation was triggering api calls with every letter ig and i saw a debouncer being used in basically in the onboarding too please review/check if i made any major mistakes i will fix it asap |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/controllers/edit_profile_controller.dart(1 hunks)lib/views/screens/edit_profile_screen.dart(6 hunks)
|
Hey @4555jan , for future reference, user with username CodeRabbit is someone else, and not the CodeRabbit ai Bot. Please tag coderabbitai to request future reviews. Although CodeRabbitAI still detects the command, it is probably annoying for the actual user with the CodeRabbit username. |
|
sorry i will make sure of that @M4dhav in the future i think i made a minor/major mistake with my commit in the files i will fix it soon are these changes that i made is mergeable or did made major mistakes ? |
|
Currently reviewing, will let you know |
M4dhav
left a comment
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.
Please share a video of the functionality
|
@M4dhav sure I will upload it tmrw |
…ler the variable was 'username Available'
|
Could you please include the part in the video where the username is valid? Also, I think the representation of valid/invalid usernames as done in the sign up flow is better than this |
|
@M4dhav ok i uploaded another video and by representation do you mean the way it shows the invalid message is the problem ?like the design is the problem ? Or my entire change is wrong ? |
|
Okay, design looks fine. One small change though, we have a minimum length of username, it is 6 or 7, you need to check for the exact value. Below that length the check should not trigger. Also, how does the UI react when a username is taken? |
|
@M4dhav i uploaded another video regarding how the ui will change if the username is taken |
|
UI Looks fine, can you make the change for minimum length please? |
|
@M4dhav i changed the minimum length to 7 as you mentioned |
M4dhav
left a comment
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.
Please run flutter gen-l10n with each PR to check untranslated.txt into source
| AppLocalizations.of( | ||
| context, | ||
| )!.usernameUnavailable, | ||
| "Username can only contain letters, numbers, dots, hyphens, and underscores (max 36 characters)", |
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.
Add localizations
| } | ||
|
|
||
| // Temporarily show checking state | ||
| controller.usernameAvailable.value = true; |
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.
This is not a checking state, this will show the user a green tick even while checking
|
actually i removed the the hardcoded error message snakebar entirely from the onChanged handler cause the validator will show the error inline with autovalidateMode. is that ok ? |
|
Yes that is fine. Could you share an updated video of the functionalituy as well |
| }, | ||
| controller: controller.usernameController, | ||
| onChanged: (value) async { | ||
| Get.closeCurrentSnackbar(); |
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.
If you are no longer opening snackbars for validation then this is unnecessary
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.
We are still showing snackbars for validation errors (invalid format and username taken), so Get.closeCurrentSnackbar() is needed isnt
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.
but you said AutoValidator will handle those inline so the only snackbar is shown when the submit button is pressed it looks like
| controller.usernameAvailable.value = false; | ||
| controller.usernameChecking.value = false; |
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.
These are unnecessary as you are already settings the values above
| if (!validUsername) { | ||
| return AppLocalizations.of( | ||
| context, | ||
| )!.usernameInvalidOrTaken; |
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.
Please make a separate string for invalid username and taken username so user can have proper feedback
|
i have updated the video please check it |
M4dhav
left a comment
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.
I think we should disable the save changes button when the username is not valid, as the autovalidator is already showing the necessary feedback inline
lib/l10n/app_en.arb
Outdated
| "thisMessageWasDeleted": "This message was deleted", | ||
| "failedToDeleteMessage": "Failed to delete message" | ||
| "failedToDeleteMessage": "Failed to delete message", | ||
| "usernameInvalidFormat": "Please enter a valid username. Only letters, numbers, dots, underscores, and hyphens are allowed.", | ||
| "usernameAlreadyTaken": "This username is already taken. Try a different one." |
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.
Please add descriptions for all 4 localization strings
|
sorry i didnt got it what u said elaborate it please |
|
|
should i add another video with the changes that we made till now |
| if (isUsernameChanged()) { | ||
| var usernameAvail = await isUsernameAvailable( | ||
| usernameController.text.trim(), | ||
| ); | ||
| if (!usernameAvail) { | ||
| usernameAvailable.value = false; | ||
| customSnackbar( | ||
| AppLocalizations.of(Get.context!)!.usernameUnavailable, | ||
| AppLocalizations.of(Get.context!)!.usernameInvalidOrTaken, | ||
| AppLocalizations.of(Get.context!)!.usernameAlreadyTaken, | ||
| LogType.error, | ||
| ); | ||
|
|
||
| SemanticsService.announce( | ||
| AppLocalizations.of(Get.context!)!.usernameInvalidOrTaken, | ||
| AppLocalizations.of(Get.context!)!.usernameAlreadyTaken, | ||
| TextDirection.ltr, | ||
| ); | ||
| return; | ||
| } |
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.
This is now unnecessary as button is disabled when username is invalid
| await databases.createDocument( | ||
| databaseId: userDatabaseID, | ||
| collectionId: usernameCollectionID, | ||
| documentId: usernameController.text.trim(), | ||
| data: {'email': authStateController.email}, | ||
| ); |
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.
Wrap this in the try catch below as well
| data: {'email': authStateController.email}, | ||
| ); | ||
|
|
||
| try { |
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.
Why 2 try catch blocks? same block can handle all conditions. Also, as you are rethrowing errors,ensure that errors are captured in the view and appropriately displayed, for example, via snackbar
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.
but we are displaying the errors in snakebar already isnt i am sorry ig rethrowing the errors is the problem is it? @M4dhav
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.
There's no problem with rethrowing as long as they are handled on the view
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.
do you think i should remove the rethrow because i think right now we are displaying the errors in the app cleanly right and so may be we should just log the error as the string what you think? actually when i wrote rethrow i wasnt sure if it really needs it
|
Also please add translation for Marathi and Gujarati for all added strings |
|
for the recent new strings as well right ? |
|
Just the ones you added in this PR |
ad0aae3 to
a8926cd
Compare
|
i had to force push earlier cause i was haveing branch issues and i removed rethrow and just logging the errors as we are already showing it snakebar |
Description
This PR adds the username unavailability indicator feature to the Edit Profile Screen, matching the functionality already present in the Onboarding Screen. Previously, the Edit Profile Screen only showed when a username was available but provided no feedback when a username was unavailable or invalid.
Changes made:
usernameAvailableCheckingobservable to track validation stateFixes #440
screen-recording-2025-11-23-100747_7v5xHbCz.mp4
Type of change
How Has This Been Tested?
Manual Testing:
Test Configuration:
Checklist:
Maintainer Checklist
Summary by CodeRabbit
Release Notes