From ef931a0d6e38f105ad964f473c76987682239c11 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Thu, 2 Jul 2026 16:48:35 -0500 Subject: [PATCH] fix(mailbox): show spinner during initial folder fetch instead of empty state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit selectFolder() kicked off its background syncFolder() fetch fire-and-forget with no loading flag, so opening a per-account folder with no cached messages yet flashed "No messages to display" for the whole IMAP fetch instead of a spinner. Adds isSyncingFolder, a StateFlow set for the duration of that sync (mirroring isRefreshing) and cleared via try/finally regardless of outcome. A private latestFolderSelection token guards the clear so a stale sync from a folder no longer selected can't hide the spinner for whichever folder is actually selected now. MailboxScreen's non-paged empty branch now holds NoMessagesState back while isSyncingFolder is true, showing a CircularProgressIndicator instead — mirroring how the unified-inbox paged branch already gates on loadState.refresh. Closes #149 Co-Authored-By: Claude Opus 4.8 --- .../org/libremail/ui/mailbox/MailboxScreen.kt | 9 ++ .../libremail/ui/mailbox/MailboxViewModel.kt | 29 +++++- .../ui/mailbox/MailboxViewModelTest.kt | 90 +++++++++++++++++++ 3 files changed, 127 insertions(+), 1 deletion(-) diff --git a/app/src/main/kotlin/org/libremail/ui/mailbox/MailboxScreen.kt b/app/src/main/kotlin/org/libremail/ui/mailbox/MailboxScreen.kt index 3a5704c..a96c1ff 100644 --- a/app/src/main/kotlin/org/libremail/ui/mailbox/MailboxScreen.kt +++ b/app/src/main/kotlin/org/libremail/ui/mailbox/MailboxScreen.kt @@ -120,6 +120,7 @@ fun MailboxScreen( val searchActive by viewModel.searchActive.collectAsStateWithLifecycle() val searchQuery by viewModel.searchQuery.collectAsStateWithLifecycle() val isRefreshing by viewModel.isRefreshing.collectAsStateWithLifecycle() + val isSyncingFolder by viewModel.isSyncingFolder.collectAsStateWithLifecycle() val error by viewModel.error.collectAsStateWithLifecycle() val selectedIds by viewModel.selectedIds.collectAsStateWithLifecycle() val pendingConfirm by viewModel.pendingConfirm.collectAsStateWithLifecycle() @@ -330,8 +331,16 @@ fun MailboxScreen( } } else if (messages.isEmpty()) { item { + // Hold the empty state back while the folder's initial background + // sync is still in flight, so opening an uncached folder doesn't + // flash "No messages to display" before the fetch has a chance to + // populate anything (issue #149). if (searchActive && searchQuery.isNotBlank()) { NoResultsState(Modifier.fillParentMaxSize()) + } else if (isSyncingFolder) { + Box(Modifier.fillParentMaxSize(), contentAlignment = Alignment.Center) { + CircularProgressIndicator() + } } else { NoMessagesState(Modifier.fillParentMaxSize()) } diff --git a/app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt b/app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt index 0a25be6..6e76da2 100644 --- a/app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt @@ -186,6 +186,22 @@ class MailboxViewModel @Inject constructor( private val _isRefreshing = MutableStateFlow(false) val isRefreshing: StateFlow = _isRefreshing.asStateFlow() + /** + * True while [selectFolder]'s background sync for the currently selected folder is in flight, + * so the screen can hold the empty state back until the initial fetch either populates the + * cache or confirms the folder really is empty (issue #149). + */ + private val _isSyncingFolder = MutableStateFlow(false) + val isSyncingFolder: StateFlow = _isSyncingFolder.asStateFlow() + + /** + * (accountId, folder) of the most recent [selectFolder] call. Used only so a completing sync + * can tell whether it's stale — superseded by a newer folder selection — before clearing + * [_isSyncingFolder], so rapid folder switching can't let a stale sync hide the spinner for + * whichever folder is actually selected now. + */ + private var latestFolderSelection: Pair? = null + private val _error = MutableStateFlow(null) val error: StateFlow = _error.asStateFlow() @@ -365,7 +381,18 @@ class MailboxViewModel @Inject constructor( _selectedAccountId.value = accountId explicitDrawerAccountId.value = accountId _selectedFolder.value = folderFullName - viewModelScope.launch { mailSyncer.syncFolder(accountId, folderFullName) } + val key = accountId to folderFullName + latestFolderSelection = key + _isSyncingFolder.value = true + viewModelScope.launch { + try { + mailSyncer.syncFolder(accountId, folderFullName) + } finally { + // Only clear if this is still the most recent selection — a stale sync superseded + // by another selectFolder() must not clobber the newer one's in-flight spinner. + if (latestFolderSelection == key) _isSyncingFolder.value = false + } + } } /** Returns to the unified inbox across all accounts. */ diff --git a/app/src/test/kotlin/org/libremail/ui/mailbox/MailboxViewModelTest.kt b/app/src/test/kotlin/org/libremail/ui/mailbox/MailboxViewModelTest.kt index c90ea90..5cc6834 100644 --- a/app/src/test/kotlin/org/libremail/ui/mailbox/MailboxViewModelTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/mailbox/MailboxViewModelTest.kt @@ -8,6 +8,7 @@ import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every import io.mockk.mockk +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableSharedFlow @@ -108,6 +109,95 @@ class MailboxViewModelTest { coVerify { syncer.syncFolder("imap:a", "Archive") } } + // Issue #149: opening a per-account folder with no cached messages yet must show a spinner + // (via isSyncingFolder) rather than "No messages to display" until the background sync settles. + @Test + fun `isSyncingFolder is true while the initial folder sync is in flight, then clears`() = runTest(testDispatcher) { + val syncer = mockk() + val gate = CompletableDeferred() + coEvery { syncer.syncFolder("imap:a", "Archive") } coAnswers { + gate.await() + Result.success(0) + } + val vm = createViewModel(accounts = listOf(alice), messages = emptyList(), syncer = syncer) + backgroundScope.launch { vm.messages.collect {} } + + vm.isSyncingFolder.test { + assertEquals(false, awaitItem()) + + vm.selectFolder("imap:a", "Archive") + assertEquals(true, awaitItem()) + + gate.complete(Unit) + assertEquals(false, awaitItem()) + } + } + + // A failed sync must still clear the flag (try/finally), so a folder that errors out doesn't + // spin forever. Syncer.syncFolder reports failure via Result.failure (see refresh()'s + // onFailure handling below), not a thrown exception, so the stub mirrors that contract. + @Test + fun `isSyncingFolder clears even when the sync fails`() = runTest(testDispatcher) { + val syncer = mockk() + val gate = CompletableDeferred() + coEvery { syncer.syncFolder("imap:a", "Archive") } coAnswers { + gate.await() + Result.failure(IllegalStateException("boom")) + } + val vm = createViewModel(accounts = listOf(alice), messages = emptyList(), syncer = syncer) + backgroundScope.launch { vm.messages.collect {} } + + vm.isSyncingFolder.test { + assertEquals(false, awaitItem()) + + vm.selectFolder("imap:a", "Archive") + assertEquals(true, awaitItem()) + + gate.complete(Unit) + assertEquals(false, awaitItem()) + } + } + + // Issue #149: switching folders before the previous folder's sync resolves must not let that + // stale completion clear the spinner for the folder actually selected now. + @Test + fun `rapidly switching folders keeps the spinner for the folder actually selected`() = runTest(testDispatcher) { + val syncer = mockk() + val archiveGate = CompletableDeferred() + val sentGate = CompletableDeferred() + coEvery { syncer.syncFolder("imap:a", "Archive") } coAnswers { + archiveGate.await() + Result.success(0) + } + coEvery { syncer.syncFolder("imap:a", "Sent") } coAnswers { + sentGate.await() + Result.success(0) + } + val vm = createViewModel(accounts = listOf(alice), messages = emptyList(), syncer = syncer) + backgroundScope.launch { vm.messages.collect {} } + + vm.isSyncingFolder.test { + assertEquals(false, awaitItem()) + + vm.selectFolder("imap:a", "Archive") + assertEquals(true, awaitItem()) + + // Switch away before Archive's sync resolves; still syncing (now Sent's own fetch). + vm.selectFolder("imap:a", "Sent") + expectNoEvents() + assertEquals(true, vm.isSyncingFolder.value) + + // Archive's now-stale sync finishing must not clear the flag out from under Sent. + archiveGate.complete(Unit) + expectNoEvents() + assertEquals(true, vm.isSyncingFolder.value) + + // Only Sent's own completion (the folder actually selected now) clears it. + sentGate.complete(Unit) + assertEquals(false, awaitItem()) + } + } + @Test fun `folders expose the drawer account's folders in order`() = runTest(testDispatcher) { val vm = createViewModel( -- 2.47.3