Update the name input onboarding screen#193
Conversation
WalkthroughThis update refactors the onboarding process by removing the age verification step and introducing a new, custom-designed name input screen with random creative nickname generation. A new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant NameGenerator
User->>App: Launches app
App->>User: Shows Welcome Screen
User->>App: Proceeds to onboarding
App->>User: Shows Name Input Screen
App->>NameGenerator: Requests random name
NameGenerator-->>App: Returns generated name
App->>User: Displays generated name (with option to refresh)
User->>App: Confirms name and continues
App->>User: Proceeds to next onboarding step
Possibly related PRs
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 8
🧹 Nitpick comments (10)
CHANGELOG.md (1)
21-22: Consider adding details to the "Internal Changes" sectionThe Internal Changes section has been created but is currently empty. Consider adding technical details about the implementation of the nickname generation feature or removal of age verification if applicable.
lib/features/create_community/create_community_widget.dart (1)
91-91: Added null assertion could cause a runtime exception.The use of the non-null assertion operator (
!) on_selectedPrivacycould potentially cause a runtime exception if null, despite the button enabling checks.Consider adding an additional null check for defensive programming:
-widget.onCreateCommunity(_communityNameController.text, _selectedPrivacy!); +if (_selectedPrivacy != null) { + widget.onCreateCommunity(_communityNameController.text, _selectedPrivacy!); +}lib/features/emergency_alert/emergency_alert_controller.dart (1)
11-12: Consider adding method documentationThis public method would benefit from documentation explaining its purpose, parameters, and return value.
+ /// Sends an emergency alert message to the specified group + /// + /// [message] - The alert text content + /// [groupIdentifier] - The target group identifier + /// + /// Returns the sent Event if successful, otherwise throws an exception Future<Event> sendEmergencyAlert( String message, GroupIdentifier groupIdentifier) async {lib/router/login/welcome_widget.dart (1)
88-98: Consider extracting text stylesText styles are repeated across the UI. Consider extracting them to reusable constants or theme extensions.
+ // In a styles.dart file or theme extension + TextStyle linkTextStyle(ThemeData themeData) => TextStyle( + color: themeData.customColors.accentColor, + fontSize: 16, + fontWeight: FontWeight.w400, + ); // Then in the widget: GestureDetector( onTap: () => _navigateToNostrLogin(context), child: Text( localization.Already_a_user, - style: TextStyle( - color: themeData.customColors.accentColor, - fontSize: 16, - fontWeight: FontWeight.w400, - ), + style: linkTextStyle(themeData), ), ),lib/router/onboarding/name_input_step_widget.dart (1)
24-24: Unused loading state variableThe
_isLoadingvariable is declared but never set to true, making it redundant in the current implementation.Either remove this variable if it's not needed, or implement the loading state functionality:
- final _isLoading = false;Or if you plan to use it in the future:
- final _isLoading = false; + bool _isLoading = false;lib/router/onboarding/utils/name_generator.dart (1)
56-70: Crystal/metal branch discards previously generated prefixes
The earlyreturnon line 67 means any previously generated luminosity/saturation prefixes (Dark,Vivid, …) are silently dropped when the 10 % crystal/metal branch is taken, so names such asDarkCrystalorVividSteelwill never appear.
If you intended to keep the full variety, append the crystal/metal string instead of returning early:- return crystals[_random.nextInt(crystals.length)]; + colorName += crystals[_random.nextInt(crystals.length)];lib/features/create_community/privacy_selection_widget.dart (1)
46-111: Improve accessibility & semantics of selectable options
Using a plainInkWell+Containerprovides no role information to screen-reader users.
Switching toRadioListTileor wrapping the whole option in aSemanticswidget withchecked/onTapHintattributes will make the component perceivable and operable for assistive technologies.Example refactor with
ListTilesemantics:- return InkWell( - onTap: () => onPrivacySelected(privacy), - child: Container( + return Semantics( + selected: isSelected, + button: true, + onTapHint: 'Select $title', + child: InkWell( + onTap: () => onPrivacySelected(privacy), + child: Container( ... - ), + ), + ), );lib/features/emergency_alert/emergency_alert_screen.dart (2)
7-12: ConsiderProviderinstead ofStateProviderfor the controller
messageProvideronly ever exposes the sameTextEditingControllerinstance and is mutated directly; using aProvideravoids the unnecessaryStateProviderindirection and rebuilds.-final messageProvider = StateProvider<TextEditingController>((ref) { +final messageProvider = Provider<TextEditingController>((ref) {
136-139: Use theme colors for better consistency
Colors.redmay clash with dark/light themes. PreferthemeData.colorScheme.error(orerrorContainerin Material-3) for automatic contrast.- backgroundColor: Colors.red, + backgroundColor: themeData.colorScheme.error,lib/router/login/login_widget.dart (1)
335-359: Interactive text doesn’t meet WCAG minimum tap target size
The terms-of-serviceGestureDetectorwraps only the text; on high-density screens the height may fall below 48 dp recommended by the Material guidelines.
Wrap it in aSizedBoxwith a minimum height or useInkWellwith padding to improve usability.
🛑 Comments failed to post (8)
lib/l10n/intl_en.arb (1)
378-387:
⚠️ Potential issueAdd missing comma after "Already_a_user" (line 381).
There's a syntax error in the JSON structure due to a missing comma after the "Already_a_user" string definition. This will cause localization parsing errors.
- "Already_a_user": "Already a user?" + "Already_a_user": "Already a user?",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements."Welcome_to_plur": "Welcome to Plur", "Create_a_profile": "Create a Profile", "Login_with_bluesky": "Login with Bluesky", "Already_a_user": "Already a user?", "Choose_privacy_setting": "Choose Privacy Setting", "Be_discovered_title": "We want to be discovered", "Be_discovered_description": "Your community will be visible in search results", "Invite_only_title": "We want to be invite only", "Invite_only_description": "Your community will only be accessible through invite links" }🧰 Tools
🪛 RuboCop (1.73)
[fatal] 382-382: unexpected token tSTRING
(Using Ruby 2.7 parser; configure usingTargetRubyVersionparameter, underAllCops)(Lint/Syntax)
test/sign_up_test.dart (1)
107-115:
⚠️ Potential issueTest is incomplete with commented assertions.
The test taps the continue button but doesn't fully verify the outcome as the final assertions are commented out. This limits test coverage of the complete onboarding flow.
Consider uncommenting and fixing the final assertions to verify navigation to the communities screen:
-//await tester.pumpAndSettle(); +await tester.pumpAndSettle(); -// Verify we're on the communities screen -//expect(find.byType(CommunitiesScreen), findsOneWidget); +// Verify we're on the communities screen +expect(find.byType(CommunitiesScreen), findsOneWidget);lib/features/emergency_alert/emergency_alert_controller.dart (1)
14-14:
⚠️ Potential issueHandle potential null nostr instance
The code uses the
!operator without first checking ifnostris null, which could lead to runtime exceptions.- final event = Event( - nostr!.publicKey, + if (nostr == null) { + throw Exception('Nostr client is not initialized'); + } + + final event = Event( + nostr.publicKey,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if (nostr == null) { throw Exception('Nostr client is not initialized'); } final event = Event( nostr.publicKey, // …other parameters… );lib/router/login/welcome_widget.dart (1)
20-49: 🛠️ Refactor suggestion
Localize dialog text
The "Coming soon" dialog text and "Ok" button text are hardcoded and should be localized.
void _showComingSoonAlert(BuildContext context) { final themeData = Theme.of(context); + final localization = S.of(context); showDialog( context: context, builder: (context) => AlertDialog( backgroundColor: themeData.customColors.cardBgColor, content: Text( - "This feature is coming soon", + localization.Feature_coming_soon, style: TextStyle( fontSize: 16, fontWeight: FontWeight.w400, color: themeData.customColors.primaryForegroundColor, ), ), actions: [ TextButton( onPressed: () => Navigator.of(context).pop(), child: Text( - "Ok", + localization.Ok, style: TextStyle( fontSize: 16, fontWeight: FontWeight.w400, color: themeData.customColors.accentColor, ), ), ), ], ), ); }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.void _showComingSoonAlert(BuildContext context) { final themeData = Theme.of(context); final localization = S.of(context); showDialog( context: context, builder: (context) => AlertDialog( backgroundColor: themeData.customColors.cardBgColor, content: Text( localization.Feature_coming_soon, style: TextStyle( fontSize: 16, fontWeight: FontWeight.w400, color: themeData.customColors.primaryForegroundColor, ), ), actions: [ TextButton( onPressed: () => Navigator.of(context).pop(), child: Text( localization.Ok, style: TextStyle( fontSize: 16, fontWeight: FontWeight.w400, color: themeData.customColors.accentColor, ), ), ), ], ), ); }lib/router/onboarding/name_input_step_widget.dart (2)
90-91: 🛠️ Refactor suggestion
Localize field label
The "Your Nickname" label is hardcoded and should be localized.
child: Text( - 'Your Nickname', + localization.Your_Nickname, style: TextStyle(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.child: Text( localization.Your_Nickname, style: TextStyle(
132-133: 🛠️ Refactor suggestion
Localize tip text
The tip text is hardcoded and should be localized for better internationalization support.
child: Text( - 'Tip: Use a random name or create your own. This name may be visible in your communities.', + localization.Name_Input_Tip, style: TextStyle(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.child: Text( localization.Name_Input_Tip, style: TextStyle(lib/router/onboarding/utils/name_generator.dart (1)
148-154: 🛠️ Refactor suggestion
Very small numeric suffix → high collision rate
The numeric part is limited to1‒100, giving only 10 000 possible combinations per color/animal pair and noticeably increasing collision probability whengenerateMultipleNamesis asked for more than a few hundred items.- final number = 1 + _random.nextInt(100); // Range from 1 to 100 + // 0001-9999 provides 9 999 variants while keeping the string compact + final number = 1 + _random.nextInt(9999);lib/features/emergency_alert/emergency_alert_screen.dart (1)
104-142: 🛠️ Refactor suggestion
Multiple rapid taps can send duplicate alerts
The "Send" button remains enabled during the async call, so an impatient user can trigger several identical requests.Disable the button while the future is in flight:
- child: FilledButton( - onPressed: () async { + child: FilledButton( + onPressed: isSending + ? null + : () async { + ref.read(isSendingProvider.notifier).state = true; ... - }, + } finally { + ref.read(isSendingProvider.notifier).state = false; + } },Introduce:
final isSendingProvider = StateProvider<bool>((_) => false);
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
lib/router/onboarding/utils/name_generator.dart (3)
79-150: Duplicate animal categories and entries in the listThere are redundant categories and animal names in the list:
- "Dragons and Wyrms" appears twice (lines 93-95 and 109-111)
- "Mythical Equines" appears twice (lines 97-99 and 113-116)
- Several animals appear multiple times (Dragon, Pegasus, Phoenix, etc.)
While duplicates increase the probability of those animals appearing, consider using weights instead for a more controlled approach.
- // Dragons and Wyrms - 'Dragon', 'Wyrm', 'Drake', 'Wyvern', 'Hydra', 'Serpent', 'Leviathan', - 'Amphiptere', 'Lindworm', 'Salamander', - // Mythical Equines - 'Unicorn', 'Pegasus', 'Kirin', 'Sleipnir', 'Hippocampus', 'Alicorn', - 'Nightmare', 'Kelpie', 'Hippogriff', 'Centaur',
161-168: Consider adding a safety mechanism for uniquenessWhile the current implementation using a Set ensures uniqueness, it could theoretically hang if you request more unique names than possible combinations (though extremely unlikely with the current variety). Consider adding a maximum iteration limit as a safety measure.
static List<String> generateMultipleNames(int count) { final Set<String> names = {}; + int attempts = 0; + const maxAttempts = 1000; // Safety limit - while (names.length < count) { + while (names.length < count && attempts < maxAttempts) { + attempts++; names.add(generateRandomName()); } + if (names.length < count) { + // Handle the case where we couldn't generate enough unique names + } return names.toList(); }
152-159: Consider adding a parameter for controlling name formatThe name generation combines three elements with a fixed format. You might want to make this more flexible by allowing different separators or formats.
- static String generateRandomName() { + static String generateRandomName({String separator = "."}) { final color = _generateColor(); final animal = _generateAnimal(); final number = 1 + _random.nextInt(100); // Range from 1 to 100 - return '$color$animal.$number'; + return '$color$animal$separator$number'; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/router/onboarding/utils/name_generator.dart(1 hunks)test/sign_up_test.dart(2 hunks)
🔇 Additional comments (3)
lib/router/onboarding/utils/name_generator.dart (2)
1-7: Well-designed utility class for username generationThe
NameGeneratorclass provides a clever approach to generating unique, memorable usernames during onboarding. The documentation clearly explains its purpose.
8-76: Color generation looks robust with good varietyThe color generation logic uses HSL color space effectively to create a wide range of visually descriptive color names. The approach with luminosity and saturation prefixes adds nice variation.
test/sign_up_test.dart (1)
13-13: Import change aligns with removal of age verificationThe removal of the CommunitiesScreen import and addition of the OnboardingScreen import aligns with the PR objective of removing the age verification step from the onboarding flow.
| //testWidgets('Sign Up flow with name input', (WidgetTester tester) async { | ||
| // final groupIdentifierRepository = MockGroupIdentifierRepository(); | ||
| // when(groupIdentifierRepository.watchGroupIdentifierList()) | ||
| // .thenAnswer((_) => Stream.value(<GroupIdentifier>[])); | ||
|
|
||
| // // Override the provider with the mocked repository | ||
| // final overrides = [ | ||
| // groupIdentifierRepositoryProvider | ||
| // .overrideWithValue(groupIdentifierRepository), | ||
| // ]; | ||
|
|
||
| // // Launch the app | ||
| // await tester.pumpWidget(ProviderScope( | ||
| // overrides: overrides, | ||
| // child: const MyApp(), | ||
| // )); | ||
| // await tester.pumpAndSettle(); | ||
|
|
||
| // // Verify we're on the welcome screen | ||
| // expect(find.byType(WelcomeWidget), findsOneWidget); | ||
|
|
||
| // // Find and tap the Get Started button | ||
| // await tester.tap(find.byKey(const Key('create_profile_button'))); | ||
| // await tester.pumpAndSettle(); | ||
|
|
||
| // //// Verify we're on the onboarding screen with name input | ||
| // expect(find.byType(OnboardingWidget), findsOneWidget); | ||
| // expect(find.byKey(const Key('name_input_title')), findsOneWidget); | ||
|
|
||
| // //// Enter the name | ||
| // await tester.enterText(find.byKey(const Key('input')), 'Test User'); | ||
| // await tester.pumpAndSettle(); | ||
|
|
||
| // // Find and tap the continue button | ||
| // final continueTextFinder = find.text('Continue'); | ||
| // expect(continueTextFinder, findsOneWidget); | ||
| // await tester.tap(continueTextFinder); | ||
| // //await tester.pumpAndSettle(); | ||
|
|
||
| // // Verify we're on the communities screen | ||
| // //expect(find.byType(CommunitiesScreen), findsOneWidget); | ||
| //}); |
There was a problem hiding this comment.
Uncomment or replace the commented test
The entire sign-up test has been commented out. While this might be a temporary solution during development, it reduces test coverage for the onboarding flow.
Either uncomment the test and update it to match the new flow (removing age verification and updating assertions), or create a new test that properly validates the current implementation.
Having active tests is crucial for ensuring that the new nickname generation feature and onboarding flow work correctly after future changes.
| // //expect(find.byType(CommunitiesScreen), findsOneWidget); | ||
| //}); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify navigation to the communities screen
The assertion for navigating to the Communities screen is commented out. Make sure to include proper verification that the flow correctly takes users to the next screen after the name input step.
🏁 Script executed:
#!/bin/bash
# Check which screen follows the onboarding name input in the app flow
grep -r --include="*.dart" "navigat.*Communit" lib/router/onboarding/
# Alternative: Find where name input proceeds to the next screen
grep -r --include="*.dart" "onNameSubmit\|nameInputContinue" lib/router/onboarding/Length of output: 157
🏁 Script executed:
#!/bin/bash
# Search for the CommunitiesScreen class reference anywhere in the repo
grep -R --include='*.dart' -n 'CommunitiesScreen' .
# Look for explicit Navigator.push calls in lib/
grep -R --include='*.dart' -n 'Navigator.push' lib/
# Look for named route navigation to a “community” route
grep -R --include='*.dart' -n 'pushNamed' lib/ | grep -i communityLength of output: 861
🏁 Script executed:
#!/bin/bash
# Show routing logic around where CommunitiesScreen is used
sed -n '180,260p' lib/router/index/index_widget.dart
# Show sign-up test from name input to the commented assertion
sed -n '80,140p' test/sign_up_test.dartLength of output: 3859
Uncomment and verify navigation to the Communities screen
After tapping the “Continue” button in test/sign_up_test.dart, you should call
await tester.pumpAndSettle();and then assert that the next screen is the CommunitiesScreen. For example, in sign_up_test.dart at lines 114–115:
- // Verify we're on the communities screen
- //expect(find.byType(CommunitiesScreen), findsOneWidget);
+ // Verify we're on the communities screen
+ await tester.pumpAndSettle();
+ expect(find.byType(CommunitiesScreen), findsOneWidget);• File: test/sign_up_test.dart
• Lines: 114–115
|
Awesome work, Itunu! |
2 participants
Issues covered
Description
Enhanced the name input experience in the onboarding flow by implementing nickname generation functionality, removing the age verification step, and improving UI for name handling throughout the app, in case the name is too long for the space it is occupying. The signup test is also commented out for now.
The nickname generation is with the following logic:
Implements a creative username generator that combines colors, animals, and numbers to create unique, memorable usernames. The generator uses:
Color Generation: Creates vibrant color names using HSL color space, with:
Animal Selection: Draws from a rich collection of:
Number Suffix: Adds a random number (1-100) to ensure uniqueness
The generator creates names like "BrightBluePhoenix.42" or "VividEmeraldDragon.7", providing an infinite combination of memorable, fantasy-inspired usernames while maintaining readability and uniqueness.
How to test
Screenshots/Video
New Nickname Input during Onboarding
Broken Long Name
Video:
Screen.Recording.2025-04-25.at.8.03.56.pm.mov
Summary by CodeRabbit