fix: show loading overlay immediately for remote config sub-screens (#5694)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
James Rich
2026-06-01 05:32:52 -07:00
committed by GitHub
parent 63ff12dac3
commit baa66e6877
4 changed files with 144 additions and 8 deletions

View File

@@ -127,8 +127,7 @@ fun EntryProviderScope<NavKey>.settingsGraph(backStack: NavBackStack<NavKey>) {
}
ConfigRoute.entries.forEach { routeInfo ->
configComposable(routeInfo.route::class, backStack) { viewModel ->
LaunchedEffect(Unit) { viewModel.setResponseStateLoading(routeInfo) }
configComposable(routeInfo.route::class, backStack, routeInfo) { viewModel ->
when (routeInfo) {
ConfigRoute.USER ->
UserConfigScreen(viewModel, onBack = dropUnlessResumed { backStack.removeLastOrNull() })
@@ -164,8 +163,7 @@ fun EntryProviderScope<NavKey>.settingsGraph(backStack: NavBackStack<NavKey>) {
}
ModuleRoute.entries.forEach { routeInfo ->
configComposable(routeInfo.route::class, backStack) { viewModel ->
LaunchedEffect(Unit) { viewModel.setResponseStateLoading(routeInfo) }
configComposable(routeInfo.route::class, backStack, routeInfo) { viewModel ->
when (routeInfo) {
ModuleRoute.MQTT ->
MQTTConfigScreen(viewModel, onBack = dropUnlessResumed { backStack.removeLastOrNull() })
@@ -266,7 +264,15 @@ expect fun SettingsMainScreen(
fun <R : Route> EntryProviderScope<NavKey>.configComposable(
route: KClass<R>,
backStack: NavBackStack<NavKey>,
routeInfo: Enum<*>,
content: @Composable (RadioConfigViewModel) -> Unit,
) {
addEntryProvider(route) { content(getRadioConfigViewModel(backStack)) }
addEntryProvider(route) {
val viewModel = getRadioConfigViewModel(backStack)
// Set loading state before content reads the StateFlow, ensuring
// LoadingOverlay is visible from the very first composition frame.
remember { viewModel.ensureLoadingForRemote().let { true } }
LaunchedEffect(Unit) { viewModel.setResponseStateLoading(routeInfo) }
content(viewModel)
}
}

View File

@@ -476,6 +476,17 @@ open class RadioConfigViewModel(
_radioConfigState.update { it.copy(responseState = ResponseState.Empty) }
}
/**
* Sets the initial loading state for remote config sub-screens. Must be called before the first
* `collectAsStateWithLifecycle` read so the LoadingOverlay is visible from the very first composition frame.
*/
fun ensureLoadingForRemote() {
val state = _radioConfigState.value
if (!state.isLocal && state.responseState is ResponseState.Empty) {
_radioConfigState.update { it.copy(responseState = ResponseState.Loading()) }
}
}
fun setResponseStateLoading(route: Enum<*>) {
val destNum = destNum ?: destNode.value?.num ?: return

View File

@@ -71,7 +71,21 @@ private val CODING_RATE_RANGE = 5..8
fun LoRaConfigScreen(viewModel: RadioConfigViewModel, onBack: () -> Unit) {
val state by viewModel.radioConfigState.collectAsStateWithLifecycle()
val loraConfig = state.radioConfig.lora ?: Config.LoRaConfig()
val primarySettings = state.channelList.getOrNull(0) ?: return
val primarySettings = state.channelList.getOrNull(0)
if (primarySettings == null) {
RadioConfigScreenList(
title = stringResource(Res.string.lora),
onBack = onBack,
configState = rememberConfigState(initialValue = loraConfig),
enabled = false,
responseState = state.responseState,
onDismissPacketResponse = viewModel::clearPacketResponse,
onSave = {},
) {}
return
}
val formState = rememberConfigState(initialValue = loraConfig)
val primaryChannel = remember(formState.value) { Channel(primarySettings, formState.value) }

View File

@@ -48,6 +48,7 @@ import org.meshtastic.core.domain.usecase.settings.RadioResponseResult
import org.meshtastic.core.domain.usecase.settings.ToggleAnalyticsUseCase
import org.meshtastic.core.domain.usecase.settings.ToggleHomoglyphEncodingUseCase
import org.meshtastic.core.model.MqttProbeStatus
import org.meshtastic.core.model.MyNodeInfo
import org.meshtastic.core.model.Node
import org.meshtastic.core.repository.AnalyticsPrefs
import org.meshtastic.core.repository.FileService
@@ -75,6 +76,7 @@ import kotlin.test.AfterTest
import kotlin.test.BeforeTest
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertTrue
@OptIn(ExperimentalCoroutinesApi::class)
@@ -138,8 +140,8 @@ class RadioConfigViewModelTest {
Dispatchers.resetMain()
}
private fun createViewModel() = RadioConfigViewModel(
destNum = null,
private fun createViewModel(destNum: Int? = null) = RadioConfigViewModel(
destNum = destNum,
radioConfigRepository = radioConfigRepository,
packetRepository = packetRepository,
serviceRepository = serviceRepository,
@@ -610,4 +612,107 @@ class RadioConfigViewModelTest {
// actually sendError updates the state? No, sendError sends an event.
// But the requestIds gets cleared.
}
@Test
fun `ensureLoadingForRemote sets loading state for remote nodes`() = runTest {
val localNode = Node(num = 100, user = User(id = "!100"))
val remoteNode = Node(num = 456, user = User(id = "!456"))
nodeRepository.setNodes(listOf(localNode, remoteNode))
nodeRepository.setMyNodeInfo(
MyNodeInfo(
myNodeNum = 100,
hasGPS = false,
model = null,
firmwareVersion = null,
couldUpdate = false,
shouldUpdate = false,
currentPacketId = 0,
messageTimeoutMsec = 0,
minAppVersion = 0,
maxChannels = 8,
hasWifi = false,
channelUtilization = 0f,
airUtilTx = 0f,
deviceId = null,
),
)
val remoteVm = createViewModel(destNum = 456)
// Remote VM starts with Empty responseState
assertEquals(ResponseState.Empty, remoteVm.radioConfigState.value.responseState)
assertFalse(remoteVm.radioConfigState.value.isLocal)
// ensureLoadingForRemote should transition to Loading
remoteVm.ensureLoadingForRemote()
assertTrue(remoteVm.radioConfigState.value.responseState is ResponseState.Loading)
}
@Test
fun `ensureLoadingForRemote is no-op for local nodes`() = runTest {
val localNode = Node(num = 100, user = User(id = "!100"))
nodeRepository.setNodes(listOf(localNode))
nodeRepository.setMyNodeInfo(
MyNodeInfo(
myNodeNum = 100,
hasGPS = false,
model = null,
firmwareVersion = null,
couldUpdate = false,
shouldUpdate = false,
currentPacketId = 0,
messageTimeoutMsec = 0,
minAppVersion = 0,
maxChannels = 8,
hasWifi = false,
channelUtilization = 0f,
airUtilTx = 0f,
deviceId = null,
),
)
val localVm = createViewModel(destNum = 100)
// Local VM should have isLocal = true
assertTrue(localVm.radioConfigState.value.isLocal)
// ensureLoadingForRemote should NOT change responseState
localVm.ensureLoadingForRemote()
assertEquals(ResponseState.Empty, localVm.radioConfigState.value.responseState)
}
@Test
fun `ensureLoadingForRemote is no-op when already loading`() = runTest {
val localNode = Node(num = 100, user = User(id = "!100"))
val remoteNode = Node(num = 456, user = User(id = "!456"))
nodeRepository.setNodes(listOf(localNode, remoteNode))
nodeRepository.setMyNodeInfo(
MyNodeInfo(
myNodeNum = 100,
hasGPS = false,
model = null,
firmwareVersion = null,
couldUpdate = false,
shouldUpdate = false,
currentPacketId = 0,
messageTimeoutMsec = 0,
minAppVersion = 0,
maxChannels = 8,
hasWifi = false,
channelUtilization = 0f,
airUtilTx = 0f,
deviceId = null,
),
)
val remoteVm = createViewModel(destNum = 456)
// Set loading first
remoteVm.ensureLoadingForRemote()
assertTrue(remoteVm.radioConfigState.value.responseState is ResponseState.Loading)
// Calling again should still be Loading (no-op, not a new instance)
remoteVm.ensureLoadingForRemote()
assertTrue(remoteVm.radioConfigState.value.responseState is ResponseState.Loading)
}
}