Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCentralizes UI/design tokens, refactors the Edit Profile panel into smaller widgets with a single save/upload flow that uploads images, persists profile fields, and awards coins; converts Profile Completeness card to a stateful animated component; and migrates multiple screens/widgets to use the new token set and updated styles. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EditProfilePanel
participant StorageService
participant ProfileRepository
participant CoinsService
participant Navigator
participant Toast
User->>EditProfilePanel: Tap Save/Update
EditProfilePanel->>EditProfilePanel: validate inputs & set isLoading
EditProfilePanel->>StorageService: upload avatar/cover (if changed)
StorageService-->>EditProfilePanel: return uploaded URLs
EditProfilePanel->>ProfileRepository: persist username/name/bio/links + image URLs
ProfileRepository-->>EditProfilePanel: persist result
EditProfilePanel->>CoinsService: maybeAwardProfileCompletion()
CoinsService-->>EditProfilePanel: award result
EditProfilePanel->>Navigator: pop edit panel (if mounted)
EditProfilePanel->>Toast: show success toast
EditProfilePanel->>EditProfilePanel: clear isLoading
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/core/widgets/popup/editProfilePanel.dart`:
- Around line 370-384: The _hasChanges / _saveProfile flow is allowing usernames
to be written without confirming availability and can be overwritten by stale
async checks; update _saveProfile to only proceed with assigning and sending
{"username": ...} when usernameEdit is true AND the availability flag is true
(e.g., available == true) and the current usernameController.text matches the
value used for the most recent availability check (or use a small
requestId/token recorded when triggering availability lookup to validate the
response) so stale async responses can't permit a taken name; apply the same
guard/validation to the other save path that updates username (the similar block
around where username is handled later in the file).
- Around line 401-410: The save logic re-introduces deleted links because
_ProfileLinkOption.value entries in linkIcons are never cleared and
_saveProfile/_saveProfile() only merges non-empty option values into
app_state.prismUser.links; update the flow so that when a link is removed you
also clear the corresponding _ProfileLinkOption.value (set to empty string)
and/or adjust the save routine (in the block guarded by linkEdit that iterates
linkIcons) to remove keys from the links map when icon.value is empty (instead
of only adding non-empty values), then persist and call _updateCurrentUser with
the cleaned map; reference linkEdit, linkIcons, _ProfileLinkOption.value,
app_state.prismUser.links and _updateCurrentUser to locate and change the code.
- Around line 374-418: The _saveProfile() flow must be made failure-safe: wrap
the sequence of remote calls (_updateCurrentUser,
processImage/processImageCover,
CoinsService.instance.maybeAwardProfileCompletion) in try/catch/finally so any
exception doesn't leave isLoading true or call setState after dispose; move
setState(()=>isLoading=true) to before try, and ensure
setState(()=>isLoading=false) is run in finally guarded by if(mounted). Avoid
mutating and persisting app_state.prismUser before the corresponding remote
write succeeds—use local copies (e.g., local variables for username, name, bio,
links, pfp/cover) and only assign to/app_state.persistPrismUser() after each
successful _updateCurrentUser, or on catch revert persisted changes and
re-persist; also only Navigator.pop(context) and toasts.codeSend("Profile
updated!") when all operations completed successfully and mounted. Ensure
processImage/processImageCover exceptions are handled similarly so they don't
short-circuit cleanup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e85dd860-812e-4295-9734-8e8e422904b8
📒 Files selected for processing (3)
lib/core/widgets/popup/editProfilePanel.dartlib/features/profile_completeness/views/widgets/profile_completeness_card.darttest/features/profile_completeness/profile_completeness_widgets_test.dart
Comment on lines
+370
to
+384
| bool get _hasChanges => | ||
| (!usernameEdit && (pfpEdit || bioEdit || linkEdit || coverEdit || nameEdit)) || | ||
| (usernameEdit && enabled); | ||
|
|
||
| Future<void> _saveProfile() async { | ||
| setState(() => isLoading = true); | ||
|
|
||
| if (usernameEdit && usernameController.text.isNotEmpty && usernameController.text.length >= 8) { | ||
| app_state.prismUser.username = usernameController.text; | ||
| app_state.persistPrismUser(); | ||
| await _updateCurrentUser( | ||
| <String, dynamic>{"username": usernameController.text}, | ||
| 'profile.edit.username', | ||
| ); | ||
| } |
There was a problem hiding this comment.
Don't let Update bypass username availability.
_hasChanges becomes true as soon as the username is syntactically valid, but _saveProfile() never requires available == true. Because the lookup is async, an older response can also overwrite the state for the latest input. That means a taken username can still reach the write path.
🔧 Suggested fix
+ bool get _canSubmitUsername =>
+ usernameEdit && enabled && available == true && !isCheckingUsername;
+
bool get _hasChanges =>
(!usernameEdit && (pfpEdit || bioEdit || linkEdit || coverEdit || nameEdit)) ||
- (usernameEdit && enabled);
+ _canSubmitUsername;
Future<void> _saveProfile() async {
setState(() => isLoading = true);
- if (usernameEdit && usernameController.text.isNotEmpty && usernameController.text.length >= 8) {
+ if (_canSubmitUsername) {
app_state.prismUser.username = usernameController.text;
app_state.persistPrismUser();
await _updateCurrentUser(
<String, dynamic>{"username": usernameController.text},
'profile.edit.username',
@@
onChanged: (value) async {
final valid = value.isNotEmpty && value.length >= 8 && !value.contains(RegExp(r"(?: |[^\w\s])+"));
setState(() => enabled = valid);
if (valid) {
+ final requested = value;
setState(() => isCheckingUsername = true);
- final isAvailable = await _isUsernameAvailable(value);
- if (mounted) {
+ final isAvailable = await _isUsernameAvailable(requested);
+ if (mounted && usernameController.text == requested) {
setState(() {
available = isAvailable;
isCheckingUsername = false;
});
}
} else {
- setState(() => available = null);
+ setState(() {
+ available = null;
+ isCheckingUsername = false;
+ });
}Also applies to: 719-740
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/core/widgets/popup/editProfilePanel.dart` around lines 370 - 384, The
_hasChanges / _saveProfile flow is allowing usernames to be written without
confirming availability and can be overwritten by stale async checks; update
_saveProfile to only proceed with assigning and sending {"username": ...} when
usernameEdit is true AND the availability flag is true (e.g., available == true)
and the current usernameController.text matches the value used for the most
recent availability check (or use a small requestId/token recorded when
triggering availability lookup to validate the response) so stale async
responses can't permit a taken name; apply the same guard/validation to the
other save path that updates username (the similar block around where username
is handled later in the file).
Comment on lines
+374
to
+418
| Future<void> _saveProfile() async { | ||
| setState(() => isLoading = true); | ||
|
|
||
| if (usernameEdit && usernameController.text.isNotEmpty && usernameController.text.length >= 8) { | ||
| app_state.prismUser.username = usernameController.text; | ||
| app_state.persistPrismUser(); | ||
| await _updateCurrentUser( | ||
| <String, dynamic>{"username": usernameController.text}, | ||
| 'profile.edit.username', | ||
| ); | ||
| } | ||
| if (_pfp != null && pfpEdit) { | ||
| await processImage(); | ||
| } | ||
| if (_cover != null && coverEdit) { | ||
| await processImageCover(); | ||
| } | ||
| if (bioEdit && bioController.text.isNotEmpty) { | ||
| app_state.prismUser.bio = bioController.text; | ||
| app_state.persistPrismUser(); | ||
| await _updateCurrentUser(<String, dynamic>{"bio": bioController.text}, 'profile.edit.bio'); | ||
| } | ||
| if (nameEdit && nameController.text.isNotEmpty) { | ||
| app_state.prismUser.name = nameController.text; | ||
| app_state.persistPrismUser(); | ||
| await _updateCurrentUser(<String, dynamic>{"name": nameController.text}, 'profile.edit.name'); | ||
| } | ||
| if (linkEdit) { | ||
| final Map<String, String> links = Map<String, String>.from(app_state.prismUser.links); | ||
| for (final icon in linkIcons) { | ||
| if (icon.value.isNotEmpty) { | ||
| links[icon.name] = icon.value; | ||
| } | ||
| } | ||
| app_state.prismUser.links = links; | ||
| app_state.persistPrismUser(); | ||
| await _updateCurrentUser(<String, dynamic>{"links": links}, 'profile.edit.links'); | ||
| } | ||
|
|
||
| await CoinsService.instance.maybeAwardProfileCompletion(); | ||
| setState(() => isLoading = false); | ||
| if (mounted) { | ||
| Navigator.pop(context); | ||
| toasts.codeSend("Profile updated!"); | ||
| } |
There was a problem hiding this comment.
Make _saveProfile() failure-safe.
Any exception from _updateCurrentUser() or maybeAwardProfileCompletion() skips the cleanup path. That leaves the panel stuck in loading, and if the route is dismissed during an await then Line 414 can call setState() after dispose. Because the local app_state.prismUser mutations happen before the remote write succeeds, failures also leave the cached profile out of sync with Firestore.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/core/widgets/popup/editProfilePanel.dart` around lines 374 - 418, The
_saveProfile() flow must be made failure-safe: wrap the sequence of remote calls
(_updateCurrentUser, processImage/processImageCover,
CoinsService.instance.maybeAwardProfileCompletion) in try/catch/finally so any
exception doesn't leave isLoading true or call setState after dispose; move
setState(()=>isLoading=true) to before try, and ensure
setState(()=>isLoading=false) is run in finally guarded by if(mounted). Avoid
mutating and persisting app_state.prismUser before the corresponding remote
write succeeds—use local copies (e.g., local variables for username, name, bio,
links, pfp/cover) and only assign to/app_state.persistPrismUser() after each
successful _updateCurrentUser, or on catch revert persisted changes and
re-persist; also only Navigator.pop(context) and toasts.codeSend("Profile
updated!") when all operations completed successfully and mounted. Ensure
processImage/processImageCover exceptions are handled similarly so they don't
short-circuit cleanup.
Comment on lines
+401
to
+410
| if (linkEdit) { | ||
| final Map<String, String> links = Map<String, String>.from(app_state.prismUser.links); | ||
| for (final icon in linkIcons) { | ||
| if (icon.value.isNotEmpty) { | ||
| links[icon.name] = icon.value; | ||
| } | ||
| } | ||
| app_state.prismUser.links = links; | ||
| app_state.persistPrismUser(); | ||
| await _updateCurrentUser(<String, dynamic>{"links": links}, 'profile.edit.links'); |
There was a problem hiding this comment.
Cleared links can come back on the next save.
The remove dialog deletes the key from app_state.prismUser.links, but the corresponding _ProfileLinkOption.value is never cleared. _saveProfile() then merges every non-empty option back into the map and never removes empty keys, so deleted or manually-cleared links can be reintroduced.
🔧 Suggested fix
() async {
linkController.text = '';
- final links = app_state.prismUser.links;
+ if (_link != null) _link!.value = '';
+ final links = Map<String, String>.from(app_state.prismUser.links);
links.remove(_link?.name);
app_state.prismUser.links = links;
app_state.persistPrismUser();
await _updateCurrentUser(
<String, dynamic>{"links": app_state.prismUser.links},
@@
if (linkEdit) {
final Map<String, String> links = Map<String, String>.from(app_state.prismUser.links);
for (final icon in linkIcons) {
if (icon.value.isNotEmpty) {
links[icon.name] = icon.value;
+ } else {
+ links.remove(icon.name);
}
}Also applies to: 845-857
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/core/widgets/popup/editProfilePanel.dart` around lines 401 - 410, The
save logic re-introduces deleted links because _ProfileLinkOption.value entries
in linkIcons are never cleared and _saveProfile/_saveProfile() only merges
non-empty option values into app_state.prismUser.links; update the flow so that
when a link is removed you also clear the corresponding _ProfileLinkOption.value
(set to empty string) and/or adjust the save routine (in the block guarded by
linkEdit that iterates linkIcons) to remove keys from the links map when
icon.value is empty (instead of only adding non-empty values), then persist and
call _updateCurrentUser with the cleaned map; reference linkEdit, linkIcons,
_ProfileLinkOption.value, app_state.prismUser.links and _updateCurrentUser to
locate and change the code.
Android App Size ReportStatus: PASS
Thresholds: max delta Top artifact bucket deltas (uncompressed bytes):
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/features/navigation/views/widgets/personalized_feed_settings_bottom_sheet.dart (1)
43-50:⚠️ Potential issue | 🟠 MajorOnly dismiss the sheet after a confirmed save.
The callback can return early at Lines 44-47 when
_persistInterests()fails, but_save()still pops the bottom sheet unconditionally. That closes the retry path and makes a partial save look successful. HaveonSavereturn a success flag orResult, only pop on success, and reset_savinginfinallywhen the save is rejected or throws.Also applies to: 105-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/navigation/views/widgets/personalized_feed_settings_bottom_sheet.dart` around lines 43 - 50, The onSave handler currently returns early when _persistInterests(settingsLocal, interests) fails but _save still pops the bottom sheet unconditionally; change onSave (both occurrences around the current block and the similar block at 105-109) to return a boolean or a Result indicating success, have _save await that result and only call Navigator.pop (or popBottomSheet) when the save succeeded, and ensure _saving is reset in a finally block so it is cleared on failure or exception; update calls to settingsLocal.set and onPreferencesSaved to occur only after a confirmed successful persist and propagate failure upward so the UI can show retry state.
♻️ Duplicate comments (3)
lib/core/widgets/popup/editProfilePanel.dart (3)
396-405:⚠️ Potential issue | 🟠 MajorDeleted links can still be reintroduced on the next save.
The remove action only mutates
app_state.prismUser.links._ProfileLinkOption.valuestays populated, and_saveProfile()only adds non-empty option values back into the outgoing map. A later unrelated save can therefore restore a link the user already removed. Clear the corresponding option value when removing, and remove empty keys while rebuildinglinks.Also applies to: 829-849
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/core/widgets/popup/editProfilePanel.dart` around lines 396 - 405, When saving links in the linkEdit block the code only updates app_state.prismUser.links but leaves each _ProfileLinkOption.value populated so removed links can reappear later; update the loop that builds links (where linkIcons is iterated in the linkEdit branch) to skip any empty values and also clear the corresponding _ProfileLinkOption.value when a link is removed, and when reconstructing links ensure you filter out empty keys (e.g., rebuild links from linkIcons but only include non-empty icon.value and delete any empty entries before assigning to app_state.prismUser.links and calling _updateCurrentUser/_saveProfile()).
369-379:⚠️ Potential issue | 🟠 MajorDon't let
Updatesubmit a taken or stale username.
_hasChangesand_saveProfile()still only gate on syntactic validity, soavailable == falsecan still reach the write path. The async lookup also writes back unconditionally, which lets an older availability response overwrite the current field state. Gate submit onavailable == true && !isCheckingUsername, and only apply a lookup result if it still matchesusernameController.text.Also applies to: 698-718
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/core/widgets/popup/editProfilePanel.dart` around lines 369 - 379, The save path currently allows submitting stale/taken usernames because _hasChanges and _saveProfile only check syntactic rules; modify _hasChanges and the save guard in _saveProfile (and the other save block around the code referenced later) to require available == true and isCheckingUsername == false when usernameEdit is true, and in your async username-availability callback only update/apply the lookup result (and app_state.prismUser.username/persistPrismUser) if the returned result corresponds to the current usernameController.text (i.e., compare the checked string to usernameController.text before writing) to avoid overwriting with stale responses.
372-414:⚠️ Potential issue | 🟠 MajorMake
_saveProfile()failure-safe before closing the panel.Any exception from
_updateCurrentUser()ormaybeAwardProfileCompletion()skips the cleanup path, leavingisLoadingstuck and making Line 409 unsafe if the route is disposed mid-save.processImage()/processImageCover()also swallow upload failures, so this method can still pop and toast success after a partial save. Wrap the whole flow intry/catch/finally, rethrow upload errors, and only persist local profile state after each remote write succeeds.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/core/widgets/popup/editProfilePanel.dart` around lines 372 - 414, The _saveProfile method must be made failure-safe: wrap the entire save flow in try/catch/finally so isLoading is always reset in finally (use setState(() => isLoading = false)), only call app_state.persistPrismUser after the corresponding remote call (_updateCurrentUser or upload functions) succeeds, let processImage and processImageCover propagate upload exceptions (rethrow) so callers can handle them, and only call Navigator.pop(context) and toasts.codeSend("Profile updated!") when mounted and no exception occurred; catch errors to show or rethrow as appropriate but avoid leaving the UI stuck or popping on partial/failed saves.
🧹 Nitpick comments (2)
lib/features/profile_completeness/views/widgets/profile_completeness_card.dart (1)
97-107: Avoid duplicating the completion reward amount.This subtitle hard-codes
25 coins, while the actual award comes fromCoinPolicy.profileCompletioninlib/core/coins/coins_service.dart. If that policy changes, the card becomes wrong immediately. Pull the display value from the same shared constant, or pass it into the widget.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/profile_completeness/views/widgets/profile_completeness_card.dart` around lines 97 - 107, The subtitle in ProfileCompletenessCard is hard-coded to '25 coins' causing duplication with the canonical value; update the Text that currently renders ' 25 coins' to use the shared constant CoinPolicy.profileCompletion (or accept the coinAmount via the widget constructor and use that prop) so the UI reads the dynamic value (e.g., `${CoinPolicy.profileCompletion} coins`) and remains correct if the policy changes; adjust imports to include CoinPolicy from lib/core/coins/coins_service.dart and update any widget tests if they assert the hard-coded string.lib/features/personalized_feed/views/pages/personalized_feed_screen.dart (1)
33-34: Keep the carousel length derived from the tokenized preview count.
_carouselPreviewCountis now the source of truth, but_FeedCarouselstill usesitemCount: 6. IfPrismFeedLayout.carouselPreviewCountchanges, the preview slice andfeedIndex = i - 2math drift apart. Derive the total from_carouselPreviewCount + 2in both places.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/personalized_feed/views/pages/personalized_feed_screen.dart` around lines 33 - 34, The carousel length is hardcoded to 6 while the source of truth is _carouselPreviewCount (aliasing PrismFeedLayout.carouselPreviewCount), causing drift; update _FeedCarousel's itemCount to use (_carouselPreviewCount + 2) instead of 6 and adjust any index math that assumes the hardcoded size (e.g., replace uses of feedIndex = i - 2 with feedIndex = i - (_carouselPreviewCount + 2 - _carouselPreviewCount) or simply compute a constant offset = 2 and derive feedIndex = i - offset so the slice logic stays consistent with _carouselPreviewCount), ensuring both the preview slice and index calculations derive from _carouselPreviewCount + 2.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@lib/features/navigation/views/widgets/personalized_feed_settings_bottom_sheet.dart`:
- Line 30: Normalize and validate the stored feed-mix string returned from
settingsLocal.get (the currentMix variable) before binding to the selector: trim
and lowercase currentMix, check it against the allowed mix values used by the
selector (ensure it includes the fallback _kDefaultFeedMix e.g. "balanced"), and
if it is not in that allowed set replace it with _kDefaultFeedMix; then pass
that sanitized value as initialFeedMix to SegmentedButton.selected so
stale/malformed values cannot produce an unmatched segment or be written back
unchanged.
---
Outside diff comments:
In
`@lib/features/navigation/views/widgets/personalized_feed_settings_bottom_sheet.dart`:
- Around line 43-50: The onSave handler currently returns early when
_persistInterests(settingsLocal, interests) fails but _save still pops the
bottom sheet unconditionally; change onSave (both occurrences around the current
block and the similar block at 105-109) to return a boolean or a Result
indicating success, have _save await that result and only call Navigator.pop (or
popBottomSheet) when the save succeeded, and ensure _saving is reset in a
finally block so it is cleared on failure or exception; update calls to
settingsLocal.set and onPreferencesSaved to occur only after a confirmed
successful persist and propagate failure upward so the UI can show retry state.
---
Duplicate comments:
In `@lib/core/widgets/popup/editProfilePanel.dart`:
- Around line 396-405: When saving links in the linkEdit block the code only
updates app_state.prismUser.links but leaves each _ProfileLinkOption.value
populated so removed links can reappear later; update the loop that builds links
(where linkIcons is iterated in the linkEdit branch) to skip any empty values
and also clear the corresponding _ProfileLinkOption.value when a link is
removed, and when reconstructing links ensure you filter out empty keys (e.g.,
rebuild links from linkIcons but only include non-empty icon.value and delete
any empty entries before assigning to app_state.prismUser.links and calling
_updateCurrentUser/_saveProfile()).
- Around line 369-379: The save path currently allows submitting stale/taken
usernames because _hasChanges and _saveProfile only check syntactic rules;
modify _hasChanges and the save guard in _saveProfile (and the other save block
around the code referenced later) to require available == true and
isCheckingUsername == false when usernameEdit is true, and in your async
username-availability callback only update/apply the lookup result (and
app_state.prismUser.username/persistPrismUser) if the returned result
corresponds to the current usernameController.text (i.e., compare the checked
string to usernameController.text before writing) to avoid overwriting with
stale responses.
- Around line 372-414: The _saveProfile method must be made failure-safe: wrap
the entire save flow in try/catch/finally so isLoading is always reset in
finally (use setState(() => isLoading = false)), only call
app_state.persistPrismUser after the corresponding remote call
(_updateCurrentUser or upload functions) succeeds, let processImage and
processImageCover propagate upload exceptions (rethrow) so callers can handle
them, and only call Navigator.pop(context) and toasts.codeSend("Profile
updated!") when mounted and no exception occurred; catch errors to show or
rethrow as appropriate but avoid leaving the UI stuck or popping on
partial/failed saves.
---
Nitpick comments:
In `@lib/features/personalized_feed/views/pages/personalized_feed_screen.dart`:
- Around line 33-34: The carousel length is hardcoded to 6 while the source of
truth is _carouselPreviewCount (aliasing PrismFeedLayout.carouselPreviewCount),
causing drift; update _FeedCarousel's itemCount to use (_carouselPreviewCount +
2) instead of 6 and adjust any index math that assumes the hardcoded size (e.g.,
replace uses of feedIndex = i - 2 with feedIndex = i - (_carouselPreviewCount +
2 - _carouselPreviewCount) or simply compute a constant offset = 2 and derive
feedIndex = i - offset so the slice logic stays consistent with
_carouselPreviewCount), ensuring both the preview slice and index calculations
derive from _carouselPreviewCount + 2.
In
`@lib/features/profile_completeness/views/widgets/profile_completeness_card.dart`:
- Around line 97-107: The subtitle in ProfileCompletenessCard is hard-coded to
'25 coins' causing duplication with the canonical value; update the Text that
currently renders ' 25 coins' to use the shared constant
CoinPolicy.profileCompletion (or accept the coinAmount via the widget
constructor and use that prop) so the UI reads the dynamic value (e.g.,
`${CoinPolicy.profileCompletion} coins`) and remains correct if the policy
changes; adjust imports to include CoinPolicy from
lib/core/coins/coins_service.dart and update any widget tests if they assert the
hard-coded string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b70a572-e644-4855-a0cc-fb44a5a9cbc1
📒 Files selected for processing (7)
lib/core/widgets/popup/editProfilePanel.dartlib/features/navigation/views/widgets/personalized_feed_settings_bottom_sheet.dartlib/features/navigation/views/widgets/prism_top_app_bar.dartlib/features/personalized_feed/views/pages/personalized_feed_screen.dartlib/features/personalized_feed/views/widgets/empty_card.dartlib/features/profile_completeness/views/widgets/profile_completeness_card.dartlib/theme/app_tokens.dart
| selected = PersonalizedInterestsCatalog.defaultSelection(catalog).toSet(); | ||
| } | ||
| final String currentMix = settingsLocal.get<String>(personalizedFeedMixLocalKey, defaultValue: 'balanced'); | ||
| final String currentMix = settingsLocal.get<String>(personalizedFeedMixLocalKey, defaultValue: _kDefaultFeedMix); |
There was a problem hiding this comment.
Normalize the stored feed-mix value before binding it to the selector.
The repository already trims/lowercases this preference and falls back to balanced, but the sheet reads the raw string. Any stale or malformed value leaves SegmentedButton.selected with no matching segment and can be written back unchanged on the next save. Sanitize against the allowed set before passing initialFeedMix.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@lib/features/navigation/views/widgets/personalized_feed_settings_bottom_sheet.dart`
at line 30, Normalize and validate the stored feed-mix string returned from
settingsLocal.get (the currentMix variable) before binding to the selector: trim
and lowercase currentMix, check it against the allowed mix values used by the
selector (ensure it includes the fallback _kDefaultFeedMix e.g. "balanced"), and
if it is not in that allowed set replace it with _kDefaultFeedMix; then pass
that sanitized value as initialFeedMix to SegmentedButton.selected so
stale/malformed values cannot produce an unmatched segment or be written back
unchanged.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
lib/features/public_profile/views/pages/profile_screen.dart (2)
614-615: Remove duplicated inline comment.The “Following count — tappable on own profile only” comment appears twice consecutively.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/public_profile/views/pages/profile_screen.dart` around lines 614 - 615, Remove the duplicated inline comment "Following count — tappable on own profile only" so only a single instance remains adjacent to the following count UI; locate the duplicate in profile_screen.dart near the code that renders the following count (e.g., inside the widget/build method that constructs the profile header or the following count tap handler) and delete the extra comment line while keeping the original comment intact.
881-921: Refine compact count formatting to avoid awkward outputs (1.0k, edge rounding).Current formatting can show trailing
.0and can round high-thousand values awkwardly. A small helper keeps display cleaner and easier to maintain.💡 Suggested refactor
class _StatPill extends StatelessWidget { const _StatPill({required this.count, required this.label}); final int count; final String label; + + String _formatCount(int value) { + if (value < 1000) return '$value'; + if (value < 1000000) { + final bool showDecimal = value < 10000 && (value % 1000 != 0); + final String raw = (value / 1000).toStringAsFixed(showDecimal ? 1 : 0); + return '${raw.replaceFirst(RegExp(r'\.0$'), '')}k'; + } + final bool showDecimal = value < 10000000 && (value % 1000000 != 0); + final String raw = (value / 1000000).toStringAsFixed(showDecimal ? 1 : 0); + return '${raw.replaceFirst(RegExp(r'\.0$'), '')}m'; + } `@override` Widget build(BuildContext context) { final secondary = Theme.of(context).colorScheme.secondary; return RichText( @@ - text: count >= 1000 ? '${(count / 1000).toStringAsFixed(count >= 10000 ? 0 : 1)}k' : '$count', + text: _formatCount(count), style: TextStyle(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/public_profile/views/pages/profile_screen.dart` around lines 881 - 921, The count formatting in _StatPill.build currently uses a raw toStringAsFixed expression which produces awkward outputs like "1.0k" and can round high-thousand values unexpectedly; extract a small helper (e.g., String formatCount(int count)) that returns plain integers under 1000, returns "Nk" with no trailing ".0" when the value is an exact thousand (e.g., 1000 -> "1k"), and returns a single decimal (trimmed of trailing zeros) for fractional thousands only when needed (e.g., 1500 -> "1.5k"), then replace the inline expression in the TextSpan text with a call to formatCount(count) so formatting is centralized and easier to maintain.lib/features/profile_completeness/views/widgets/profile_completeness_card.dart (1)
45-45: UsesendAnnouncement+ ambient direction here.
SemanticsService.announceis deprecated and called out as incompatible with multiple windows. This also hardcodesTextDirection.ltr; preferSemanticsService.sendAnnouncement(View.of(context), ..., Directionality.of(context)), and optionally guard it withMediaQuery.supportsAnnounceOf(context). (api.flutter.dev)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/profile_completeness/views/widgets/profile_completeness_card.dart` at line 45, Replace the deprecated SemanticsService.announce call with SemanticsService.sendAnnouncement using the current view and ambient text direction: call SemanticsService.sendAnnouncement(View.of(context), 'Opening profile editor', Directionality.of(context)); and wrap it in a guard like if (MediaQuery.supportsAnnounceOf(context)) to avoid announcing on platforms that don't support it; ensure this change uses the existing BuildContext where the original announce was invoked (refer to SemanticsService.sendAnnouncement, View.of(context), Directionality.of(context), and MediaQuery.supportsAnnounceOf).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@lib/features/profile_completeness/views/widgets/profile_completeness_card.dart`:
- Around line 123-142: The CTA remains visually enabled while _isLoading is
true; update the FilledButton's onPressed logic to also check _isLoading and
return null when loading (i.e., make onPressed null if widget.onCompleteNow ==
null OR _isLoading) so the button is disabled during the spinner display; adjust
the onPressed expression where FilledButton is defined (referencing
FilledButton, _isLoading, widget.onCompleteNow, and _handleComplete) so the
interaction and accessibility states match the AnimatedSwitcher loading state.
In `@lib/features/public_profile/views/pages/profile_screen.dart`:
- Around line 589-607: The bio emptiness check uses (widget.bio ??
"").isNotEmpty which treats whitespace-only strings as non-empty; update the
conditional checks to use (widget.bio ?? "").trim().isNotEmpty and pass
widget.bio!.trim() into the Text widget (and where the extra SizedBox is
conditionally rendered) so whitespace-only bios don't render an empty bio block
or spacing; update both occurrences referencing widget.bio and the Text display
in this widget tree.
---
Nitpick comments:
In
`@lib/features/profile_completeness/views/widgets/profile_completeness_card.dart`:
- Line 45: Replace the deprecated SemanticsService.announce call with
SemanticsService.sendAnnouncement using the current view and ambient text
direction: call SemanticsService.sendAnnouncement(View.of(context), 'Opening
profile editor', Directionality.of(context)); and wrap it in a guard like if
(MediaQuery.supportsAnnounceOf(context)) to avoid announcing on platforms that
don't support it; ensure this change uses the existing BuildContext where the
original announce was invoked (refer to SemanticsService.sendAnnouncement,
View.of(context), Directionality.of(context), and
MediaQuery.supportsAnnounceOf).
In `@lib/features/public_profile/views/pages/profile_screen.dart`:
- Around line 614-615: Remove the duplicated inline comment "Following count —
tappable on own profile only" so only a single instance remains adjacent to the
following count UI; locate the duplicate in profile_screen.dart near the code
that renders the following count (e.g., inside the widget/build method that
constructs the profile header or the following count tap handler) and delete the
extra comment line while keeping the original comment intact.
- Around line 881-921: The count formatting in _StatPill.build currently uses a
raw toStringAsFixed expression which produces awkward outputs like "1.0k" and
can round high-thousand values unexpectedly; extract a small helper (e.g.,
String formatCount(int count)) that returns plain integers under 1000, returns
"Nk" with no trailing ".0" when the value is an exact thousand (e.g., 1000 ->
"1k"), and returns a single decimal (trimmed of trailing zeros) for fractional
thousands only when needed (e.g., 1500 -> "1.5k"), then replace the inline
expression in the TextSpan text with a call to formatCount(count) so formatting
is centralized and easier to maintain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 837def7c-7419-4663-a47b-a6851a787f3b
📒 Files selected for processing (2)
lib/features/profile_completeness/views/widgets/profile_completeness_card.dartlib/features/public_profile/views/pages/profile_screen.dart
Comment on lines
+123
to
+142
| FilledButton( | ||
| onPressed: widget.onCompleteNow == null ? null : _handleComplete, | ||
| style: FilledButton.styleFrom( | ||
| backgroundColor: PrismColors.brandPink, | ||
| foregroundColor: PrismColors.onPrimary, | ||
| minimumSize: const Size(0, 36), | ||
| padding: const EdgeInsets.symmetric(horizontal: 14), | ||
| shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(8)), | ||
| ), | ||
| child: AnimatedSwitcher( | ||
| duration: const Duration(milliseconds: 180), | ||
| child: _isLoading | ||
| ? SizedBox( | ||
| key: const ValueKey('loading'), | ||
| width: 14, | ||
| height: 14, | ||
| child: CircularProgressIndicator(strokeWidth: 2, color: colorScheme.onPrimary), | ||
| ) | ||
| : const Text('Finish', key: ValueKey('label')), | ||
| ), |
There was a problem hiding this comment.
Disable the CTA while _isLoading is true.
Line 43 already guards against a second invocation, but the button still presents as enabled while the spinner is shown. Setting onPressed to null during loading keeps the interaction and accessibility states aligned.
🎛️ Suggested tweak
- onPressed: widget.onCompleteNow == null ? null : _handleComplete,
+ onPressed: widget.onCompleteNow == null || _isLoading ? null : _handleComplete,📝 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.
Suggested change
| FilledButton( | |
| onPressed: widget.onCompleteNow == null ? null : _handleComplete, | |
| style: FilledButton.styleFrom( | |
| backgroundColor: PrismColors.brandPink, | |
| foregroundColor: PrismColors.onPrimary, | |
| minimumSize: const Size(0, 36), | |
| padding: const EdgeInsets.symmetric(horizontal: 14), | |
| shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(8)), | |
| ), | |
| child: AnimatedSwitcher( | |
| duration: const Duration(milliseconds: 180), | |
| child: _isLoading | |
| ? SizedBox( | |
| key: const ValueKey('loading'), | |
| width: 14, | |
| height: 14, | |
| child: CircularProgressIndicator(strokeWidth: 2, color: colorScheme.onPrimary), | |
| ) | |
| : const Text('Finish', key: ValueKey('label')), | |
| ), | |
| FilledButton( | |
| onPressed: widget.onCompleteNow == null || _isLoading ? null : _handleComplete, | |
| style: FilledButton.styleFrom( | |
| backgroundColor: PrismColors.brandPink, | |
| foregroundColor: PrismColors.onPrimary, | |
| minimumSize: const Size(0, 36), | |
| padding: const EdgeInsets.symmetric(horizontal: 14), | |
| shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(8)), | |
| ), | |
| child: AnimatedSwitcher( | |
| duration: const Duration(milliseconds: 180), | |
| child: _isLoading | |
| ? SizedBox( | |
| key: const ValueKey('loading'), | |
| width: 14, | |
| height: 14, | |
| child: CircularProgressIndicator(strokeWidth: 2, color: colorScheme.onPrimary), | |
| ) | |
| : const Text('Finish', key: ValueKey('label')), | |
| ), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@lib/features/profile_completeness/views/widgets/profile_completeness_card.dart`
around lines 123 - 142, The CTA remains visually enabled while _isLoading is
true; update the FilledButton's onPressed logic to also check _isLoading and
return null when loading (i.e., make onPressed null if widget.onCompleteNow ==
null OR _isLoading) so the button is disabled during the spinner display; adjust
the onPressed expression where FilledButton is defined (referencing
FilledButton, _isLoading, widget.onCompleteNow, and _handleComplete) so the
interaction and accessibility states match the AnimatedSwitcher loading state.
Comment on lines
+589
to
+607
| if ((widget.bio ?? "").isNotEmpty) | ||
| SizedBox( | ||
| width: MediaQuery.of(context).size.width * 0.72, | ||
| child: Text( | ||
| widget.bio!, | ||
| textAlign: TextAlign.center, | ||
| // 2 lines: bios up to 150 chars deserve more space. | ||
| maxLines: 2, | ||
| overflow: TextOverflow.ellipsis, | ||
| style: TextStyle( | ||
| fontFamily: PrismFonts.proximaNova, | ||
| color: Theme.of(context).colorScheme.secondary.withValues(alpha: 0.65), | ||
| fontSize: 13, | ||
| fontWeight: FontWeight.normal, | ||
| height: 1.45, | ||
| ), | ||
| ), | ||
| ), | ||
| ), | ||
| if ((widget.bio ?? "").isNotEmpty) const SizedBox(height: 2), |
There was a problem hiding this comment.
Trim bio before checking emptiness.
On Line 589, whitespace-only bios still pass isNotEmpty, so an empty-looking bio block and spacing can render. Use trim() in both the condition and displayed value.
💡 Suggested fix
- if ((widget.bio ?? "").isNotEmpty)
+ if ((widget.bio ?? "").trim().isNotEmpty)
SizedBox(
width: MediaQuery.of(context).size.width * 0.72,
child: Text(
- widget.bio!,
+ widget.bio!.trim(),
textAlign: TextAlign.center,
// 2 lines: bios up to 150 chars deserve more space.
maxLines: 2,
overflow: TextOverflow.ellipsis,
@@
- if ((widget.bio ?? "").isNotEmpty) const SizedBox(height: 2),
+ if ((widget.bio ?? "").trim().isNotEmpty) const SizedBox(height: 2),📝 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.
Suggested change
| if ((widget.bio ?? "").isNotEmpty) | |
| SizedBox( | |
| width: MediaQuery.of(context).size.width * 0.72, | |
| child: Text( | |
| widget.bio!, | |
| textAlign: TextAlign.center, | |
| // 2 lines: bios up to 150 chars deserve more space. | |
| maxLines: 2, | |
| overflow: TextOverflow.ellipsis, | |
| style: TextStyle( | |
| fontFamily: PrismFonts.proximaNova, | |
| color: Theme.of(context).colorScheme.secondary.withValues(alpha: 0.65), | |
| fontSize: 13, | |
| fontWeight: FontWeight.normal, | |
| height: 1.45, | |
| ), | |
| ), | |
| ), | |
| ), | |
| if ((widget.bio ?? "").isNotEmpty) const SizedBox(height: 2), | |
| if ((widget.bio ?? "").trim().isNotEmpty) | |
| SizedBox( | |
| width: MediaQuery.of(context).size.width * 0.72, | |
| child: Text( | |
| widget.bio!.trim(), | |
| textAlign: TextAlign.center, | |
| // 2 lines: bios up to 150 chars deserve more space. | |
| maxLines: 2, | |
| overflow: TextOverflow.ellipsis, | |
| style: TextStyle( | |
| fontFamily: PrismFonts.proximaNova, | |
| color: Theme.of(context).colorScheme.secondary.withValues(alpha: 0.65), | |
| fontSize: 13, | |
| fontWeight: FontWeight.normal, | |
| height: 1.45, | |
| ), | |
| ), | |
| ), | |
| if ((widget.bio ?? "").trim().isNotEmpty) const SizedBox(height: 2), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/public_profile/views/pages/profile_screen.dart` around lines 589
- 607, The bio emptiness check uses (widget.bio ?? "").isNotEmpty which treats
whitespace-only strings as non-empty; update the conditional checks to use
(widget.bio ?? "").trim().isNotEmpty and pass widget.bio!.trim() into the Text
widget (and where the extra SizedBox is conditionally rendered) so
whitespace-only bios don't render an empty bio block or spacing; update both
occurrences referencing widget.bio and the Text display in this widget tree.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/core/widgets/menuButton/favWallpaperButton.dart (1)
74-91:⚠️ Potential issue | 🟠 MajorEnsure loading state is cleared on failure, not only success.
At line 84,
favCheckis chained with.then(...)withoutawaitandisLoadingis reset only in the success callback. IffavCheckfails, the spinner remains indefinitely since the error path has no handler.Suggested fix
Future<void> onFav(FavouriteWallEntity? wall) async { setState(() { isLoading = true; }); if (wall == null) { setState(() { isLoading = false; }); return; } - context.favouriteWallsAdapter(listen: false).favCheck(wall).then((value) { - analytics.track(FavStatusChangedEvent(wallId: wall.id, provider: wall.source.legacyProviderString)); - if (mounted) { - setState(() { - isLoading = false; - }); - } - }); + try { + await context.favouriteWallsAdapter(listen: false).favCheck(wall); + analytics.track(FavStatusChangedEvent(wallId: wall.id, provider: wall.source.legacyProviderString)); + } finally { + if (mounted) { + setState(() { + isLoading = false; + }); + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/core/widgets/menuButton/favWallpaperButton.dart` around lines 74 - 91, In onFav(FavouriteWallEntity? wall) ensure isLoading is cleared on both success and failure by awaiting the asynchronous call to context.favouriteWallsAdapter(listen: false).favCheck(wall) inside a try/catch/finally: call favCheck with await, track analytics in the try block after a successful response, handle/log errors in the catch block, and setState to set isLoading = false in the finally block so the spinner is always cleared regardless of errors.lib/core/widgets/popup/editProfilePanel.dart (1)
247-253:⚠️ Potential issue | 🟡 MinorMissing
mountedcheck beforesetStatein async callback.The
.then()callback callssetStatewithout verifying the widget is still mounted. If the user dismisses the panel while the GitHub upload is in-flight, this will throw a "setState called after dispose" exception.Suggested fix
.then( - (value) => setState(() { - pfpUrl = value.content!.downloadUrl!; - pfpPath = value.content!.path!; - pfpSha = value.content!.sha!; - }), + (value) { + if (!mounted) return; + setState(() { + pfpUrl = value.content!.downloadUrl!; + pfpPath = value.content!.path!; + pfpSha = value.content!.sha!; + }); + }, );The same pattern applies to
uploadFileCover()at lines 273-279.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/core/widgets/popup/editProfilePanel.dart` around lines 247 - 253, The async callbacks that call setState (updating pfpUrl, pfpPath, pfpSha) need a mounted check to avoid "setState called after dispose" when the panel is dismissed; in the .then() handlers for the GitHub upload and similarly in uploadFileCover(), wrap the setState call with an if (!mounted) return; or if (mounted) { setState(() { ... }); } so you only update state when the widget is still mounted — update the handlers that set pfpUrl/pfpPath/pfpSha and the callback inside uploadFileCover() accordingly.
🧹 Nitpick comments (2)
lib/features/palette/domain/bloc/wallpaper_detail_bloc.dart (1)
112-114: Remove redundant guards in index cycling logic.Line 113 already constrains
nextIndexvia% colors.length, so the check on Line 114 is unreachable. Also, after Line 109 (contains), Line 112 (currentIndex < 0) is redundant. Simplifying this block will make the wrap-around behavior explicit.Suggested simplification
- final currentIndex = colors.indexOf(accent); - if (currentIndex < 0) return; - final nextIndex = (currentIndex + 1) % colors.length; - if (nextIndex >= colors.length) return; + final currentIndex = colors.indexOf(accent); + final nextIndex = (currentIndex + 1) % colors.length; final nextColor = colors[nextIndex];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/palette/domain/bloc/wallpaper_detail_bloc.dart` around lines 112 - 114, The index-guarding logic is overly defensive: after checking that the list contains the value and using modulo to compute nextIndex, the guards `if (currentIndex < 0) return;` and `if (nextIndex >= colors.length) return;` are redundant; simplify by first guarding an empty list (e.g., `if (colors.isEmpty) return;`), keep the contains check, compute `final nextIndex = (currentIndex + 1) % colors.length;` and then use nextIndex directly (remove the two redundant ifs). Update the code in the WallpaperDetailBloc where `currentIndex`, `nextIndex`, `colors`, and the `contains` check are used to make wrap-around explicit and concise.lib/core/firestore/firestore_tracked_client.dart (1)
550-579: Consider adding jitter to exponential backoff to avoid thundering herd.The retry logic is well-implemented with proper exponential backoff. However, without jitter, multiple clients experiencing the same transient error may retry in sync, potentially causing repeated contention.
♻️ Optional: Add jitter to backoff delay
if (_isTransientFirestoreError(mapped) && attempt < maxRetries) { attempt++; - final int delayMs = (500 * math.pow(2, attempt - 1)).round(); + final int baseDelayMs = (500 * math.pow(2, attempt - 1)).round(); + final int jitterMs = math.Random().nextInt(baseDelayMs ~/ 2); + final int delayMs = baseDelayMs + jitterMs; logger.w( '[Firestore] transient error (${mapped.code}) on transaction — retrying ($attempt/$maxRetries) after ${delayMs}ms, sourceTag: $sourceTag', );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/core/firestore/firestore_tracked_client.dart` around lines 550 - 579, The retry backoff in the transaction loop currently computes delayMs deterministically and should include jitter to avoid thundering herd; modify the retry block (around attempt, maxRetries, delayMs, the logger.w call and the await Future<void>.delayed) to compute a jitteredDelayMs using a Random (from dart:math) and apply a small randomized factor (e.g., multiply delayMs by a value between 0.5–1.5 or add/subtract up to X% of delayMs), log the jitteredDelayMs in the logger.w message, and use jitteredDelayMs for the Future.delayed instead of the original delayMs so retries are randomized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/auth/google_auth.dart`:
- Around line 199-205: The cancellation-detection logic currently treats
GoogleSignInExceptionCode.unknownError and message substrings 'no credential' /
'no credentials available' as user cancellations; update the logic in the block
that checks error.code and the message (the code that sets final String message
= error.toString().toLowerCase()) to only treat
GoogleSignInExceptionCode.canceled and the text 'cancelled' (and optionally
'user canceled') as cancellations—remove GoogleSignInExceptionCode.unknownError
from the error.code check and remove the message.contains checks for 'no
credential' and 'no credentials available' so real credential/misc errors are
not suppressed as cancellations.
In `@lib/core/widgets/popup/editProfilePanel.dart`:
- Around line 857-864: The current onChanged handler sets linkEdit based only on
whether any linkIcons have non-empty values, which prevents saving when a user
clears all links; instead capture the initial/persisted link state (e.g., a Map
or List like initialLinkValues keyed by the same identifier used by linkIcons)
when the panel is initialized and then in the onChanged for the link input
compare the current linkIcons values against initialLinkValues to compute
changed (true if any value differs, including emptied values) and call
setState(() => linkEdit = changed); update the snapshot when a save successfully
persists changes so subsequent edits compare against the new baseline.
---
Outside diff comments:
In `@lib/core/widgets/menuButton/favWallpaperButton.dart`:
- Around line 74-91: In onFav(FavouriteWallEntity? wall) ensure isLoading is
cleared on both success and failure by awaiting the asynchronous call to
context.favouriteWallsAdapter(listen: false).favCheck(wall) inside a
try/catch/finally: call favCheck with await, track analytics in the try block
after a successful response, handle/log errors in the catch block, and setState
to set isLoading = false in the finally block so the spinner is always cleared
regardless of errors.
In `@lib/core/widgets/popup/editProfilePanel.dart`:
- Around line 247-253: The async callbacks that call setState (updating pfpUrl,
pfpPath, pfpSha) need a mounted check to avoid "setState called after dispose"
when the panel is dismissed; in the .then() handlers for the GitHub upload and
similarly in uploadFileCover(), wrap the setState call with an if (!mounted)
return; or if (mounted) { setState(() { ... }); } so you only update state when
the widget is still mounted — update the handlers that set pfpUrl/pfpPath/pfpSha
and the callback inside uploadFileCover() accordingly.
---
Nitpick comments:
In `@lib/core/firestore/firestore_tracked_client.dart`:
- Around line 550-579: The retry backoff in the transaction loop currently
computes delayMs deterministically and should include jitter to avoid thundering
herd; modify the retry block (around attempt, maxRetries, delayMs, the logger.w
call and the await Future<void>.delayed) to compute a jitteredDelayMs using a
Random (from dart:math) and apply a small randomized factor (e.g., multiply
delayMs by a value between 0.5–1.5 or add/subtract up to X% of delayMs), log the
jitteredDelayMs in the logger.w message, and use jitteredDelayMs for the
Future.delayed instead of the original delayMs so retries are randomized.
In `@lib/features/palette/domain/bloc/wallpaper_detail_bloc.dart`:
- Around line 112-114: The index-guarding logic is overly defensive: after
checking that the list contains the value and using modulo to compute nextIndex,
the guards `if (currentIndex < 0) return;` and `if (nextIndex >= colors.length)
return;` are redundant; simplify by first guarding an empty list (e.g., `if
(colors.isEmpty) return;`), keep the contains check, compute `final nextIndex =
(currentIndex + 1) % colors.length;` and then use nextIndex directly (remove the
two redundant ifs). Update the code in the WallpaperDetailBloc where
`currentIndex`, `nextIndex`, `colors`, and the `contains` check are used to make
wrap-around explicit and concise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 61bed434-cdbf-4af2-8a8d-41b8bc3732e8
📒 Files selected for processing (9)
lib/auth/google_auth.dartlib/core/firestore/firestore_tracked_client.dartlib/core/widgets/menuButton/favWallpaperButton.dartlib/core/widgets/popup/editProfilePanel.dartlib/data/pexels/provider/pexelsWithoutProvider.dartlib/features/category_feed/views/widgets/collections_grid.dartlib/features/navigation/views/widgets/prism_top_app_bar.dartlib/features/onboarding_v2/src/biz/onboarding_v2_bloc.j.dartlib/features/palette/domain/bloc/wallpaper_detail_bloc.dart
✅ Files skipped from review due to trivial changes (1)
- lib/features/navigation/views/widgets/prism_top_app_bar.dart
Comment on lines
+199
to
+205
| return error.code == GoogleSignInExceptionCode.canceled || error.code == GoogleSignInExceptionCode.unknownError; | ||
| } | ||
| final String message = error.toString().toLowerCase(); | ||
| return message.contains('user canceled') || message.contains('cancelled'); | ||
| return message.contains('user canceled') || | ||
| message.contains('cancelled') || | ||
| message.contains('no credential') || | ||
| message.contains('no credentials available'); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For google_sign_in v7.2.0, what are the documented meanings of GoogleSignInExceptionCode.canceled vs GoogleSignInExceptionCode.unknownError, and is unknownError guaranteed to be user-initiated cancellation? Also, are "no credential" / "no credentials available" documented cancellation signals?
💡 Result:
GoogleSignInExceptionCode.canceled: “The operation was canceled by the user.” [1]GoogleSignInExceptionCode.unknownError: “A catch-all … for implementations that need to return a code that does not have a corresponding known code”, and the exception description should contain details. [1]
Is unknownError guaranteed to be user-initiated cancellation?
No. It’s explicitly documented as a catch-all for uncategorized errors, not as a cancellation signal. [1]
Are “no credential” / “No credential(s) available” documented cancellation signals?
No. In the Android Credential Manager API, NoCredentialException means no viable credential is available, while GetCredentialCancellationException is the documented “user intentionally cancels” case. [2]
In google_sign_in_android, the “No credential available: …” message is produced for the noCredential failure type (mapped to unknownError in the button flow), not for a cancellation. [3]
Also note: even canceled itself is not guaranteed to be truly user-initiated on Android—google_sign_in_android documents that some configuration errors can surface as GoogleSignInExceptionCode.canceled, and the plugin can’t distinguish those from real user cancellation. [4]
Sources
[1] GoogleSignInExceptionCode enum docs (google_sign_in v7.2.0) (pub.dev)
[2] AndroidX Credential Manager GetCredentialException / subclasses docs (developer.android.com)
[3] google_sign_in_android implementation mapping noCredential → “No credential available …” (chromium.googlesource.com)
[4] google_sign_in_android troubleshooting docs about “canceled” sometimes being configuration errors (pub.dev)
Citations:
- 1: https://pub.dev/documentation/google_sign_in/latest/google_sign_in/GoogleSignInExceptionCode.html
- 2: https://developer.android.com/reference/androidx/credentials/exceptions/GetCredentialException
- 3: https://chromium.googlesource.com/external/github.com/flutter/packages/%2B/refs/tags/google_sign_in_android-v7.2.7/packages/google_sign_in/google_sign_in_android/lib/google_sign_in_android.dart
- 4: https://pub.dev/documentation/google_sign_in_android/latest/
Do not classify unknownError or "no credential" messages as user cancellation.
Lines 199 and 202-205 misclassify non-cancellation errors as cancellations. Per the google_sign_in v7.2.0 documentation:
GoogleSignInExceptionCode.unknownErroris a catch-all for uncategorized errors, not a cancellation signal.- "no credential" / "no credentials available" indicate missing credentials (not user cancellation); the Android Credential Manager distinguishes these from user-initiated cancellation via separate exception types.
Because this codebase suppresses errors for canceled sign-ins (session_repository_impl.dart:127-139, onboarding_v2_shell.dart:109-122, settings_screen.dart:394-407, signInPopUp.dart:207-220), misclassifying real failures as cancellation silently swallows error feedback to users.
Remove unknownError from line 199 and remove the 'no credential' and 'no credentials available' checks from lines 202-205. Retain only canceled and 'cancelled'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/auth/google_auth.dart` around lines 199 - 205, The cancellation-detection
logic currently treats GoogleSignInExceptionCode.unknownError and message
substrings 'no credential' / 'no credentials available' as user cancellations;
update the logic in the block that checks error.code and the message (the code
that sets final String message = error.toString().toLowerCase()) to only treat
GoogleSignInExceptionCode.canceled and the text 'cancelled' (and optionally
'user canceled') as cancellations—remove GoogleSignInExceptionCode.unknownError
from the error.code check and remove the message.contains checks for 'no
credential' and 'no credentials available' so real credential/misc errors are
not suppressed as cancellations.
Comment on lines
+857
to
+864
| onChanged: (value) { | ||
| if (value.toLowerCase().contains('${_link?.validator.toLowerCase()}')) { | ||
| if (_link != null) _link!.value = value; | ||
| } else if (value.isEmpty) { | ||
| if (_link != null) _link!.value = ''; | ||
| } | ||
| final changed = linkIcons.any((icon) => icon.value.isNotEmpty); | ||
| setState(() => linkEdit = changed); |
There was a problem hiding this comment.
linkEdit detection prevents saving cleared links.
Setting linkEdit based solely on whether any linkIcons value is non-empty:
final changed = linkIcons.any((icon) => icon.value.isNotEmpty);
setState(() => linkEdit = changed);If a user clears all their existing links (making every icon.value empty), linkEdit becomes false, the save button is disabled, and the cleared state cannot be persisted.
Consider comparing against the initial/persisted link values instead:
Suggested fix
onChanged: (value) {
if (value.toLowerCase().contains('${_link?.validator.toLowerCase()}')) {
if (_link != null) _link!.value = value;
} else if (value.isEmpty) {
if (_link != null) _link!.value = '';
}
- final changed = linkIcons.any((icon) => icon.value.isNotEmpty);
- setState(() => linkEdit = changed);
+ final originalLinks = app_state.prismUser.links;
+ final changed = linkIcons.any((icon) {
+ final original = originalLinks[icon.name] ?? '';
+ return icon.value != original;
+ });
+ setState(() => linkEdit = changed);
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/core/widgets/popup/editProfilePanel.dart` around lines 857 - 864, The
current onChanged handler sets linkEdit based only on whether any linkIcons have
non-empty values, which prevents saving when a user clears all links; instead
capture the initial/persisted link state (e.g., a Map or List like
initialLinkValues keyed by the same identifier used by linkIcons) when the panel
is initialized and then in the onChanged for the link input compare the current
linkIcons values against initialLinkValues to compute changed (true if any value
differs, including emptied values) and call setState(() => linkEdit = changed);
update the snapshot when a save successfully persists changes so subsequent
edits compare against the new baseline.
LiquidatorCoder
approved these changes
Apr 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
UI Improvements
Bug Fixes
Tests