508-improve-chat-list-search-and-refresh-triggers #517
Conversation
…reen for improved search functionality
codeswot
requested review from
Quwaysim,
erskingardner,
josefinalliende and
untreu2
WalkthroughRaised iOS minimum deployment target from 12.0 to 13.0 across plist, Podfile, and Xcode project. Refactored ChatListScreen internals to add threshold-based gesture handling for search and pull-to-refresh, new loading/refresh states and animations, concurrent provider data loading, search query and focus management, and scroll physics changes. Public APIs unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant SC as ScrollController
participant CLS as ChatListScreen
participant UI as Animations/UI
participant Data as Providers/Services
U->>SC: Drag / Tap / Type
SC-->>CLS: Scroll / gesture events
CLS->>CLS: calculate thresholds & decide action
alt Trigger refresh
CLS->>UI: show loading animation
CLS->>Data: _loadAllProviderData() (concurrent)
Data-->>CLS: results or errors
CLS->>UI: stop animation
CLS-->>U: refreshed list
else Trigger search
CLS->>UI: reveal search UI (animate)
U->>CLS: input query
CLS->>CLS: update _searchQuery & filter items
CLS-->>U: filtered list
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
lib/ui/contact_list/chat_list_screen.dart (1)
233-241: Remove manual disposal of the Riverpod notifier in ChatListScreen.disposePollingNotifier extends
Notifier<bool>and overrides its owndispose()for internal cleanup, but Riverpod will call that automatically. Manually calling_pollingNotifier.dispose()risks stepping on Provider’s lifecycle. Instead, stop polling or invalidate the provider:• File: lib/ui/contact_list/chat_list_screen.dart
Replace indispose()(around line 234):@override void dispose() { - _pollingNotifier.dispose(); + _pollingNotifier.stopPolling(); // or use ref.invalidate(pollingProvider); _searchController.dispose(); _scrollController.dispose(); _loadingAnimationController.dispose(); _searchAnimationController.dispose(); WelcomeNotificationService.clearContext(); super.dispose(); }Tagging as a required refactor to align with Riverpod best practices.
🧹 Nitpick comments (4)
lib/ui/contact_list/chat_list_screen.dart (4)
185-191: Optional: autofocus the search field when it becomes visibleThis improves UX by letting users type immediately after the short swipe.
Apply this diff:
void _triggerSearch() { _hasSearchTriggered = true; setState(() { _isSearchVisible = true; }); + FocusScope.of(context).requestFocus(_searchFocusNode); }
75-89: Remove unused _searchAnimationController (dead code) or put it to useThe controller is created and disposed but not used anywhere. Either wire it into the search UI animation or remove to reduce complexity.
Apply these diffs to remove it:
- late AnimationController _searchAnimationController;- _searchAnimationController = AnimationController( - duration: _searchAnimationDuration, - vsync: this, - );And in dispose():
- _searchAnimationController.dispose();Also applies to: 58-59
159-167: Consider using Theme.of(context).platform for platform checksdefaultTargetPlatform is fine, but using Theme.of(context).platform can better reflect runtime platform theming (especially if overridden in tests or for web). Not a blocker.
90-93: Minor styling nits per guidelines
- There are a few blank lines within functions (e.g., _initializeControllers). The internal style guide asks to avoid blank lines within functions. Not critical; feel free to ignore if it hurts readability.
Also applies to: 244-510
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
ios/Flutter/AppFrameworkInfo.plist(1 hunks)ios/Podfile(1 hunks)ios/Runner.xcodeproj/project.pbxproj(3 hunks)lib/ui/contact_list/chat_list_screen.dart(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit Inference Engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/ui/contact_list/chat_list_screen.dart
🧠 Learnings (1)
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use Riverpod to manage state. Use StreamProviders to watch for state changes that come from the rust api. See keepAlive if you need to keep the state alive.
Applied to files:
lib/ui/contact_list/chat_list_screen.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Flutter CI
🔇 Additional comments (3)
ios/Runner.xcodeproj/project.pbxproj (1)
459-459: LGTM: Deployment target bumped to 13.0 in all relevant build configsThe IPHONEOS_DEPLOYMENT_TARGET updates look consistent across Profile/Debug/Release at the project level. This aligns with the PR objective.
Also applies to: 599-599, 650-650
ios/Flutter/AppFrameworkInfo.plist (1)
24-24: LGTM: MinimumOSVersion set to 13.0Matches the project settings and Podfile intent.
lib/ui/contact_list/chat_list_screen.dart (1)
41-49: Good move: extracted gesture thresholds and timings into named constantsClear, self-documenting thresholds and durations replace magic numbers. This aligns with the guidelines and improves maintainability.
There was a problem hiding this comment.
I tried it locally and worked perfect in my ios emulator. It feels way more intuitive for me now. previously I thought that is was a bit "random" if the search or loading appeared 😆.
I still have pending to test this tomorrow on physical devices 😉.
Last but not least, my only concern is about upgrading the minimum ios version. I'd rather keep it as lower as possible. As a former old ios version user, I prefered that apps worked partially in my phone (if it wasn't nothing critical, which I guess it is the case, until we don't support payments) ... but even in that case of critical functionalities, I'd prefer to just block some features to old version users instead of preventing them from downloading the app.
…enhance error logging in ChatListScreen
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/ui/contact_list/chat_list_screen.dart (1)
247-256: UsestopPolling()instead of manually disposing the provider’s NotifierManually calling
_pollingNotifier.dispose()on a provider‐ownedPollingNotifiercan conflict with Riverpod’s lifecycle and leave the notifier in an invalid state. SincePollingNotifieralready exposes a safestopPolling()method to halt its timer, update yourdisposeoverride to call that and allow Riverpod to handle cleanup of the notifier itself.• In lib/ui/contact_list/chat_list_screen.dart @ dispose():
@override void dispose() { - _pollingNotifier.dispose(); + _pollingNotifier.stopPolling(); _searchController.dispose(); _scrollController.dispose(); _loadingAnimationController.dispose(); _searchAnimationController.dispose(); WelcomeNotificationService.clearContext(); super.dispose(); }
♻️ Duplicate comments (1)
lib/ui/contact_list/chat_list_screen.dart (1)
215-226: Refresh lifecycle is correctly guarded and resilient.The try/finally ensures refresh state is reset even on errors, and the loading animation is well-contained.
Also applies to: 228-238, 240-246
🧹 Nitpick comments (3)
lib/ui/contact_list/chat_list_screen.dart (3)
108-116: Remove duplicate state reset in _loadData to avoid double setState.state is set to not-loading both inside try and in finally. Keep the finally-only reset to prevent extra rebuilds.
try { _setLoadingState(isLoading: true); await _loadAllProviderData(); - _setLoadingState(isLoading: false); } catch (e, st) { _log.severe('Error loading data: $e $st'); } finally { _setLoadingState(isLoading: false); }
201-207: Also unfocus the search field when clearing to ensure the keyboard dismisses.Currently the UI hides, but the keyboard may remain up if focus stays.
void _clearSearch() { setState(() { _searchController.clear(); _searchQuery = ''; _isSearchVisible = false; }); + _unfocusSearchIfNeeded(); }
49-50: _searchAnimationController appears unused; consider removing to reduce complexity.You initialize and dispose it, but it’s never driven/consumed. If there’s no planned use, drop the controller and its duration constant.
- static const Duration _searchAnimationDuration = Duration(milliseconds: 300);- _searchAnimationController = AnimationController( - duration: _searchAnimationDuration, - vsync: this, - );Additionally, remove its disposal:
// In dispose(): // _searchAnimationController.dispose();Also applies to: 86-90
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
ios/Podfile(1 hunks)lib/ui/contact_list/chat_list_screen.dart(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- ios/Podfile
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit Inference Engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/ui/contact_list/chat_list_screen.dart
🧠 Learnings (1)
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use Riverpod to manage state. Use StreamProviders to watch for state changes that come from the rust api. See keepAlive if you need to keep the state alive.
Applied to files:
lib/ui/contact_list/chat_list_screen.dart
🔇 Additional comments (6)
lib/ui/contact_list/chat_list_screen.dart (6)
72-75: Nice decomposition of init/setup flow.initState delegating to small, focused methods improves readability and testability. Post-frame scheduling for one-time setup is appropriate here.
Also applies to: 77-90, 92-95, 96-96
137-145: Gesture detection refactor looks solid and matches the PR intent.
- Pull distance computed from negative overscroll is correct.
- Reset conditions and gating via loading/refreshing prevent re-entrancy.
- Refresh wins over search when both thresholds are crossed. Good ordering.
- Platform-aware thresholds via ScreenUtil height are reasonable.
Also applies to: 147-155, 156-165, 167-175, 177-187, 188-200
314-314: Good focus management on UI interactions.Unfocusing on tap actions and tying the clear icon to clear logic improves UX consistency.
Also applies to: 330-331, 346-347, 450-451, 476-477
320-322: Consistent, always-scrollable physics is a good choice here.Ensures pull-to-search/refresh works even with short lists and keeps iOS-typical bounce.
40-41: Logging integration is appropriate.Using
package:loggingwith context-rich messages in catch blocks is helpful for diagnosing gesture/load issues.Also applies to: 108-116
167-175: Dart SDK constraint supports record typespubspec.yaml specifies
sdk: ^3.7.2, which is ≥ 3.0 and so record return types are fully supported. No changes needed.
… cleaner implementation
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
lib/ui/contact_list/chat_list_screen.dart (2)
100-108: Resolved: loading state now resets via try/finallyThe previous risk of leaving
_isLoadingDatatrue on errors is fixed with try/finally; severe logging adds observability.
207-217: Resolved: refresh state now resets via try/finally with error loggingThis avoids a stuck “refreshing” state on exceptions. Good follow-through on the prior recommendation.
🧹 Nitpick comments (2)
lib/ui/contact_list/chat_list_screen.dart (2)
89-96: Guard post-frame callback against disposed stateIf the widget is disposed before the callback executes, using
contextor calling async loaders could throw. Add a mounted check at the start of the callback.Apply this diff:
void _scheduleInitialSetup() { WidgetsBinding.instance.addPostFrameCallback((_) { + if (!mounted) return; WelcomeNotificationService.initialize(context); WelcomeNotificationService.setupWelcomeNotifications(ref); _loadData(); _pollingNotifier.startPolling(); }); }
179-183: Refresh trigger is fine; consider documenting unawaited intentCalling
_performRefresh()without awaiting is fine (non-blocking UI). If you enforce “unawaited futures” lints, consider marking intent (e.g., comment or usingunawaitedif available).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
lib/ui/contact_list/chat_list_screen.dart(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit Inference Engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/ui/contact_list/chat_list_screen.dart
🧠 Learnings (2)
📓 Common learnings
Learnt from: codeswot
PR: parres-hq/whitenoise_flutter#517
File: ios/Podfile:0-0
Timestamp: 2025-08-20T05:36:39.566Z
Learning: Flutter 3.35.1 and later versions require iOS 13.0 as the minimum deployment target. Projects using modern Flutter versions cannot support iOS 12 or earlier due to framework requirements, regardless of Apple's App Store policies.
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use Riverpod to manage state. Use StreamProviders to watch for state changes that come from the rust api. See keepAlive if you need to keep the state alive.
Applied to files:
lib/ui/contact_list/chat_list_screen.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Flutter CI
🔇 Additional comments (18)
lib/ui/contact_list/chat_list_screen.dart (18)
1-1: New imports for platform checks and logging look good
foundation.dartfordefaultTargetPlatformandlogging.dartforLoggerare appropriate for the new behavior.Also applies to: 9-9
40-50: Good: centralized logger and named threshold/animation constants
- Logger scoped to the screen.
- Platform-specific thresholds match PR intent (iOS: 10%/25%, Android: 8%/20%).
- Magic numbers are eliminated via constants.
62-65: Clear UI state flags introducedFlags for loading, refreshing, and search visibility are well named and follow boolean verb/noun guidance.
70-73: Init sequence refactor reads cleanlySplitting controller/scroll setup and initial scheduling improves readability and keeps
initStatesmall.
75-83: Animation controller initialization uses constants and proper vsyncDuration via constant and
Curves.easeInOutare appropriate for the loading affordance.
85-87: Scroll listener hookup is straightforwardListener delegates to
_onScroll, matching the refactor toward small focused methods.
119-126: Parallel provider loads are appropriate
Future.waitis a good fit here and matches the refactor goal. Centralizing loads improves maintainability.
131-136: Scroll processing guard + early reset keep logic predictable
- Triggers reset when offset bounces back above -20.
- Gesture processing is skipped while loading or refreshing.
138-144: Trigger reset offset is reasonableUsing a small negative offset as a reset threshold avoids retriggering while still pulled. Good choice.
145-145: Good: disable gesture processing during load/refreshPrevents overlapping actions and racey states.
147-156: Distance-based gesture processing implements the PR behavior cleanlyComputes pull distance and selects refresh over search when both thresholds are crossed. Clear precedence and flags prevent double fire.
158-166: Threshold calculation by platform and viewport heightUsing
1.shkeeps it responsive to device height; platform branching matches the described behavior.
168-178: Trigger predicates are correct and mutually exclusive
- Refresh: distance >= refresh threshold and not already triggered.
- Search: distance >= search threshold, not triggered or visible, and refresh not triggered.
184-191: Search trigger shows field and focuses correctlyState toggling and focus request align with the intended UX.
192-199: Clear search UX is handled and focus is dismissedState updates and focus management look correct.
201-206: Idempotent unfocus helper is cleanAvoids unnecessary calls if focus is already absent.
310-313: Cross-platform scroll physics chosen correctly for pull gestures
AlwaysScrollableScrollPhysicswithBouncingScrollPhysicsensures negative offsets required for the distance-based triggers on both iOS and Android. Matches the PR goal.
321-321: Nice: global and per-item taps dismiss search focusThis prevents stray focus/keyboard state while navigating or opening the new chat sheet, and aligns with the refactor to centralize focus handling.
Also applies to: 337-337, 440-440, 466-467
| void _setLoadingState({required bool isLoading}) { | ||
| setState(() { | ||
| _isSearchVisible = false; | ||
| _isLoadingData = true; | ||
| _isLoadingData = isLoading; | ||
| if (isLoading) { | ||
| _isSearchVisible = false; | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Prevent setState after dispose in _setLoadingState
Because _loadData awaits multiple futures, _setLoadingState(isLoading: false) in finally can run after disposal, causing “setState() called after dispose()”.
Add a mounted guard.
void _setLoadingState({required bool isLoading}) {
- setState(() {
+ if (!mounted) return;
+ setState(() {
_isLoadingData = isLoading;
if (isLoading) {
_isSearchVisible = false;
}
});
}🤖 Prompt for AI Agents
In lib/ui/contact_list/chat_list_screen.dart around lines 110 to 117,
_setLoadingState calls setState unconditionally which can run after the State
has been disposed (causing "setState() called after dispose()"); add a mounted
guard at the start of the method (e.g. return early if not mounted) before
invoking setState so the method exits silently when the widget is disposed,
preserving the existing state updates when mounted.
| void _setRefreshState({required bool isRefreshing}) { | ||
| setState(() { | ||
| _isRefreshing = false; | ||
| _hasRefreshTriggered = false; | ||
| _isRefreshing = isRefreshing; | ||
| if (isRefreshing) { | ||
| _isSearchVisible = false; | ||
| _hasSearchTriggered = false; | ||
| } else { | ||
| _hasRefreshTriggered = false; | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Prevent setState after dispose in _setRefreshState
This method is called in finally after awaits; without a guard, it can call setState after dispose.
void _setRefreshState({required bool isRefreshing}) {
- setState(() {
+ if (!mounted) return;
+ setState(() {
_isRefreshing = isRefreshing;
if (isRefreshing) {
_isSearchVisible = false;
_hasSearchTriggered = false;
} else {
_hasRefreshTriggered = false;
}
});
}📝 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 _setRefreshState({required bool isRefreshing}) { | |
| setState(() { | |
| _isRefreshing = false; | |
| _hasRefreshTriggered = false; | |
| _isRefreshing = isRefreshing; | |
| if (isRefreshing) { | |
| _isSearchVisible = false; | |
| _hasSearchTriggered = false; | |
| } else { | |
| _hasRefreshTriggered = false; | |
| } | |
| }); | |
| } | |
| void _setRefreshState({required bool isRefreshing}) { | |
| if (!mounted) return; | |
| setState(() { | |
| _isRefreshing = isRefreshing; | |
| if (isRefreshing) { | |
| _isSearchVisible = false; | |
| _hasSearchTriggered = false; | |
| } else { | |
| _hasRefreshTriggered = false; | |
| } | |
| }); | |
| } |
🤖 Prompt for AI Agents
In lib/ui/contact_list/chat_list_screen.dart around lines 219 to 229,
_setRefreshState can call setState after the State has been disposed because
it's invoked from a finally after awaits; guard against calling setState on an
unmounted widget by returning early if mounted is false (e.g., add "if
(!mounted) return;" at the start of the method) or check mounted before invoking
setState so the state update only runs when the widget is still mounted.
| Future<void> _executeRefreshSequence() async { | ||
| _loadingAnimationController.forward(); | ||
| await _loadData(); | ||
| await Future.delayed(_animationDelay); | ||
| await _loadingAnimationController.reverse(); | ||
| } |
There was a problem hiding this comment.
Guard against disposed state before continuing animations
If the widget gets disposed while awaiting _loadData() or the delay, calling reverse() on a disposed controller will throw. Check mounted after awaits.
Future<void> _executeRefreshSequence() async {
_loadingAnimationController.forward();
await _loadData();
+ if (!mounted) return;
await Future.delayed(_animationDelay);
- await _loadingAnimationController.reverse();
+ if (!mounted) return;
+ await _loadingAnimationController.reverse();
}📝 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> _executeRefreshSequence() async { | |
| _loadingAnimationController.forward(); | |
| await _loadData(); | |
| await Future.delayed(_animationDelay); | |
| await _loadingAnimationController.reverse(); | |
| } | |
| Future<void> _executeRefreshSequence() async { | |
| _loadingAnimationController.forward(); | |
| await _loadData(); | |
| if (!mounted) return; | |
| await Future.delayed(_animationDelay); | |
| if (!mounted) return; | |
| await _loadingAnimationController.reverse(); | |
| } |
🤖 Prompt for AI Agents
In lib/ui/contact_list/chat_list_screen.dart around lines 231 to 236, the async
refresh sequence calls _loadingAnimationController.reverse() after awaiting
_loadData() and a delay, which can throw if the widget was disposed during those
awaits; after each await check mounted and return early (or skip the reverse) if
not mounted, ensuring you only call reverse on a live controller, and optionally
stop/cleanup the controller if the widget is no longer mounted.
| @override | ||
| void dispose() { | ||
| _pollingNotifier.dispose(); | ||
| _pollingNotifier.stopPolling(); |
There was a problem hiding this comment.
Dispose the FocusNode to avoid leaks
_searchFocusNode is created but not disposed, which can leak resources.
@override
void dispose() {
_pollingNotifier.stopPolling();
_searchController.dispose();
+ _searchFocusNode.dispose();
_scrollController.dispose();
_loadingAnimationController.dispose();
WelcomeNotificationService.clearContext();
super.dispose();
}🤖 Prompt for AI Agents
In lib/ui/contact_list/chat_list_screen.dart around line 240, the
_searchFocusNode created for the search field is not being disposed which can
leak resources; update the widget's dispose method to call
_searchFocusNode.dispose() (and keep the existing
_pollingNotifier.stopPolling()); ensure disposal happens before calling
super.dispose() and guard against double-dispose if necessary.
josefinalliende
deleted the
508-improve-chat-list-search-and-refresh-triggers
branch
…reen for improved search functionality
Description
Fix Chat List Search and Refresh Swipe Detection
Problem: Chat list search and refresh actions were
triggered based on swipe speed, which confused users
since it wasn't clear how to activate one action versus
the other. This approach was particularly unreliable on
Android devices.
Solution: Changed from velocity-based to distance-based
swipe detection:
height)
height)
Android (8%/20%) for better compatibility
Secondary improvements:
methods
behavior
Result: Users now have a predictable, intuitive way to
trigger search (short swipe) vs refresh (long swipe)
actions, with improved reliability across iOS and Android
platforms.
Type of Change
Checklist
just precommitto ensure that formatting and linting are correctjust check-flutter-coverageto ensure that flutter coverage rules are passingCHANGELOG.mdfile with your changes (if they affect the user experience)Summary by CodeRabbit
New Features
UX Improvements
Refactor
Chores