guettli/sharedinbox
1
0
Fork 0
Code Issues 21 Pull Requests 6 Actions Packages Projects Releases Wiki Activity

fix: implement global undo UI and optimistic IMAP moves for better UX

Browse Source
This commit is contained in:
Thomas SharedInbox
2026-05-09 15:35:17 +02:00
parent e2759ac062
commit d405b37308
18 changed files with 306 additions and 143 deletions
Download Patch File Download Diff File Expand all files Collapse all files
done.md
+9
View File
@@ -6,6 +6,15 @@ Tasks get moved from next.md to done.md
## Tasks
- **Optimistic UI**: Both IMAP and JMAP `moveEmail` operations are now optimistic,
updating the local database immediately instead of waiting for sync. This
provides instant feedback and ensures rows are available for Undo actions.
- **Global Undo Support**: Introduced `UndoShell` and `ShellRoute` to provide a
consistent "Undo" experience across all screens, automatically surfacing the
Undo SnackBar whenever a destructive action is performed.
- **Improved Thread Support**: Fixed a bug where deleting emails from the
`ThreadDetailScreen` lacked Undo logic.
## Undo Feature Fix (IMAP)
Fixed a bug where undoing an email deletion or move would fail for IMAP accounts
lib/core/models/email.dart
+42
View File
@@ -40,6 +40,48 @@ class Email {
this.inReplyTo,
this.references,
});
Email copyWith({
String? id,
String? accountId,
String? mailboxPath,
int? uid,
String? subject,
DateTime? sentAt,
DateTime? receivedAt,
List<EmailAddress>? from,
List<EmailAddress>? to,
List<EmailAddress>? cc,
String? preview,
bool? isSeen,
bool? isFlagged,
bool? hasAttachment,
String? threadId,
String? messageId,
String? inReplyTo,
String? references,
}) {
return Email(
id: id ?? this.id,
accountId: accountId ?? this.accountId,
mailboxPath: mailboxPath ?? this.mailboxPath,
uid: uid ?? this.uid,
subject: subject ?? this.subject,
sentAt: sentAt ?? this.sentAt,
receivedAt: receivedAt ?? this.receivedAt,
from: from ?? this.from,
to: to ?? this.to,
cc: cc ?? this.cc,
preview: preview ?? this.preview,
isSeen: isSeen ?? this.isSeen,
isFlagged: isFlagged ?? this.isFlagged,
hasAttachment: hasAttachment ?? this.hasAttachment,
threadId: threadId ?? this.threadId,
messageId: messageId ?? this.messageId,
inReplyTo: inReplyTo ?? this.inReplyTo,
references: references ?? this.references,
);
}
}
/// A group of related emails sharing the same thread.
lib/core/repositories/email_repository.dart
+5 -1
View File
@@ -27,7 +27,11 @@ abstract class EmailRepository {
bool? flagged,
});
Future<void> moveEmail(String emailId, String destMailboxPath);
Future<void> deleteEmail(String emailId);
/// Deletes the email. Returns the path of the mailbox it was moved to
/// (e.g. Trash) if it was a soft-delete, or null if it was hard-deleted.
Future<String?> deleteEmail(String emailId);
Future<void> sendEmail(String accountId, EmailDraft draft);
/// Downloads [attachment] bytes from the server (or local cache) and returns
lib/core/services/undo_service.dart
+21 -12
View File
@@ -31,30 +31,39 @@ class UndoService extends StateNotifier<UndoAction?> {
state = _history.isNotEmpty ? _history.last : null;
final repo = _ref.read(emailRepositoryProvider);
// For IMAP, the rows were hard-deleted, so we must restore them first.
if (action.originalEmails.isNotEmpty) {
await repo.restoreEmails(action.originalEmails);
}
for (final id in action.emailIds) {
// Try to cancel the original change.
// Deletes might have been implemented as moves to Trash, so try both.
// 1. Try to cancel the original change (if not started yet).
final cancelled = await repo.cancelPendingChange(id, 'delete') ||
await repo.cancelPendingChange(id, 'move');
// Move the email back to its source to reverse local DB state and
// (if not cancelled) enqueue the reverse change on the server.
try {
final original = action.originalEmails.isEmpty
? null
: action.originalEmails.where((e) => e.id == id).firstOrNull;
// 2. If row is missing (hard delete), restore it first.
// We restore it at its CURRENT state (where it is on the server,
// or where it was moving to).
if (original != null) {
final currentPath = cancelled
? action.sourceMailboxPath
: (action.destinationMailboxPath ?? action.sourceMailboxPath);
await repo.restoreEmails([original.copyWith(mailboxPath: currentPath)]);
}
// 3. Move it back to source.
// This updates local DB optimistically and (if not cancelled) enqueues
// a reverse move on the server.
await repo.moveEmail(id, action.sourceMailboxPath);
if (cancelled) {
// If we cancelled the original change, and then moved it back,
// we've just enqueued a NEW 'move' change that is redundant.
// 4. If we successfully cancelled the original, the reverse move
// we just enqueued is redundant.
await repo.cancelPendingChange(id, 'move');
}
} catch (e) {
// If it still fails, nothing more we can do locally.
// Best effort.
}
}
}
lib/data/repositories/email_repository_impl.dart
+15 -5
View File
@@ -1478,17 +1478,25 @@ class EmailRepositoryImpl implements EmailRepository {
'dest': destMailboxPath,
}),
);
await (_db.delete(_db.emails)..where((t) => t.id.equals(emailId))).go();
// Optimistic: move the cached row locally instead of hard-deleting.
await (_db.update(_db.emails)..where((t) => t.id.equals(emailId))).write(
EmailsCompanion(mailboxPath: Value(destMailboxPath)),
);
await _updateThread(
row.accountId,
row.mailboxPath,
row.threadId ?? emailId,
);
// Destination will be updated when synced (IMAP move is a delete + copy).
await _updateThread(
row.accountId,
destMailboxPath,
row.threadId ?? emailId,
);
// Destination UID will be updated when synced (IMAP move is a delete + copy).
}
@override
Future<void> deleteEmail(String emailId) async {
Future<String?> deleteEmail(String emailId) async {
final row = await (_db.select(_db.emails)
..where((t) => t.id.equals(emailId)))
.getSingle();
@@ -1503,7 +1511,8 @@ class EmailRepositoryImpl implements EmailRepository {
.getSingleOrNull();
if (trashRow != null && trashRow.path != row.mailboxPath) {
return moveEmail(emailId, trashRow.path);
await moveEmail(emailId, trashRow.path);
return trashRow.path;
}
// Already in Trash or no Trash folder — hard delete.
@@ -1520,7 +1529,7 @@ class EmailRepositoryImpl implements EmailRepository {
row.mailboxPath,
row.threadId ?? emailId,
);
return;
return null;
}
await _enqueueChange(
@@ -1535,6 +1544,7 @@ class EmailRepositoryImpl implements EmailRepository {
row.mailboxPath,
row.threadId ?? emailId,
);
return null;
}
// ── pending_changes queue ──────────────────────────────────────────────────
lib/ui/router.dart
+90 -84
View File
@@ -15,109 +15,115 @@ import 'package:sharedinbox/ui/screens/sieve_script_edit_screen.dart';
import 'package:sharedinbox/ui/screens/sieve_scripts_screen.dart';
import 'package:sharedinbox/ui/screens/sync_log_screen.dart';
import 'package:sharedinbox/ui/screens/thread_detail_screen.dart';
import 'package:sharedinbox/ui/widgets/undo_shell.dart';
final router = GoRouter(
initialLocation: '/accounts',
routes: [
GoRoute(
path: '/accounts',
builder: (ctx, state) => const AccountListScreen(),
ShellRoute(
builder: (ctx, state, child) => UndoShell(child: child),
routes: [
GoRoute(
path: 'add',
builder: (ctx, state) => const AddAccountScreen(),
),
GoRoute(
path: ':accountId/edit',
builder: (ctx, state) => EditAccountScreen(
accountId: state.pathParameters['accountId']!,
),
),
GoRoute(
path: ':accountId/sync-log',
builder: (ctx, state) => SyncLogScreen(
accountId: state.pathParameters['accountId']!,
),
),
GoRoute(
path: ':accountId/sieve',
builder: (ctx, state) => SieveScriptsScreen(
accountId: state.pathParameters['accountId']!,
),
),
GoRoute(
path: ':accountId/sieve/edit',
builder: (ctx, state) => SieveScriptEditScreen(
accountId: state.pathParameters['accountId']!,
script: state.extra as SieveScript?,
),
),
GoRoute(
path: ':accountId/search',
builder: (ctx, state) => SearchScreen(
accountId: state.pathParameters['accountId']!,
),
),
GoRoute(
path: ':accountId/emails/by-address/:address',
builder: (ctx, state) => AddressEmailsScreen(
accountId: state.pathParameters['accountId']!,
address: state.pathParameters['address']!,
),
),
GoRoute(
path: ':accountId/mailboxes',
builder: (ctx, state) =>
MailboxListScreen(accountId: state.pathParameters['accountId']!),
path: '/accounts',
builder: (ctx, state) => const AccountListScreen(),
routes: [
GoRoute(
path: ':mailboxPath/emails',
builder: (ctx, state) => EmailListScreen(
path: 'add',
builder: (ctx, state) => const AddAccountScreen(),
),
GoRoute(
path: ':accountId/edit',
builder: (ctx, state) => EditAccountScreen(
accountId: state.pathParameters['accountId']!,
mailboxPath: state.pathParameters['mailboxPath']!,
),
),
GoRoute(
path: ':accountId/sync-log',
builder: (ctx, state) => SyncLogScreen(
accountId: state.pathParameters['accountId']!,
),
),
GoRoute(
path: ':accountId/sieve',
builder: (ctx, state) => SieveScriptsScreen(
accountId: state.pathParameters['accountId']!,
),
),
GoRoute(
path: ':accountId/sieve/edit',
builder: (ctx, state) => SieveScriptEditScreen(
accountId: state.pathParameters['accountId']!,
script: state.extra as SieveScript?,
),
),
GoRoute(
path: ':accountId/search',
builder: (ctx, state) => SearchScreen(
accountId: state.pathParameters['accountId']!,
),
),
GoRoute(
path: ':accountId/emails/by-address/:address',
builder: (ctx, state) => AddressEmailsScreen(
accountId: state.pathParameters['accountId']!,
address: state.pathParameters['address']!,
),
),
GoRoute(
path: ':accountId/mailboxes',
builder: (ctx, state) => MailboxListScreen(
accountId: state.pathParameters['accountId']!,),
routes: [
GoRoute(
path: ':emailId',
builder: (ctx, state) => EmailDetailScreen(
emailId: state.pathParameters['emailId']!,
path: ':mailboxPath/emails',
builder: (ctx, state) => EmailListScreen(
accountId: state.pathParameters['accountId']!,
mailboxPath: state.pathParameters['mailboxPath']!,
),
routes: [
GoRoute(
path: ':emailId',
builder: (ctx, state) => EmailDetailScreen(
emailId: state.pathParameters['emailId']!,
),
),
],
),
GoRoute(
path: ':mailboxPath/threads/:threadId',
builder: (ctx, state) => ThreadDetailScreen(
accountId: state.pathParameters['accountId']!,
mailboxPath: Uri.decodeComponent(
state.pathParameters['mailboxPath']!,
),
threadId: Uri.decodeComponent(
state.pathParameters['threadId']!,
),
),
),
],
),
GoRoute(
path: ':mailboxPath/threads/:threadId',
builder: (ctx, state) => ThreadDetailScreen(
accountId: state.pathParameters['accountId']!,
mailboxPath: Uri.decodeComponent(
state.pathParameters['mailboxPath']!,
),
threadId: Uri.decodeComponent(
state.pathParameters['threadId']!,
),
),
),
],
),
GoRoute(
path: '/search',
builder: (ctx, state) => const SearchScreen(),
),
GoRoute(
path: '/compose',
builder: (ctx, state) {
final extra = state.extra as Map<String, dynamic>?;
return ComposeScreen(
accountId: extra?['accountId'] as String?,
replyToEmailId: extra?['replyToEmailId'] as String?,
prefillTo: extra?['prefillTo'] as String?,
prefillCc: extra?['prefillCc'] as String?,
prefillSubject: extra?['prefillSubject'] as String?,
prefillBody: extra?['prefillBody'] as String?,
);
},
),
],
),
GoRoute(
path: '/search',
builder: (ctx, state) => const SearchScreen(),
),
GoRoute(
path: '/compose',
builder: (ctx, state) {
final extra = state.extra as Map<String, dynamic>?;
return ComposeScreen(
accountId: extra?['accountId'] as String?,
replyToEmailId: extra?['replyToEmailId'] as String?,
prefillTo: extra?['prefillTo'] as String?,
prefillCc: extra?['prefillCc'] as String?,
prefillSubject: extra?['prefillSubject'] as String?,
prefillBody: extra?['prefillBody'] as String?,
);
},
),
],
);
lib/ui/screens/email_detail_screen.dart
+2 -1
View File
@@ -133,7 +133,7 @@ class _EmailDetailScreenState extends ConsumerState<EmailDetailScreen> {
),
);
if (confirmed != true || !context.mounted) return;
await repo.deleteEmail(widget.emailId);
final destPath = await repo.deleteEmail(widget.emailId);
if (header != null) {
ref.read(undoServiceProvider.notifier).pushAction(
@@ -143,6 +143,7 @@ class _EmailDetailScreenState extends ConsumerState<EmailDetailScreen> {
type: UndoType.delete,
emailIds: [widget.emailId],
sourceMailboxPath: header.mailboxPath,
destinationMailboxPath: destPath,
originalEmails: [header],
),
);
lib/ui/screens/email_list_screen.dart
+6 -25
View File
@@ -118,34 +118,11 @@ class _EmailListScreenState extends ConsumerState<EmailListScreen> {
if (value.trim().isNotEmpty) unawaited(_runSearch(value.trim()));
}
void _showUndoSnackbar(UndoAction action) {
ScaffoldMessenger.of(context).clearSnackBars();
ScaffoldMessenger.of(context).showSnackBar(
SnackBar(
content: Text(
action.type == UndoType.delete
? '${action.emailIds.length} email(s) moved to Trash'
: '${action.emailIds.length} email(s) moved',
),
action: SnackBarAction(
label: 'Undo',
onPressed: () => ref.read(undoServiceProvider.notifier).undo(),
),
),
);
}
@override
Widget build(BuildContext context) {
final repo = ref.watch(emailRepositoryProvider);
final accountAsync = ref.watch(accountByIdProvider(widget.accountId));
ref.listen<UndoAction?>(undoServiceProvider, (previous, next) {
if (next != null && previous?.id != next.id) {
_showUndoSnackbar(next);
}
});
return Scaffold(
appBar: _selecting ? _selectionBar() : _normalBar(repo, accountAsync),
drawer: _selecting
@@ -389,8 +366,9 @@ class _EmailListScreenState extends ConsumerState<EmailListScreen> {
.whereType<Email>()
.toList();
String? lastDestPath;
for (final id in ids) {
await repo.deleteEmail(id);
lastDestPath = await repo.deleteEmail(id);
}
final action = UndoAction(
@@ -399,6 +377,7 @@ class _EmailListScreenState extends ConsumerState<EmailListScreen> {
type: UndoType.delete,
emailIds: ids,
sourceMailboxPath: widget.mailboxPath,
destinationMailboxPath: lastDestPath,
originalEmails: originalEmails,
);
ref.read(undoServiceProvider.notifier).pushAction(action);
@@ -626,8 +605,9 @@ class _EmailListScreenState extends ConsumerState<EmailListScreen> {
);
ref.read(undoServiceProvider.notifier).pushAction(action);
} else {
String? lastDestPath;
for (final id in t.emailIds) {
await repo.deleteEmail(id);
lastDestPath = await repo.deleteEmail(id);
}
final action = UndoAction(
@@ -636,6 +616,7 @@ class _EmailListScreenState extends ConsumerState<EmailListScreen> {
type: type,
emailIds: t.emailIds,
sourceMailboxPath: widget.mailboxPath,
destinationMailboxPath: lastDestPath,
originalEmails: originalEmails,
);
ref.read(undoServiceProvider.notifier).pushAction(action);
lib/ui/screens/thread_detail_screen.dart
+20 -1
View File
@@ -7,6 +7,7 @@ import 'package:go_router/go_router.dart';
import 'package:intl/intl.dart';
import 'package:sharedinbox/core/models/email.dart';
import 'package:sharedinbox/core/models/undo_action.dart';
import 'package:sharedinbox/core/utils/html_utils.dart';
import 'package:sharedinbox/di.dart';
@@ -250,7 +251,25 @@ class _EmailMessageCardState extends ConsumerState<_EmailMessageCard> {
),
);
if (confirmed == true) {
unawaited(ref.read(emailRepositoryProvider).deleteEmail(widget.email.id));
final repo = ref.read(emailRepositoryProvider);
// Fetch data first for IMAP undo support
final original = await repo.getEmail(widget.email.id);
final destPath = await repo.deleteEmail(widget.email.id);
if (original != null) {
ref.read(undoServiceProvider.notifier).pushAction(
UndoAction(
id: DateTime.now().toIso8601String(),
accountId: widget.email.accountId,
type: UndoType.delete,
emailIds: [widget.email.id],
sourceMailboxPath: widget.email.mailboxPath,
destinationMailboxPath: destPath,
originalEmails: [original],
),
);
}
}
}
}
lib/ui/widgets/undo_shell.dart
+39
View File
@@ -0,0 +1,39 @@
import 'package:flutter/material.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:sharedinbox/core/models/undo_action.dart';
import 'package:sharedinbox/di.dart';
class UndoShell extends ConsumerWidget {
const UndoShell({super.key, required this.child});
final Widget child;
@override
Widget build(BuildContext context, WidgetRef ref) {
ref.listen<UndoAction?>(undoServiceProvider, (previous, next) {
if (next != null && previous?.id != next.id) {
_showUndoSnackbar(context, ref, next);
}
});
return child;
}
void _showUndoSnackbar(BuildContext context, WidgetRef ref, UndoAction action) {
final scaffoldMessenger = ScaffoldMessenger.of(context);
scaffoldMessenger.clearSnackBars();
scaffoldMessenger.showSnackBar(
SnackBar(
content: Text(
action.type == UndoType.delete
? '${action.emailIds.length} email(s) moved to Trash'
: '${action.emailIds.length} email(s) moved',
),
action: SnackBarAction(
label: 'Undo',
onPressed: () => ref.read(undoServiceProvider.notifier).undo(),
),
),
);
}
}
test/integration/account_sync_manager_test.dart
+1 -1
View File
@@ -138,7 +138,7 @@ class _FakeEmails implements EmailRepository {
Future<void> restoreEmails(List<Email> emails) async {}
@override
Future<void> deleteEmail(String id) async {}
Future<String?> deleteEmail(String id) async => null;
@override
Stream<String> get onChangesQueued => const Stream.empty();
test/unit/account_sync_manager_test.dart
+1 -1
View File
@@ -61,7 +61,7 @@ class FakeEmailRepository implements EmailRepository {
Future<void> restoreEmails(List<Email> emails) async {}
@override
Future<void> deleteEmail(String id) async {}
Future<String?> deleteEmail(String id) async => null;
@override
Stream<String> get onChangesQueued => const Stream.empty();
@override
test/unit/account_sync_manager_test.mocks.dart
+3 -4
View File
@@ -341,14 +341,13 @@ class MockEmailRepository extends _i1.Mock implements _i9.EmailRepository {
) as _i4.Future<void>);
@override
_i4.Future<void> deleteEmail(String? emailId) => (super.noSuchMethod(
_i4.Future<String?> deleteEmail(String? emailId) => (super.noSuchMethod(
Invocation.method(
#deleteEmail,
[emailId],
),
returnValue: _i4.Future<void>.value(),
returnValueForMissingStub: _i4.Future<void>.value(),
) as _i4.Future<void>);
returnValue: _i4.Future<String?>.value(),
) as _i4.Future<String?>);
@override
_i4.Future<void> sendEmail(
test/unit/email_repository_impl_test.dart
+5 -2
View File
@@ -513,7 +513,7 @@ void main() {
expect(changes.first.payload, contains('"flagged":false'));
});
test('moveEmail enqueues move change and removes email from local DB',
test('moveEmail enqueues move change and updates local mailboxPath (optimistic)',
() async {
final r = _makeRepos();
await r.accounts.addAccount(_account, 'pw');
@@ -532,7 +532,10 @@ void main() {
final changes = await r.db.select(r.db.pendingChanges).get();
expect(changes.first.changeType, 'move');
expect(changes.first.payload, contains('Archive'));
expect(await r.emails.getEmail('acc-1:5'), isNull);
final email = await r.emails.getEmail('acc-1:5');
expect(email, isNotNull);
expect(email!.mailboxPath, 'Archive');
});
test('deleteEmail enqueues delete change and removes email from local DB',
test/unit/undo_reproduction_test.dart → test/unit/undo_logic_test.dart
+42
View File
@@ -1,3 +1,4 @@
import 'dart:convert';
import 'package:drift/drift.dart' show Value;
import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:flutter_test/flutter_test.dart';
@@ -181,4 +182,45 @@ void main() {
.get();
expect(restored, isNotEmpty, reason: 'JMAP email should be restored to Inbox after undo');
});
test('Undo deletion for IMAP enqueues reverse move if cancel fails', () async {
const emailId = 'acc1:101';
final original = await repo.getEmail(emailId);
// 1. Delete
final destPath = await repo.deleteEmail(emailId);
expect(destPath, 'Trash');
// 2. Mark the pending change as "attempted" so it cannot be cancelled
await (db.update(db.pendingChanges)..where((t) => t.resourceId.equals(emailId))).write(
const PendingChangesCompanion(attempts: Value(1)),
);
// 3. Undo
final action = UndoAction(
id: 'undo3',
accountId: 'acc1',
type: UndoType.delete,
emailIds: [emailId],
sourceMailboxPath: 'INBOX',
destinationMailboxPath: destPath,
originalEmails: [original!],
);
container.read(undoServiceProvider.notifier).pushAction(action);
await container.read(undoServiceProvider.notifier).undo();
// 4. Verify local state
final restored = await (db.select(db.emails)
..where((t) => t.id.equals(emailId))
..where((t) => t.mailboxPath.equals('INBOX')))
.get();
expect(restored, isNotEmpty);
// 5. Verify a NEW pending change was enqueued (Trash -> INBOX)
final changes = await db.select(db.pendingChanges).get();
final reverseMove = changes.firstWhere((c) => c.changeType == 'move' && c.attempts == 0);
final payload = jsonDecode(reverseMove.payload) as Map<String, dynamic>;
expect(payload['mailboxPath'], 'Trash');
expect(payload['dest'], 'INBOX');
});
}
test/unit/undo_service_test.dart
+1 -1
View File
@@ -143,7 +143,7 @@ void main() {
container.read(undoServiceProvider.notifier).pushAction(action);
await container.read(undoServiceProvider.notifier).undo();
verify(mockEmailRepo.restoreEmails([email])).called(1);
verify(mockEmailRepo.restoreEmails(any)).called(1);
verify(mockEmailRepo.moveEmail('e1', 'INBOX')).called(1);
});
test/unit/undo_service_test.mocks.dart
+3 -4
View File
@@ -209,14 +209,13 @@ class MockEmailRepository extends _i1.Mock implements _i3.EmailRepository {
) as _i4.Future<void>);
@override
_i4.Future<void> deleteEmail(String? emailId) => (super.noSuchMethod(
_i4.Future<String?> deleteEmail(String? emailId) => (super.noSuchMethod(
Invocation.method(
#deleteEmail,
[emailId],
),
returnValue: _i4.Future<void>.value(),
returnValueForMissingStub: _i4.Future<void>.value(),
) as _i4.Future<void>);
returnValue: _i4.Future<String?>.value(),
) as _i4.Future<String?>);
@override
_i4.Future<void> sendEmail(
test/widget/helpers.dart
+1 -1
View File
@@ -213,7 +213,7 @@ class FakeEmailRepository implements EmailRepository {
Future<void> restoreEmails(List<Email> emails) async {}
@override
Future<void> deleteEmail(String emailId) async {}
Future<String?> deleteEmail(String emailId) async => null;
@override
Stream<String> get onChangesQueued => const Stream.empty();