fix(profile): improve validation and fix padding error#66
fix(profile): improve validation and fix padding error#66amankv1234 wants to merge 1 commit intoAOSSIE-Org:mainfrom
Conversation
WalkthroughAdds a new stateful Profile UI that loads/saves a username and persistent ID via SharedPreferences, validates input, previews the composite name+id, and navigates either to HomeScreen (replace) or back (pop) based on an onLogin flag. Uses nanoid for ID generation and updates Global.myName on navigation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant P as Profile Widget
participant SP as SharedPreferences
participant N as Navigator
participant H as HomeScreen
Note over P: Init
P->>SP: read p_name, p_id
SP-->>P: stored values or null
P->>P: set state (name, id, loading=false)
U->>P: edits name in TextField
P->>P: validate (non-empty, no '@', len>3)
U->>P: taps Save
alt form valid
P->>SP: set p_name, p_id
SP-->>P: ack
P->>P: update Global.myName
alt onLogin == true
P->>N: pushReplacement(HomeScreen)
N->>H: build
else onLogin == false
P->>N: pop()
end
else invalid
P-->>U: show validator error / snackbar
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
solution (3)
74-76: Preview doesn’t update while typing
Text('${myName.text}...')won’t rebuild on controller changes. UseValueListenableBuilderto make it reactive.- // show what the final username will look like - Text('Your username will be: ${myName.text}$customLengthId'), + // show what the final username will look like + ValueListenableBuilder<TextEditingValue>( + valueListenable: myName, + builder: (_, value, __) => + Text('Your username will be: ${value.text}$customLengthId'), + ),
51-61: Navigation polish + trim before persisting to GlobalGuard
popto avoid no‑route errors in edge cases and trim the value assigned toGlobal.myName.- Global.myName = myName.text; - if (!widget.onLogin) { - Navigator.pop(context); - } else { + Global.myName = myName.text.trim(); + if (!widget.onLogin) { + if (Navigator.of(context).canPop()) { + Navigator.of(context).pop(); + } else { + Navigator.pushReplacement( + context, + MaterialPageRoute(builder: (context) => const HomeScreen()), + ); + } + } else { Navigator.pushReplacement( context, MaterialPageRoute(builder: (context) => const HomeScreen()), ); }
79-94: Prevent typing “@” at the source with an input formatter (keeps validator too)Optional, but improves UX and reduces error toasts.
TextFormField( controller: myName, + inputFormatters: [FilteringTextInputFormatter.deny(RegExp(r'@'))], decoration: const InputDecoration(Add import:
import 'package:flutter/material.dart'; +import 'package:flutter/services.dart';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
solution(1 hunks)
🔇 Additional comments (1)
solution (1)
51-61: Confirm: should Global.myName include the ID suffix?Preview shows "name+id" but profile.dart assigns Global.myName = name (no id); Global.myName is used as the P2P deviceName and as message sender/receiver — a mismatch will cause routing/peer-match failures if peers use "name+id".
- lib/pages/profile.dart — getString('p_name') at :50, getString('p_id') at :51; prefs.setString('p_name')/:p_id at :79–:80; Global.myName = name at :61 and :81.
- lib/p2p/adhoc_housekeeping.dart — "sender": Global.myName at :211 and :241; compare decodedMessage['receiver'] == Global.myName at :366; nearby init deviceName: Global.myName at :386.
- lib/components/message_panel.dart — message payloads use Global.myName as sender and in Payload constructors (e.g., :176, :185, :206, :322, :329).
- lib/classes/global.dart — static String myName = '' at :27.
Decide canonical identifier: if canonical ID is "name+id", set Global.myName to include the id before navigation (and persist the same string); otherwise ensure peers/preview use only the plain name so comparisons remain consistent.
| Future<void> getDetails() async { | ||
| final prefs = await SharedPreferences.getInstance(); | ||
| final name = prefs.getString('p_name') ?? ''; | ||
| final id = prefs.getString('p_id') ?? ''; | ||
|
|
||
| setState(() { | ||
| myName.text = name; | ||
| if (id.isNotEmpty) customLengthId = id; | ||
| loading = false; | ||
| }); | ||
|
|
||
| // If profile already exists and we arrived here from login flow, go home | ||
| if (name.isNotEmpty && id.isNotEmpty && widget.onLogin) { | ||
| navigateToHomeScreen(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix: avoid setState/navigation after dispose; schedule post‑frame navigation
getDetails awaits then calls setState and may navigate immediately. If the widget unmounts meanwhile, this will throw. Add mounted checks and schedule navigation with addPostFrameCallback.
Future<void> getDetails() async {
- final prefs = await SharedPreferences.getInstance();
- final name = prefs.getString('p_name') ?? '';
- final id = prefs.getString('p_id') ?? '';
+ final prefs = await SharedPreferences.getInstance();
+ if (!mounted) return;
+ final name = prefs.getString('p_name') ?? '';
+ final id = prefs.getString('p_id') ?? '';
- setState(() {
- myName.text = name;
- if (id.isNotEmpty) customLengthId = id;
- loading = false;
- });
+ if (!mounted) return;
+ setState(() {
+ myName.text = name;
+ if (id.isNotEmpty) customLengthId = id;
+ loading = false;
+ });
// If profile already exists and we arrived here from login flow, go home
- if (name.isNotEmpty && id.isNotEmpty && widget.onLogin) {
- navigateToHomeScreen();
- }
+ if (name.isNotEmpty && id.isNotEmpty && widget.onLogin) {
+ WidgetsBinding.instance.addPostFrameCallback((_) {
+ if (!mounted) return;
+ navigateToHomeScreen();
+ });
+ }
}📝 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.
| Future<void> getDetails() async { | |
| final prefs = await SharedPreferences.getInstance(); | |
| final name = prefs.getString('p_name') ?? ''; | |
| final id = prefs.getString('p_id') ?? ''; | |
| setState(() { | |
| myName.text = name; | |
| if (id.isNotEmpty) customLengthId = id; | |
| loading = false; | |
| }); | |
| // If profile already exists and we arrived here from login flow, go home | |
| if (name.isNotEmpty && id.isNotEmpty && widget.onLogin) { | |
| navigateToHomeScreen(); | |
| } | |
| } | |
| Future<void> getDetails() async { | |
| final prefs = await SharedPreferences.getInstance(); | |
| if (!mounted) return; | |
| final name = prefs.getString('p_name') ?? ''; | |
| final id = prefs.getString('p_id') ?? ''; | |
| if (!mounted) return; | |
| setState(() { | |
| myName.text = name; | |
| if (id.isNotEmpty) customLengthId = id; | |
| loading = false; | |
| }); | |
| // If profile already exists and we arrived here from login flow, go home | |
| if (name.isNotEmpty && id.isNotEmpty && widget.onLogin) { | |
| WidgetsBinding.instance.addPostFrameCallback((_) { | |
| if (!mounted) return; | |
| navigateToHomeScreen(); | |
| }); | |
| } | |
| } |
🤖 Prompt for AI Agents
In solution around lines 34 to 49, getDetails awaits async work then calls
setState and may immediately navigate; if the widget was disposed in the
meantime this will throw. Fix by checking mounted before calling setState (skip
updating state if not mounted), and when deciding to navigate schedule the
navigation in a post-frame callback
(WidgetsBinding.instance.addPostFrameCallback) and re-check mounted before
calling navigateToHomeScreen; ensure no setState or navigation runs when mounted
is false.
| final v = value?.trim() ?? ''; | ||
| if (v.isEmpty) return 'Please enter a name'; | ||
| if (v.contains('@')) return 'Do not use the @ character'; | ||
| if (v.length <= 3) return 'Name should be greater than 3 characters'; | ||
| return null; |
There was a problem hiding this comment.
Validation off‑by‑one vs PR objective (“minimum of 3 characters”)
Current rule enforces 4+. Adjust to 3+ and fix the message.
- if (v.length <= 3) return 'Name should be greater than 3 characters';
+ if (v.length < 3) return 'Name must be at least 3 characters';📝 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.
| final v = value?.trim() ?? ''; | |
| if (v.isEmpty) return 'Please enter a name'; | |
| if (v.contains('@')) return 'Do not use the @ character'; | |
| if (v.length <= 3) return 'Name should be greater than 3 characters'; | |
| return null; | |
| final v = value?.trim() ?? ''; | |
| if (v.isEmpty) return 'Please enter a name'; | |
| if (v.contains('@')) return 'Do not use the @ character'; | |
| if (v.length < 3) return 'Name must be at least 3 characters'; | |
| return null; |
🤖 Prompt for AI Agents
In solution around lines 88 to 92, the validator currently rejects names of
length 3 (using v.length <= 3) and the message says "greater than 3 characters"
which conflicts with the PR objective of a minimum of 3; change the length check
to v.length < 3 and update the message to something like "Name should be at
least 3 characters" (keep existing trim and other checks unchanged).
|
add my code to solve your first issue |
1 participant
What
Why
How
Summary by CodeRabbit