fix(ui): address below-cut UI/Compose review nits (#308) #437

Merged
JMR-dev merged 2 commits from refactor-308-ui-nits into main 2026-07-08 17:24:42 +00:00
17 changed files with 310 additions and 44 deletions
@@ -16,6 +16,7 @@ import androidx.compose.ui.test.onNodeWithContentDescription
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.compose.ui.test.performScrollTo
import androidx.lifecycle.SavedStateHandle
import androidx.lifecycle.compose.collectAsStateWithLifecycle
import androidx.navigation.NavType
import androidx.navigation.compose.NavHost
@@ -98,6 +99,7 @@ class BatteryOptimizationStepTest {
BatteryOptimizationManager(context),
ContactsPermissionManager(context),
settingsRepository,
SavedStateHandle(),
)
onboarding.onAccountAdded(FIRST_ACCOUNT_ID)
@@ -110,6 +110,7 @@ class OnboardingFlowTest {
BatteryOptimizationManager(appContext),
ContactsPermissionManager(appContext),
SettingsRepository(appContext),
SavedStateHandle(),
)
composeTestRule.setContent {
LibreMailTheme(darkTheme = false, dynamicColor = false) {
@@ -63,7 +63,10 @@ class AccountSetupViewModel @Inject constructor(
fun onOutlookResult(data: Intent?) {
if (data == null) {
_state.update { it.copy(error = "Microsoft sign-in was cancelled") }
// A null result Intent is a normal user cancel: backing out of the Microsoft sign-in tab
// returns RESULT_CANCELED with no data. That is expected, not a failure, so it is a no-op —
// surfacing an error snackbar for a deliberate cancel is just noise (#308).
AppLog.d(TAG, "Outlook sign-in cancelled by the user; no-op")
return
}
viewModelScope.launch {
@@ -262,9 +262,10 @@ private fun FormattingToolbar(
onFont: (String?) -> Unit,
onInsertImage: (() -> Unit)?,
) {
val content = value.annotatedString.toRichContent()
val start = value.selection.min
val end = value.selection.max
// Parsing the field into RichTextContent and the dozen selection scans that light up the buttons is
// a per-keystroke hot path; memoize the whole derivation on the field value so a recomposition that
// doesn't change it reuses the result instead of re-parsing + re-scanning (#308).
val toolbar = remember(value) { toolbarStateOf(value) }
Row(
modifier = Modifier
.fillMaxWidth()
@@ -276,63 +277,61 @@ private fun FormattingToolbar(
FormatButton(
label = "B",
description = stringResource(R.string.format_bold),
active = RichTextEditing.isStyled(content, start, end, RichStyle.Bold),
active = toolbar.bold,
fontWeight = FontWeight.Bold,
onClick = { onToggleStyle(RichStyle.Bold) },
)
FormatButton(
label = "I",
description = stringResource(R.string.format_italic),
active = RichTextEditing.isStyled(content, start, end, RichStyle.Italic),
active = toolbar.italic,
fontStyle = FontStyle.Italic,
onClick = { onToggleStyle(RichStyle.Italic) },
)
FormatButton(
label = "U",
description = stringResource(R.string.format_underline),
active = RichTextEditing.isStyled(content, start, end, RichStyle.Underline),
active = toolbar.underline,
underline = true,
onClick = { onToggleStyle(RichStyle.Underline) },
)
FormatButton(
label = "S",
description = stringResource(R.string.format_strikethrough),
active = RichTextEditing.isStyled(content, start, end, RichStyle.Strikethrough),
active = toolbar.strikethrough,
strikethrough = true,
onClick = { onToggleStyle(RichStyle.Strikethrough) },
)
val fontColorArgb = RichTextEditing.styleAt(content, start, end, RichStyle.FontColor::class.java)?.argb
FormatButton(
label = "A",
description = stringResource(R.string.format_color),
active = fontColorArgb != null,
tint = fontColorArgb?.let { Color(it) },
active = toolbar.fontColorArgb != null,
tint = toolbar.fontColorArgb?.let { Color(it) },
onClick = onFontColor,
)
val highlightArgb = RichTextEditing.styleAt(content, start, end, RichStyle.Highlight::class.java)?.argb
FormatButton(
label = "H",
description = stringResource(R.string.format_highlight),
active = false,
swatchColor = highlightArgb?.let { Color(it) },
swatchColor = toolbar.highlightArgb?.let { Color(it) },
onClick = onHighlight,
)
FormatButton(
label = "•",
description = stringResource(R.string.format_bullet_list),
active = RichTextEditing.hasBlock(content, start, end, BlockMarker.BULLET),
active = toolbar.bullet,
onClick = { onToggleBlock(BlockMarker.BULLET) },
)
FormatButton(
label = "1.",
description = stringResource(R.string.format_numbered_list),
active = RichTextEditing.hasBlock(content, start, end, BlockMarker.ORDERED),
active = toolbar.ordered,
onClick = { onToggleBlock(BlockMarker.ORDERED) },
)
FormatButton(
label = "❝",
description = stringResource(R.string.format_quote),
active = RichTextEditing.hasBlock(content, start, end, BlockMarker.QUOTE),
active = toolbar.quote,
onClick = { onToggleBlock(BlockMarker.QUOTE) },
)
FormatButton(
@@ -347,12 +346,10 @@ private fun FormattingToolbar(
// so its click lands on the button's on-screen center. Any control inserted *before* the block
// buttons shifts them right and can push the bullet past the viewport, making that tap miss —
// so these wider controls are appended last, leaving every pre-existing button in its tested spot.
val fontCss = RichTextEditing.styleAt(content, start, end, RichStyle.FontFamily::class.java)?.css
FontPicker(selectedCss = fontCss, onSelect = onFont)
val fontSizePt = RichTextEditing.styleAt(content, start, end, RichStyle.FontSize::class.java)?.pt
FontSizePicker(selectedPt = fontSizePt, onSelect = onFontSize)
FontPicker(selectedCss = toolbar.fontCss, onSelect = onFont)
FontSizePicker(selectedPt = toolbar.fontSizePt, onSelect = onFontSize)
ParagraphAlignmentControl(
selected = RichTextEditing.alignmentAt(content, start, end),
selected = toolbar.alignment,
onSelect = onAlignment,
)
// The image button is appended last (like the other trailing controls) so it never shifts the
@@ -368,6 +365,53 @@ private fun FormattingToolbar(
}
}
/**
* The [FormattingToolbar]'s button state derived from the field's current [TextFieldValue]: the
* active/inactive toggles plus the currently-applied color/font/size/alignment the pickers read back.
* Bundled so the parse + selection scans that produce them can be computed once per field value and
* memoized, keeping the per-keystroke toolbar off the re-parse-everything path (#308).
*/
internal data class ToolbarState(
val bold: Boolean,
val italic: Boolean,
val underline: Boolean,
val strikethrough: Boolean,
val fontColorArgb: Int?,
val highlightArgb: Int?,
val bullet: Boolean,
val ordered: Boolean,
val quote: Boolean,
val fontCss: String?,
val fontSizePt: Int?,
val alignment: RichAlign?,
)
/**
* Parses [value] into [RichTextContent] once and runs every toolbar selection scan over that single
* parse, so [FormattingToolbar] derives all its button state in one pass instead of re-parsing the
* whole body and re-scanning per button on each recomposition (#308). Pure over plain
* [TextFieldValue] so it is JVM-unit-testable.
*/
internal fun toolbarStateOf(value: TextFieldValue): ToolbarState {
val content = value.annotatedString.toRichContent()
val start = value.selection.min
val end = value.selection.max
return ToolbarState(
bold = RichTextEditing.isStyled(content, start, end, RichStyle.Bold),
italic = RichTextEditing.isStyled(content, start, end, RichStyle.Italic),
underline = RichTextEditing.isStyled(content, start, end, RichStyle.Underline),
strikethrough = RichTextEditing.isStyled(content, start, end, RichStyle.Strikethrough),
fontColorArgb = RichTextEditing.styleAt(content, start, end, RichStyle.FontColor::class.java)?.argb,
highlightArgb = RichTextEditing.styleAt(content, start, end, RichStyle.Highlight::class.java)?.argb,
bullet = RichTextEditing.hasBlock(content, start, end, BlockMarker.BULLET),
ordered = RichTextEditing.hasBlock(content, start, end, BlockMarker.ORDERED),
quote = RichTextEditing.hasBlock(content, start, end, BlockMarker.QUOTE),
fontCss = RichTextEditing.styleAt(content, start, end, RichStyle.FontFamily::class.java)?.css,
fontSizePt = RichTextEditing.styleAt(content, start, end, RichStyle.FontSize::class.java)?.pt,
alignment = RichTextEditing.alignmentAt(content, start, end),
)
}
@Composable
private fun FormatButton(
label: String,
@@ -16,6 +16,7 @@ import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.input.pointer.pointerInput
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.semantics.clearAndSetSemantics
import androidx.core.content.ContextCompat
import androidx.fragment.app.FragmentActivity
import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel
@@ -70,7 +71,21 @@ fun AppLockGateHost(viewModel: AppLockViewModel = hiltViewModel(), content: @Com
LaunchedEffect(uiState) { if (uiState is AppLockUiState.Unlocked) hasEverUnlocked = true }
Box(modifier = Modifier.fillMaxSize()) {
if (hasEverUnlocked) content()
// While the app is covered (Checking/Locked) the opaque LockCover hides the content visually and
// blocks its input, but the content stays in the composition (so its state survives a re-lock).
// Clear the covered subtree out of the semantics/accessibility tree so an accessibility service
// (e.g. TalkBack) can't traverse the occluded mailbox/compose nodes behind the cover (#308). When
// Unlocked the modifier is dropped, restoring the content's full semantics.
val contentCovered = uiState !is AppLockUiState.Unlocked
Box(
modifier = if (contentCovered) {
Modifier.fillMaxSize().clearAndSetSemantics { }
} else {
Modifier.fillMaxSize()
},
) {
if (hasEverUnlocked) content()
}
when (val state = uiState) {
AppLockUiState.Unlocked -> Unit
@@ -2,6 +2,7 @@
package org.libremail.ui.onboarding
import android.content.Intent
import androidx.lifecycle.SavedStateHandle
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import dagger.hilt.android.lifecycle.HiltViewModel
@@ -13,6 +14,8 @@ import org.libremail.contacts.ContactsPermissionManager
import org.libremail.data.settings.SettingsRepository
import org.libremail.push.BatteryOptimizationManager
import org.libremail.push.BatteryPromptDecision
import org.libremail.reporting.AppLog
import org.libremail.reporting.accountLogRef
import javax.inject.Inject
/**
@@ -28,11 +31,18 @@ class OnboardingViewModel @Inject constructor(
private val batteryOptimizationManager: BatteryOptimizationManager,
private val contactsPermissionManager: ContactsPermissionManager,
private val settingsRepository: SettingsRepository,
private val savedStateHandle: SavedStateHandle,
) : ViewModel() {
/** The id of the first account added this session, or null if none has been added yet. */
var firstAddedAccountId: String? = null
private set
/**
* The id of the first account added this session, or null if none has been added yet. Held in
* [SavedStateHandle] rather than a plain field so it survives process death: this ViewModel is
* scoped to the onboarding nav-graph back-stack entry, whose saved state is restored after a
* process kill. Without that, a kill mid-onboarding — e.g. the excursion to system battery
* settings on the opt-in step — would drop the id and finish onto the unfiltered mailbox instead
* of the first account's inbox, defeating #30.
*/
val firstAddedAccountId: String? get() = savedStateHandle[KEY_FIRST_ACCOUNT_ID]
private val _batteryPromptNeeded = MutableStateFlow<Boolean?>(null)
@@ -82,7 +92,9 @@ class OnboardingViewModel @Inject constructor(
/** Records a freshly added account. Only the first one sticks — later adds don't overwrite it. */
fun onAccountAdded(accountId: String) {
if (firstAddedAccountId == null) {
firstAddedAccountId = accountId
savedStateHandle[KEY_FIRST_ACCOUNT_ID] = accountId
// PII-free: the id embeds the email, so reference it only via its non-reversible log ref.
AppLog.i(TAG, "onboarding recorded first account ${accountLogRef(accountId)}")
}
}
@@ -128,4 +140,11 @@ class OnboardingViewModel @Inject constructor(
fun markContactsPromptHandled() {
viewModelScope.launch { settingsRepository.setContactsPromptHandled(true) }
}
private companion object {
const val TAG = "OnboardingVM"
/** SavedStateHandle key for [firstAddedAccountId], persisted across process death (#308). */
const val KEY_FIRST_ACCOUNT_ID = "onboarding_first_added_account_id"
}
}
@@ -85,6 +85,7 @@ fun ReaderScreen(
val noAppMessage = stringResource(R.string.attachment_no_app)
val downloadFailedTemplate = stringResource(R.string.attachment_download_failed)
val replyFailedMessage = stringResource(R.string.reader_reply_failed)
val starFailedMessage = stringResource(R.string.reader_star_failed)
LaunchedEffect(state.deleted) {
if (state.deleted) onBack()
@@ -105,6 +106,8 @@ fun ReaderScreen(
is ReaderEvent.ComposeFailed ->
snackbarHostState.showSnackbar(event.message ?: replyFailedMessage)
ReaderEvent.StarFailed -> snackbarHostState.showSnackbar(starFailedMessage)
}
}
}
@@ -50,6 +50,9 @@ sealed interface ReaderEvent {
/** Building the reply/forward draft failed; the screen surfaces [message] (or a generic fallback). */
data class ComposeFailed(val message: String?) : ReaderEvent
/** Persisting a star toggle failed; the optimistic flip was rolled back and the screen notifies. */
data object StarFailed : ReaderEvent
}
@HiltViewModel
@@ -140,7 +143,19 @@ class ReaderViewModel @Inject constructor(
val message = _state.value.message ?: return
val starred = !message.isStarred
_state.update { it.copy(message = message.copy(isStarred = starred)) }
viewModelScope.launch { repository.setStarred(messageId, starred) }
viewModelScope.launch {
repository.setStarred(messageId, starred).onFailure { e ->
// The optimistic flip already updated the UI; the persist failed, so reconcile by rolling
// it back (the star would otherwise stay stuck in a state the store never accepted) and
// notify the screen. Revert to the pre-toggle value (!starred) of the current message.
AppLog.w(READER_TAG, "toggleStar persist failed; rolling back optimistic star", e)
_state.update { current ->
val shown = current.message ?: return@update current
current.copy(message = shown.copy(isStarred = !starred))
}
_events.send(ReaderEvent.StarFailed)
}
}
}
fun loadRemoteImages() = _state.update { it.copy(loadRemoteImages = true) }
@@ -2,9 +2,12 @@
package org.libremail.ui.settings
import android.content.Intent
import androidx.annotation.VisibleForTesting
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
@@ -12,6 +15,7 @@ import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import org.libremail.R
import org.libremail.contacts.ContactsPermissionManager
import org.libremail.data.security.AppLockManager
@@ -62,6 +66,12 @@ class SettingsViewModel @Inject constructor(
/** Whether this app is exempt from battery optimization ("Unrestricted"). */
val batteryUnrestricted: StateFlow<Boolean> = _batteryUnrestricted.asStateFlow()
// Blocking Keystore/DataStore work on the app-lock disable path runs here — off the main
// dispatcher — mirroring AppLockViewModel's threading policy so the reseal never runs Keystore
// crypto on the main thread (#308). Injectable so unit tests can pin it to their scheduler.
@VisibleForTesting
internal var defaultDispatcher: CoroutineDispatcher = Dispatchers.Default
fun toggleAdvanced() = _advancedExpanded.update { !it }
/** Persist the account order the user produced by dragging in Settings (issue #164). */
@@ -110,13 +120,19 @@ class SettingsViewModel @Inject constructor(
// Reseal under the master key whenever an auth seal actually exists — gate on the seal, not
// the encryptCache setting (a separate store that can already be off while the on-disk DB is
// still auth-sealed). Do it BEFORE dropping the gate, or the next launch can't open the
// cache. If it fails, keep app-lock on rather than strand the passphrase.
if (databaseKeyStore.hasAuthSealedPassphrase()) {
if (runCatching { databaseKeyStore.sealWithMaster() }.isFailure) {
_appLockMessage.value = R.string.app_lock_disable_failed
return@update
// cache. If it fails, keep app-lock on rather than strand the passphrase. The Keystore/
// DataStore reseal is blocking crypto, so it runs off the main thread (#308).
val resealed = withContext(defaultDispatcher) {
if (databaseKeyStore.hasAuthSealedPassphrase()) {
runCatching { databaseKeyStore.sealWithMaster() }.isSuccess
} else {
true
}
}
if (!resealed) {
_appLockMessage.value = R.string.app_lock_disable_failed
return@update
}
settingsRepository.setAppLock(false)
}
}
+1
View File
@@ -151,6 +151,7 @@
<!-- Reader -->
<string name="reader_reply">Reply</string>
<string name="reader_reply_failed">Couldn\'t start the reply</string>
<string name="reader_star_failed">Couldn\'t update star</string>
<string name="attachments_title">Attachments</string>
<string name="attachment_downloaded">Downloaded</string>
<string name="message_available_offline">Available offline</string>
@@ -124,13 +124,18 @@ class AccountSetupViewModelTest {
}
@Test
fun `a cancelled sign-in (null result) surfaces a cancellation message`() {
fun `a cancelled sign-in (null result) is a no-op, not an error`() {
val vm = viewModel()
vm.onOutlookResult(null)
assertEquals("Microsoft sign-in was cancelled", vm.state.value.error)
// #308: a normal cancel (backing out of the sign-in tab → RESULT_CANCELED, null data) must not
// raise an error snackbar; it leaves the screen untouched (idle, no error) and only breadcrumbs.
assertNull(vm.state.value.error)
assertEquals(SetupStatus.IDLE, vm.state.value.status)
val entry = logBuffer.snapshot().single()
assertEquals('D', entry.level)
assertTrue(entry.message.contains("cancelled"), entry.message)
}
@Test
@@ -263,7 +268,9 @@ class AccountSetupViewModelTest {
@Test
fun `consumeError clears a surfaced error`() {
val vm = viewModel()
vm.onOutlookResult(null)
// Surface a real error first (a cancel is now a no-op), then confirm consumeError clears it.
vm.onOutlookLaunchFailed(IllegalStateException("appauth exploded"))
assertEquals("appauth exploded", vm.state.value.error)
vm.consumeError()
@@ -348,6 +348,37 @@ class RichTextEditorTest {
assertTrue(isBulletActive(bothLines.copy(selection = fullText)))
}
@Test
fun `toolbarStateOf derives every button's state from the field value in a single pass`() {
// A run carrying bold + color + size, centered, on a bullet line. applyBlock inserts "• " and
// lands the selection back on the styled text, so every scan sees the same run the user is on.
var value = field("hello", TextRange(0, 5))
value = applyStyle(value, RichStyle.Bold, linkColor)
value = applyStyle(value, RichStyle.FontColor(0xFF112233.toInt()), linkColor)
value = applyStyle(value, RichStyle.FontSize(18), linkColor)
value = applyAlignment(value, RichAlign.CENTER, linkColor, noFont)
value = applyBlock(value, BlockMarker.BULLET, linkColor, noFont)
val state = toolbarStateOf(value)
assertTrue(state.bold)
assertFalse(state.italic)
assertFalse(state.underline)
assertFalse(state.strikethrough)
assertEquals(0xFF112233.toInt(), state.fontColorArgb)
assertEquals(null, state.highlightArgb)
assertEquals(18, state.fontSizePt)
assertEquals(null, state.fontCss)
assertTrue(state.bullet)
assertFalse(state.ordered)
assertFalse(state.quote)
assertEquals(RichAlign.CENTER, state.alignment)
// Each field equals reading the same predicate directly off the value: the memoization the
// toolbar wraps this in (remember(value)) is behaviour-preserving, just computed once (#308).
assertEquals(isBoldActive(value), state.bold)
assertEquals(isBulletActive(value), state.bullet)
}
/** Mirrors exactly what FormattingToolbar reads to decide a style button's active/inactive tint. */
private fun isBoldActive(value: TextFieldValue): Boolean = RichTextEditing.isStyled(
value.annotatedString.toRichContent(),
@@ -4,6 +4,7 @@ package org.libremail.ui.lock
import android.content.Context
import androidx.compose.material3.Text
import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.runtime.DisposableEffect
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.junit4.v2.createComposeRule
import androidx.compose.ui.test.onNodeWithText
@@ -24,6 +25,7 @@ import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
import org.robolectric.annotation.Config
import org.robolectric.annotation.GraphicsMode
import kotlin.test.assertEquals
/**
* Robolectric JVM Compose test (#384, umbrella #373) for [AppLockGateHost], the gate that wraps the
@@ -109,26 +111,50 @@ class AppLockGateHostJvmTest {
}
@Test
fun reLockAfterUnlock_drawsLockScreenButKeepsContentComposed() {
fun reLockAfterUnlock_keepsContentComposedButGatesItOutOfTheAccessibilityTree() {
val state = MutableStateFlow<AppLockUiState>(AppLockUiState.Unlocked)
val vm = mockk<AppLockViewModel>(relaxed = true)
every { vm.uiState } returns state
// Track the content's composition lifecycle directly: entered once and never disposed proves the
// content composable is RETAINED across a re-lock (its nav/scroll/draft state survives) rather
// than torn down — independently of whether its semantics are currently in the tree.
var entered = 0
var disposed = 0
composeTestRule.setContent {
CompositionLocalProvider(LocalLifecycleOwner provides resumedOwner) {
LibreMailTheme(darkTheme = false, dynamicColor = false) {
AppLockGateHost(viewModel = vm) { Text(CONTENT) }
AppLockGateHost(viewModel = vm) {
DisposableEffect(Unit) {
entered++
onDispose { disposed++ }
}
Text(CONTENT)
}
}
}
}
composeTestRule.onNodeWithText(CONTENT).assertIsDisplayed()
assertEquals(1, entered)
// Re-lock: once ever unlocked, the content stays composed (its nav/scroll/draft state survives)
// and the lock screen is drawn OVER it rather than replacing it.
// Re-lock: the lock screen is drawn OVER the content. The content stays composed (entered==1,
// never disposed), but #308 clears it out of the semantics/accessibility tree so TalkBack can't
// traverse the occluded mailbox nodes behind the opaque cover — assertDoesNotExist confirms the
// content's text is no longer reachable in the (a11y-facing) semantics tree.
state.value = AppLockUiState.Locked()
composeTestRule.waitForIdle()
composeTestRule.onNodeWithText(string(R.string.app_lock_title)).assertIsDisplayed()
composeTestRule.onNodeWithText(CONTENT).assertExists()
composeTestRule.onNodeWithText(CONTENT).assertDoesNotExist()
assertEquals(1, entered)
assertEquals(0, disposed)
// Unlocking again restores the content's semantics (same retained composition — still one entry).
state.value = AppLockUiState.Unlocked
composeTestRule.waitForIdle()
composeTestRule.onNodeWithText(CONTENT).assertIsDisplayed()
assertEquals(1, entered)
assertEquals(0, disposed)
}
private companion object {
@@ -1,12 +1,16 @@
// SPDX-License-Identifier: GPL-3.0-or-later
package org.libremail.ui.onboarding
import android.util.Log
import androidx.lifecycle.SavedStateHandle
import io.mockk.Runs
import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.every
import io.mockk.just
import io.mockk.mockk
import io.mockk.mockkStatic
import io.mockk.unmockkAll
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.UnconfinedTestDispatcher
@@ -30,10 +34,19 @@ class OnboardingViewModelTest {
private val testDispatcher = UnconfinedTestDispatcher()
@Before
fun setUp() = Dispatchers.setMain(testDispatcher)
fun setUp() {
Dispatchers.setMain(testDispatcher)
// onAccountAdded now breadcrumbs the first account via AppLog (#308); android.util.Log is a
// no-op stub that throws "not mocked" under plain JVM tests, so stub it so the call can't crash.
mockkStatic(Log::class)
every { Log.i(any(), any()) } returns 0
}
@After
fun tearDown() = Dispatchers.resetMain()
fun tearDown() {
Dispatchers.resetMain()
unmockkAll()
}
private fun batteryManager(supported: Boolean = true, unrestricted: Boolean = false) =
mockk<BatteryOptimizationManager> {
@@ -55,7 +68,8 @@ class OnboardingViewModelTest {
battery: BatteryOptimizationManager = batteryManager(),
contacts: ContactsPermissionManager = contactsManager(),
settings: SettingsRepository = settingsRepository(),
) = OnboardingViewModel(battery, contacts, settings)
savedStateHandle: SavedStateHandle = SavedStateHandle(),
) = OnboardingViewModel(battery, contacts, settings, savedStateHandle)
@Test
fun `battery prompt is needed when not unrestricted and not handled`() = runTest(testDispatcher) {
@@ -123,6 +137,21 @@ class OnboardingViewModelTest {
assertEquals("imap:first@example.com", vm.firstAddedAccountId)
}
@Test
fun `the first added account id survives a process-death recreation via SavedStateHandle`() =
runTest(testDispatcher) {
// #308: onboarding can be process-killed mid-flow (e.g. the excursion to system battery
// settings). The first-account id lives in SavedStateHandle, so a recreated ViewModel over
// the restored state must still finish onto that account's inbox rather than the unified one.
val handle = SavedStateHandle()
viewModel(savedStateHandle = handle).onAccountAdded("imap:first@example.com")
// Simulate restoration after a kill: a brand-new handle seeded from the saved state, then a
// fresh ViewModel over it (a plain field would have been lost — this is exactly #30's guard).
val restored = SavedStateHandle(handle.keys().associateWith { handle.get<Any?>(it) })
assertEquals("imap:first@example.com", viewModel(savedStateHandle = restored).firstAddedAccountId)
}
@Test
fun `marking the battery prompt handled persists the flag`() = runTest(testDispatcher) {
val repo = settingsRepository()
@@ -55,6 +55,9 @@ class ReaderViewModelActionsTest {
every { Log.d(any(), any()) } returns 0
every { Log.i(any(), any()) } returns 0
every { Log.w(any<String>(), any<String>()) } returns 0
// toggleStar's rollback path logs via AppLog.w(tag, msg, throwable) -> Log.w(String, String,
// Throwable); stub that overload too so the failing-write test's coroutine doesn't crash on it.
every { Log.w(any<String>(), any<String>(), any()) } returns 0
every { Log.e(any(), any(), any()) } returns 0
}
@@ -78,6 +81,7 @@ class ReaderViewModelActionsTest {
coEvery { repo.openMessage(messageId) } returns Result.success(message)
every { repo.observeAttachments(messageId) } returns flowOf(listOf(attachment(0)))
coEvery { repo.downloadedAttachmentParts(messageId) } returns emptySet()
coEvery { repo.setStarred(any(), any()) } returns Result.success(Unit)
return repo
}
@@ -181,6 +185,28 @@ class ReaderViewModelActionsTest {
coVerify { repo.setStarred(messageId, true) }
}
@Test
fun `toggleStar rolls the optimistic star back and emits StarFailed when the write fails`() = runTest(dispatcher) {
val repo = loadedRepo()
coEvery { repo.setStarred(messageId, true) } returns Result.failure(RuntimeException("db locked"))
val vm = viewModel(repo)
advanceUntilIdle()
assertFalse(vm.state.value.message!!.isStarred)
vm.events.test {
vm.toggleStar()
// The optimistic flip lands immediately, before the (failing) persist runs.
assertTrue(vm.state.value.message!!.isStarred)
advanceUntilIdle()
assertEquals(ReaderEvent.StarFailed, awaitItem())
// The failed write is reconciled: the star is rolled back to its pre-toggle value so the
// UI never stays stuck in a state the store rejected.
assertFalse(vm.state.value.message!!.isStarred)
cancelAndIgnoreRemainingEvents()
}
}
@Test
fun `toggleStar does nothing before the message has loaded`() = runTest(dispatcher) {
val repo = mockk<MailRepository>(relaxed = true)
@@ -9,6 +9,7 @@ import io.mockk.mockk
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.test.StandardTestDispatcher
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import kotlinx.coroutines.test.advanceUntilIdle
import kotlinx.coroutines.test.resetMain
@@ -107,6 +108,26 @@ class SettingsViewModelTest {
coVerify(exactly = 0) { settingsRepository.setAppLock(false) }
}
@Test
fun `the disable-path reseal is dispatched off the main thread, not run inline on main`() = runTest(dispatcher) {
val databaseKeyStore = mockk<DatabaseKeyStore>(relaxed = true)
coEvery { databaseKeyStore.hasAuthSealedPassphrase() } returns true
val settingsRepository = mockk<SettingsRepository>(relaxed = true)
val vm = viewModel(databaseKeyStore = databaseKeyStore, settingsRepository = settingsRepository)
// Swap the pinned dispatcher for a queue-and-drain one (sharing the scheduler) so we can see
// that the blocking Keystore reseal is DISPATCHED off-main rather than run inline on Main's
// unconfined pass — the point of #308: no Keystore crypto on the main dispatcher.
vm.defaultDispatcher = StandardTestDispatcher(testScheduler)
vm.setAppLock(false)
// Dispatched to defaultDispatcher, so it has not run on the current (unconfined Main) pass...
coVerify(exactly = 0) { databaseKeyStore.sealWithMaster() }
advanceUntilIdle()
// ...only once that off-main dispatcher is drained.
coVerify(exactly = 1) { databaseKeyStore.sealWithMaster() }
}
@Test
fun `disabling app-lock with no auth seal just drops the gate`() = runTest(dispatcher) {
val databaseKeyStore = mockk<DatabaseKeyStore>(relaxed = true)
@@ -139,6 +160,10 @@ class SettingsViewModelTest {
batteryOptimizationManager = mockk(relaxed = true),
contactsPermissionManager = mockk(relaxed = true),
syncScheduler = mockk(relaxed = true),
)
).apply {
// Pin the off-main reseal dispatcher (#308) to the test scheduler so the disable path's
// withContext(defaultDispatcher) work is advanced deterministically by advanceUntilIdle().
defaultDispatcher = dispatcher
}
}
}
+3
View File
@@ -79,6 +79,9 @@ style:
- '**/data/repository/AccountRepositoryImplTest.kt'
- '**/ui/reader/ReaderViewModelTest.kt'
- '**/ui/reader/ReaderViewModelActionsTest.kt'
# First-account onboarding breadcrumb (issue #308): onAccountAdded logs via AppLog, so this
# suite mockkStatic(Log) so the call doesn't crash on the throwing JVM stub.
- '**/ui/onboarding/OnboardingViewModelTest.kt'
MagicNumber:
# dp / sp / duration literals are idiomatic inline in Compose.
ignoreAnnotated: ['Composable']