plan: refresh plan for issue #533

.pre-commit-config.yaml

@@ -53,9 +53,3 @@ repos:
         entry: bash -c 'cd "$(git rev-parse --show-toplevel)" && nix develop --command task check-ci-images'
         pass_filenames: false
         files: ^(ci/main\.go|\.fvmrc)$
-      - id: dagger-versions-aligned
-        name: verify Dagger version is consistent across dagger.json, flake.nix, Dockerfile and DAGGER.md
-        language: system
-        entry: bash -c 'cd "$(git rev-parse --show-toplevel)" && scripts/check_dagger_versions.sh'
-        pass_filenames: false
-        files: ^(ci/dagger\.json|flake\.nix|\.forgejo/Dockerfile|DAGGER\.md)$

ISSUE-484.md

@@ -1,100 +0,0 @@
-## Root cause analysis
-
-The "Load remote images" button is rendered in two places: `lib/ui/screens/email_detail_screen.dart:228-262` (single mail view) and `lib/ui/screens/thread_detail_screen.dart:203-237` (thread view). Both call the same pattern:
-
-```dart
-onPressed: () {
-  setState(() => _loadRemoteImages = true);   // 1. schedule rebuild
-  if (senderEmail != null) {
-    unawaited(...addTrustedImageSender(senderEmail));  // 2. fire-and-forget DB write
-    ScaffoldMessenger.of(ctx).showSnackBar(SnackBar(   // 3. queue snack bar
-      duration: const Duration(seconds: 3),
-      ...
-    ));
-  }
-}
-```
-
-Although `duration: 3s` is already set, the snack bar fails to auto-dismiss. This mirrors the bug fixed in PR #401 (issue #399): there, a snack bar fired during a navigation transition and the duration timer "didn't start correctly" because the snack bar was queued on an unstable scaffold.
-
-Here, the analogous instability comes from three rebuilds that all land between `showSnackBar` and the moment the SnackBar's enter-animation would normally complete and start its dismiss timer:
-
-1. The synchronous `setState` flips `_loadRemoteImages` → `true`, which immediately removes the "Load remote images" button (the very widget whose `onPressed` was running) and swaps the `SecureEmailWebView` into the rebuilt subtree with `loadRemoteImages: true`. The WebView's `didUpdateWidget` then triggers an async `loadHtmlString` reload (see `lib/ui/widgets/secure_email_webview.dart:100-106`), which subsequently calls `setState(() => _height = h)` inside `_measureHeight`.
-2. The fire-and-forget `addTrustedImageSender` write resolves a moment later, the `trustedImageSendersProvider` stream emits, and `ref.watch(trustedImageSendersProvider)` in `email_detail_screen.dart:197` causes another rebuild of the whole screen body — including the `Scaffold`'s body subtree that hosts the snack bar overlay's host context.
-3. These rebuilds happen during the SnackBar's enter animation, so the `_SnackBarState` ends up holding stale animation state and the per-snack-bar timer that schedules `hideCurrentSnackBar` after `duration` never fires.
-
-## Plan
-
-### Fix
-
-Queue the snack bar **before** mutating state, so it reaches `ScaffoldMessenger` while the Scaffold subtree is still stable, and defer the state change to a post-frame callback so the snack bar's enter-animation can finish before the WebView reload and the provider-driven rebuild run.
-
-In `lib/ui/screens/email_detail_screen.dart`, replace the body of `OutlinedButton.icon.onPressed` at lines 231-261 with:
-
-```dart
-onPressed: () {
-  if (senderEmail != null) {
-    unawaited(
-      ref
-          .read(userPreferencesRepositoryProvider)
-          .addTrustedImageSender(senderEmail),
-    );
-    ScaffoldMessenger.of(ctx).showSnackBar(
-      SnackBar(
-        duration: const Duration(seconds: 3),
-        content: const Text(
-          'Images will be loaded automatically for this sender.',
-        ),
-        action: SnackBarAction(
-          label: 'View',
-          onPressed: () {
-            if (mounted) {
-              unawaited(
-                context.push(
-                  '/accounts/trusted-senders',
-                  extra: senderEmail,
-                ),
-              );
-            }
-          },
-        ),
-      ),
-    );
-  }
-  WidgetsBinding.instance.addPostFrameCallback((_) {
-    if (mounted) setState(() => _loadRemoteImages = true);
-  });
-},
-```
-
-Apply the same reordering to `lib/ui/screens/thread_detail_screen.dart:206-236`.
-
-The key changes:
-- `showSnackBar` runs first, on the still-stable scaffold subtree.
-- `setState` (which triggers WebView swap-in and subsequent rebuilds) is deferred to a post-frame callback.
-- When `senderEmail == null` (no trusted-sender to register, so no snack bar), the post-frame callback still flips `_loadRemoteImages` to true — preserving existing behavior of the button working even for unknown senders.
-
-### Tests
-
-Add a widget test in `test/widget/email_detail_screen_test.dart` that:
-
-1. Pumps an `EmailDetailScreen` with an HTML body and a non-empty `From` header.
-2. Taps the "Load remote images" button.
-3. Verifies the snack bar with text "Images will be loaded automatically for this sender." appears.
-4. Calls `tester.pump(const Duration(seconds: 4))` (or uses `tester.pumpAndSettle` after a 3.5s pump).
-5. Verifies the snack bar is gone (`expect(find.byType(SnackBar), findsNothing)`).
-6. Verifies `_loadRemoteImages` did flip, by checking that the "Load remote images" button is no longer present.
-
-Add an analogous test in `test/widget/thread_detail_screen_test.dart` (or wherever thread tests live; create the file if it does not exist yet — use the email_detail test as a template).
-
-### Out of scope
-
-- The "First update agent loop, fix search bug" line in the issue body is two unrelated todo notes the reporter jotted down (the search bug is tracked separately). This plan does not address them.
-- Other `showSnackBar` call sites in `email_detail_screen.dart` (download success/failure, copy-to-clipboard, raw-email errors, etc.) are not affected by the same rebuild pattern and stay unchanged.
-
-### Verification checklist
-
-- [ ] `dart test` (or the project's `task test` equivalent) passes, including the two new widget tests.
-- [ ] Manual: open a single mail in `EmailDetailScreen` with HTML body from a sender not yet trusted; tap "Load remote images"; verify snack bar appears, images load, and snack bar disappears after ~3 seconds.
-- [ ] Manual: tap "View" on the snack bar before it dismisses; verify it navigates to `/accounts/trusted-senders` and that the snack bar is dismissed by the navigation as expected.
-- [ ] Manual: repeat in `ThreadDetailScreen`.

ISSUE-533.md

@@ -0,0 +1,101 @@
+# Plan: Consolidate Email-List UI
+
+## Goal
+
+Three list surfaces — folder view (`EmailListScreen`), combined inbox (`CombinedInboxScreen`), and search results (in `SearchScreen` plus the in-screen search in `EmailListScreen`, plus `AddressEmailsScreen`) — currently duplicate selection state, swipe-dismiss handling, batch actions (archive/delete/spam/move/snooze), and tile rendering. Unify into one widget so behaviour and UI are identical everywhere. Thread detail view is intentionally out of scope.
+
+## Current duplication
+
+| Concern | `EmailListScreen` | `CombinedInboxScreen` | `SearchScreen` / `AddressEmailsScreen` |
+| --- | --- | --- | --- |
+| Tile widget | `EmailThreadTile` + `ThreadTile` (search) | `EmailThreadTile` | `ThreadTile` / inline `ListTile` |
+| Selection set | thread + per-email | thread only | none |
+| Selection AppBar/BottomBar | full set of 5 actions | archive + delete only | n/a |
+| Swipe dismiss | archive/delete + undo | archive/delete + undo (copy) | none |
+| Batch actions | archive, delete, spam, move, snooze | archive, delete (re-implemented) | none |
+
+Tile widgets `lib/ui/widgets/email_thread_tile.dart` and `lib/ui/widgets/thread_tile.dart` render the same fields in slightly different layouts.
+
+## Target architecture
+
+### 1. New widget `lib/ui/widgets/email_thread_list.dart`
+
+A self-contained `ConsumerStatefulWidget` that owns selection state and renders the list. API:
+
+```dart
+EmailThreadList({
+  required Stream<List<EmailThread>> threads,        // folder + combined inbox
+  // or:
+  required List<EmailThread> staticThreads,          // search / address results
+  required EmailListContext listContext,             // see below
+  bool showAccountLabel = false,                     // combined inbox
+  bool showLocationLabel = false,                    // search / cross-mailbox
+  bool enableSwipe = true,
+  bool enablePagination = true,
+  List<EmailBatchAction> actions = EmailBatchAction.standard,
+  ValueChanged<EmailThread>? onTap,                  // null → default navigation
+})
+```
+
+`EmailListContext` carries `accountId?` (nullable for combined/global views) and `mailboxPath?` (nullable for combined/global views). Batch actions read these to scope role lookups and undo-action source paths; when null they fall back to per-thread `t.accountId` / `t.mailboxPath` (this is how `CombinedInboxScreen._batchArchive` already groups by account).
+
+Encapsulated:
+- `_selectedThreadIds` plus toggle/clear/select-all helpers.
+- `_currentThreads` (last stream emission).
+- `_limit` pagination with `Load more`.
+- Selection-mode `AppBar` and `BottomAppBar` rendering — driven by the host scaffold via two builders the widget exposes (`buildSelectionAppBar`, `buildSelectionBottomBar`) so the host keeps Scaffold ownership but doesn't reimplement them.
+- Swipe-to-archive / swipe-to-delete + undo push.
+
+### 2. Shared action layer `lib/ui/screens/email_action_helpers.dart`
+
+Extend the existing file with the batch ops currently duplicated:
+
+```dart
+enum EmailBatchAction { archive, delete, markSpam, move, snooze }
+
+Future<void> batchMoveToRole(WidgetRef ref, BuildContext ctx, {
+  required List<EmailThread> threads,
+  required String role,
+  required String dialogTitle,
+  required String createFolderName,
+});
+
+Future<void> batchDelete(WidgetRef ref, {required List<EmailThread> threads});
+Future<void> batchMove(WidgetRef ref, BuildContext ctx, {required List<EmailThread> threads});
+Future<void> batchSnooze(WidgetRef ref, BuildContext ctx, {required List<EmailThread> threads});
+Future<void> swipeDismiss(WidgetRef ref, EmailThread thread, DismissDirection dir);
+```
+
+Each function fetches `originalEmails`, runs the repo calls, and pushes a single `UndoAction`. Grouping by `accountId` lives here so combined-inbox-style multi-account selections work for every action (not only archive/delete). `_batchMoveToRole` from `EmailListScreen` and `_batchArchive` from `CombinedInboxScreen` collapse into one function.
+
+### 3. Unify the tile widgets
+
+Keep `ThreadTile` (`lib/ui/widgets/thread_tile.dart`) as the single tile. Move the `Dismissible` wrapper out — `EmailThreadList` owns swipe — and add the optional `showAccount` subtitle currently in `EmailThreadTile`. Delete `lib/ui/widgets/email_thread_tile.dart`.
+
+## Screen refactors
+
+- `combined_inbox_screen.dart` — drop selection state, swipe handler, batch methods, `_buildThreadList`, `_selectionBottomBar`. Replace body with `EmailThreadList(stream: emailRepo.observeAllInboxThreads(...), listContext: const EmailListContext.allAccounts(), showAccountLabel: true)`. AppBar/drawer/FAB stay.
+- `email_list_screen.dart` — keep search-bar, sync banner, folder drawer, `Mark all as read`. Replace `_buildStreamBody` and `_buildEmailList` with `EmailThreadList`. Drop selection state, `_toggleThreadSelection`, `_selectionBottomBar`, `_batch*` methods, `_onSwipeDismissed`. The search path inside this screen (results from `searchEmails`) becomes `EmailThreadList(staticThreads: results.map(EmailThread.fromEmail).toList(), enableSwipe: false)` — the per-email vs per-thread split goes away once everything is treated as a thread of one.
+- `search_screen.dart` — Messages section uses `EmailThreadList(staticThreads: ..., enableSwipe: false, showLocationLabel: true, actions: EmailBatchAction.standard)`, so global search results now support the same selection + batch actions. Folders and Addresses sections unchanged.
+- `address_emails_screen.dart` — replace inline `ListView.builder` with `EmailThreadList`, gaining selection/swipe/batch parity.
+
+## Migration steps
+
+1. Add `EmailThreadList` widget with selection, swipe, pagination, and AppBar/BottomBar builders. Lift the existing logic verbatim from `EmailListScreen` so behaviour is unchanged.
+2. Promote the five batch ops + swipe handler into `email_action_helpers.dart`; switch `EmailListScreen` to call them. Keep tile tests passing.
+3. Fold `showAccount` and `Dismissible`-out into `ThreadTile`; delete `EmailThreadTile`; update imports.
+4. Migrate `CombinedInboxScreen` to `EmailThreadList`. Combined inbox now supports spam/move/snooze (was missing). Verify multi-account batches still group correctly.
+5. Migrate the search-result branch inside `EmailListScreen`, then the Messages section of `SearchScreen`, then `AddressEmailsScreen`.
+6. Run `flutter analyze` and the integration tests under `integration_test/` (folder, combined inbox, and search exercise the same code path now, so a single regression test set covers all surfaces).
+
+## Out of scope
+
+- `ThreadDetailScreen` — single-thread message list, intentionally different.
+- Repository / DB code in `lib/core/repositories/` — no changes; the unification is purely on the UI layer.
+- Folder drawer, sync banner, search bar — remain owned by their hosting screens.
+
+## Risks / open questions
+
+- Combined inbox currently has no `mailboxPath` per selection — confirmed handled by grouping selected threads by `accountId` then looking up archive/junk/etc. per group. The same grouping must work for the move/snooze sheet (the destination picker needs an account; when multiple accounts are selected, prompt per-account or block — recommend block with a SnackBar to mirror existing per-folder constraints; flag for user feedback).
+- Snooze for cross-account selection: same constraint as above — implementation should iterate accounts.
+- Swipe-dismiss in search results: currently disabled in `SearchScreen` and `AddressEmailsScreen`. Plan keeps `enableSwipe: false` for those to avoid disorienting users when a swiped item disappears from a filtered list; revisit if user wants parity.

Taskfile.yml

@@ -712,11 +712,6 @@ tasks:
     cmds:
       - scripts/check_ci_images.sh
 
-  check-dagger-versions:
-    desc: Verify ci/dagger.json, flake.nix, .forgejo/Dockerfile and DAGGER.md pin the same Dagger version
-    cmds:
-      - scripts/check_dagger_versions.sh
-
   _integrations:
     internal: true
     run: once

flake.nix

@@ -49,16 +49,14 @@
           '';
         };
 
