perf: remove 2s startup delay, optimistic UI updates, background cache cleanup#4423
perf: remove 2s startup delay, optimistic UI updates, background cache cleanup#4423JacobiusMakes wants to merge 1 commit intolinagora:masterfrom
Conversation
…he cleanup Three performance improvements: 1. Remove hardcoded 2-second delay on app startup (home_controller.dart:108) - The Future.delayed(2.seconds) blocked every app launch unnecessarily 2. Move cache cleanup to background (home_controller.dart:117-125) - Cache cleanup (email, login URL, username) no longer blocks auth flow - getAuthenticatedAccountAction() starts immediately - Cleanup runs concurrently in the background 3. Add optimistic UI updates for star/unstar and read/unread - Star toggle: UI updates immediately on tap, server confirms async - Read/unread toggle: same pattern — instant visual feedback - Fixes linagora#4190 (performance issue to edit thread favorites) Related issues: linagora#3694, linagora#2955, linagora#4190 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WalkthroughThis pull request optimizes performance by addressing delays in navigation and UI responsiveness. In Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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/features/home/presentation/home_controller.dart`:
- Around line 119-128: Make the fire-and-forget cache cleanup explicit and
surface errors: import unawaited from dart:async and wrap the Future.wait([...])
call with unawaited(...). Attach error handling (e.g., .catchError or try/catch
inside an async closure) to the Future.wait of
_cleanupEmailCacheInteractor.execute,
_cleanupRecentLoginUrlCacheInteractor.execute and
_cleanupRecentLoginUsernameCacheInteractor.execute so any exceptions are logged
(use your controller logger or print) and remove eagerError: true since you’ll
handle errors yourself; keep getAuthenticatedAccountAction() as-is.
In
`@lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart`:
- Around line 1227-1231: The optimistic update calling
updateEmailFlagByEmailIds([emailId], readActions: readActions) needs a rollback
on failure: before performing the optimistic update capture the prior flag state
for the affected email(s) (e.g., current read/star values), then in
handleFailureViewState() detect MarkAsEmailReadFailure and
MarkAsStarEmailFailure and call updateEmailFlagByEmailIds(...) with the captured
previous values to revert the UI; apply the same capture-and-rollback approach
for the similar optimistic block around the second occurrence (lines ~1246-1250)
so failures restore the original flags.
- Around line 1227-1231: Change the optimistic update call to use the correct
named parameter readAction instead of readActions in
updateEmailFlagByEmailIds([emailId], readAction: readAction) and ensure you
capture the prior read/star state before applying the optimistic change; then,
in handleFailureViewState add handlers for MarkAsEmailReadFailure and
MarkAsStarEmailFailure (the failures thrown by
_markAsEmailReadInteractor.execute and the star interactor) that revert the
optimistic UI by calling updateEmailFlagByEmailIds with the saved previous state
(e.g., readAction / star flag) for the affected emailIds so the UI is restored
on network/error failures.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 20a7929f-9df1-43b4-b60c-d3c3df783e3f
📒 Files selected for processing (2)
lib/features/home/presentation/home_controller.dartlib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart
| // Start auth immediately — don't block on cache cleanup | ||
| getAuthenticatedAccountAction(); | ||
|
|
||
| // Run cache cleanup in background (non-blocking) | ||
| Future.wait([ | ||
| _cleanupEmailCacheInteractor.execute(EmailCleanupRule(Duration.defaultCacheInternal)), | ||
| _cleanupRecentLoginUrlCacheInteractor.execute(RecentLoginUrlCleanupRule()), | ||
| _cleanupRecentLoginUsernameCacheInteractor.execute(RecentLoginUsernameCleanupRule()), | ||
| ], eagerError: true).then((_) => getAuthenticatedAccountAction()); | ||
| ], eagerError: true); | ||
| } |
There was a problem hiding this comment.
Fire-and-forget Future.wait will silently swallow errors.
The Future.wait is not awaited and has no error handler, so any exceptions thrown by the cleanup interactors will be lost. The eagerError: true parameter has no effect since nothing listens for the error.
While cache cleanup failures are likely non-critical, silently swallowing errors makes debugging harder if cleanup ever fails unexpectedly.
Proposed fix: add error handling to log failures
// Run cache cleanup in background (non-blocking)
- Future.wait([
+ unawaited(Future.wait([
_cleanupEmailCacheInteractor.execute(EmailCleanupRule(Duration.defaultCacheInternal)),
_cleanupRecentLoginUrlCacheInteractor.execute(RecentLoginUrlCleanupRule()),
_cleanupRecentLoginUsernameCacheInteractor.execute(RecentLoginUsernameCleanupRule()),
- ], eagerError: true);
+ ]).catchError((error, stackTrace) {
+ logWarning('HomeController::_cleanupCache:Exception = $error');
+ }));Note: Import unawaited from dart:async to make the fire-and-forget intent explicit. Also, eagerError: true can be removed since you're catching errors anyway and individual failures don't need to short-circuit the others.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/home/presentation/home_controller.dart` around lines 119 - 128,
Make the fire-and-forget cache cleanup explicit and surface errors: import
unawaited from dart:async and wrap the Future.wait([...]) call with
unawaited(...). Attach error handling (e.g., .catchError or try/catch inside an
async closure) to the Future.wait of _cleanupEmailCacheInteractor.execute,
_cleanupRecentLoginUrlCacheInteractor.execute and
_cleanupRecentLoginUsernameCacheInteractor.execute so any exceptions are logged
(use your controller logger or print) and remove eagerError: true since you’ll
handle errors yourself; keep getAuthenticatedAccountAction() as-is.
| // Optimistic UI update — show read/unread state immediately | ||
| updateEmailFlagByEmailIds( | ||
| [emailId], | ||
| readActions: readActions, | ||
| ); |
There was a problem hiding this comment.
Add the rollback path for failed optimistic toggles.
These flags are updated before the network call, but handleFailureViewState() around Lines 565-615 never restores the prior state for MarkAsEmailReadFailure or MarkAsStarEmailFailure. A failed request will leave the list showing the optimistic value until some later refresh, which misses the revert-on-error behavior in this PR.
Also applies to: 1246-1250
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart`
around lines 1227 - 1231, The optimistic update calling
updateEmailFlagByEmailIds([emailId], readActions: readActions) needs a rollback
on failure: before performing the optimistic update capture the prior flag state
for the affected email(s) (e.g., current read/star values), then in
handleFailureViewState() detect MarkAsEmailReadFailure and
MarkAsStarEmailFailure and call updateEmailFlagByEmailIds(...) with the captured
previous values to revert the UI; apply the same capture-and-rollback approach
for the similar optimistic block around the second occurrence (lines ~1246-1250)
so failures restore the original flags.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
fd 'update_current_emails_flags_extension\.dart$' -x sed -n '1,80p' {}
rg -n -C2 'updateEmailFlagByEmailIds\(' lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dartRepository: linagora/tmail-flutter
Length of output: 4092
Fix the named argument and add failure handlers for optimistic updates.
Line 1230 uses the wrong named argument: the parameter is readAction (singular), not readActions (plural). This will fail to compile.
Additionally, the optimistic UI update at lines 1227–1231 is never reverted if the _markAsEmailReadInteractor.execute(...) call fails. The handleFailureViewState method has no handler for MarkAsEmailReadFailure or MarkAsStarEmailFailure, leaving stale UI state after network errors. Add failure handlers that revert the optimistic changes to the previous state.
Minimal fix for the named argument
updateEmailFlagByEmailIds(
[emailId],
- readActions: readActions,
+ readAction: readActions,
);📝 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.
| // Optimistic UI update — show read/unread state immediately | |
| updateEmailFlagByEmailIds( | |
| [emailId], | |
| readActions: readActions, | |
| ); | |
| // Optimistic UI update — show read/unread state immediately | |
| updateEmailFlagByEmailIds( | |
| [emailId], | |
| readAction: readActions, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart`
around lines 1227 - 1231, Change the optimistic update call to use the correct
named parameter readAction instead of readActions in
updateEmailFlagByEmailIds([emailId], readAction: readAction) and ensure you
capture the prior read/star state before applying the optimistic change; then,
in handleFailureViewState add handlers for MarkAsEmailReadFailure and
MarkAsStarEmailFailure (the failures thrown by
_markAsEmailReadInteractor.execute and the star interactor) that revert the
optimistic UI by calling updateEmailFlagByEmailIds with the saved previous state
(e.g., readAction / star flag) for the affected emailIds so the UI is restored
on network/error failures.
|
Thanks a lot for the outstanding contribution to the project. 1 and 2 makes sens to me (let's have validation from flutter dude cc @hoangdat @dab246 ) Regarding 3 (apparently it triggered the rabbit) I would like we come up with a resilient failure management strategy. The subject is harder than it looks so I suggest we split commit 3 onto a separate pr so that we do not delay 1 and 2. Best regards Benoit |
|
Hi @JacobiusMakes Thank you for creating a PR to contribute to our project. The solutions you provided have a few points we need to consider: 1 - Remove hardcoded 2-second startup delayThe delay should only be removed on Android and Web. On iOS, HomeView displays the Lottie welcome animation if (PlatformInfo.isIOS) await Future.delayed(2.seconds);2 - Background cache cleanup (non-blocking auth)We've tracked the Hive boxes used by each operation: cleanup tasks write to Future<void> _cleanupCache() async {
await HiveCacheConfig.instance.onUpgradeDatabase(cachingManager);
Future.wait([
_cleanupEmailCacheInteractor.execute(EmailCleanupRule(Duration.defaultCacheInternal)),
_cleanupRecentLoginUrlCacheInteractor.execute(RecentLoginUrlCleanupRule()),
_cleanupRecentLoginUsernameCacheInteractor.execute(RecentLoginUsernameCleanupRule()),
]).then((_) {}).catchError((e) { logError('HomeController::_cleanupCache(): $e'); });
getAuthenticatedAccountAction();
}WDYT? |
3 participants
Summary
Three performance improvements that reduce perceived load time and improve UI responsiveness:
1. Remove hardcoded 2-second startup delay
File:
home_controller.dart:108Future.delayed(2.seconds)was blocking every app launch before any navigation logic ran. Removed entirely — no functional dependency on this delay.2. Background cache cleanup (non-blocking auth)
File:
home_controller.dart:117-125Previously,
getAuthenticatedAccountAction()waited for all three cache cleanup tasks to complete. Now:3. Optimistic UI updates for star and read/unread
File:
mailbox_dashboard_controller.dart:1238-1246, 1220-1236Star/unstar and read/unread toggles now update the UI immediately on tap, before the server confirms. The network call still happens — but the user sees instant feedback instead of waiting for the round-trip.
Fixes #4190
Related: #3694, #2955
Test plan
Generated with Claude Code
Summary by CodeRabbit
Performance Improvements
User Interface Enhancements