-        # The dagger/nix flake's Nix wrapper is a broken self-exec loop, so we
-        # fetch the CLI binary directly.  Keep this version in lockstep with
-        # ci/dagger.json (engineVersion) and .forgejo/Dockerfile (DAGGER_VERSION) —
-        # scripts/check_dagger_versions.sh enforces this.
-        daggerCli = pkgs.stdenv.mkDerivation {
+        # The dagger/nix flake pins 0.20.8, whose Nix wrapper is a broken self-exec
+        # loop.  Fetch 0.21.4 directly so the pre-commit dart-check hook can run.
+        dagger021 = pkgs.stdenv.mkDerivation {
           pname = "dagger";
-          version = "0.20.8";
+          version = "0.21.4";
           src = pkgs.fetchurl {
-            url = "https://dl.dagger.io/dagger/releases/0.20.8/dagger_v0.20.8_linux_amd64.tar.gz";
-            sha256 = "1ns6wq2z1skd2fq9lbrcali0s8kn24p3haamnjjgchg6zlv6b960";
+            url = "https://dl.dagger.io/dagger/releases/0.21.4/dagger_v0.21.4_linux_amd64.tar.gz";
+            sha256 = "0wlnbr4g5069755131yjp2a6alacn64f1c8b27xn0cbynq3zicjd";
           };
           sourceRoot = ".";
           installPhase = ''
@@ -71,7 +69,7 @@
         devShells.default = pkgs.mkShell {
           buildInputs = with pkgs; [
             # Dagger CLI
-            daggerCli
+            dagger021
 
             # Go compiler — for Dagger development
             go

scripts/check_dagger_versions.sh

@@ -1,49 +0,0 @@
-#!/usr/bin/env bash
-# Verify that the Dagger version is consistent across the project.
-#
-# The Dagger CLI must speak the same protocol as the engine it talks to.  We
-# pin the version in four places (engine image in DAGGER.md, the CLI in
-# flake.nix, the CLI in the Forgejo runner Dockerfile, and the module
-# engineVersion in ci/dagger.json).  This script fails if any of them drift.
-set -euo pipefail
-
-ROOT=$(git rev-parse --show-toplevel)
-
-# ci/dagger.json — strip leading "v" for comparison.
-dagger_json=$(grep -oE '"engineVersion"[[:space:]]*:[[:space:]]*"[^"]+"' "$ROOT/ci/dagger.json" \
-  | sed -E 's/.*"v?([^"]+)"$/\1/')
-
-# flake.nix — the dagger021 derivation's CLI download URL.
-flake_nix=$(grep -oE 'dagger_v[0-9]+\.[0-9]+\.[0-9]+_linux' "$ROOT/flake.nix" \
-  | head -n1 \
-  | sed -E 's/dagger_v([0-9.]+)_linux/\1/')
-
-# .forgejo/Dockerfile — DAGGER_VERSION env on the install line.
-dockerfile=$(grep -oE 'DAGGER_VERSION=[0-9]+\.[0-9]+\.[0-9]+' "$ROOT/.forgejo/Dockerfile" \
-  | head -n1 \
-  | cut -d= -f2)
-
-# DAGGER.md — engine image tag in the example systemd unit.
-dagger_md=$(grep -oE 'dagger/nix/v[0-9]+\.[0-9]+\.[0-9]+' "$ROOT/DAGGER.md" \
-  | head -n1 \
-  | sed -E 's@.*/v@@')
-
-printf 'ci/dagger.json    engineVersion = v%s\n' "$dagger_json"
-printf 'flake.nix         dagger021     = %s\n'  "$flake_nix"
-printf '.forgejo/Dockerf. DAGGER_VERSION= %s\n'  "$dockerfile"
-printf 'DAGGER.md         engine tag    = v%s\n' "$dagger_md"
-
-for v in "$flake_nix" "$dockerfile" "$dagger_md"; do
-  if [ -z "$v" ]; then
-    echo "ERROR: failed to parse a Dagger version reference." >&2
-    exit 1
-  fi
-  if [ "$v" != "$dagger_json" ]; then
-    echo "" >&2
-    echo "ERROR: Dagger versions are out of sync." >&2
-    echo "  Align ci/dagger.json, flake.nix, .forgejo/Dockerfile and DAGGER.md to the same version." >&2
-    exit 1
-  fi
-done
-
-echo "Dagger versions aligned (v$dagger_json)."