Skip to content

cleanup #4401

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
frett merged 4 commits into from
Apr 23, 2026
Merged

cleanup #4401

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d63a620
Replace custom padding extension with standard Compose padding import
frett Apr 20, 2026
b555e55
Use withContext(NonCancellable) instead of launch(NonCancellable) for…
frett Apr 20, 2026
e130862
s/State/UiState
frett Feb 26, 2026
2acab6f
s/Event/UiEvent
frett Feb 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/src/main/kotlin/org/cru/godtools/ui/drawer/DrawerMenuLayout.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import android.content.Context
import android.content.Intent
import androidx.activity.compose.BackHandler
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.rememberScrollState
import androidx.compose.foundation.verticalScroll
import androidx.compose.material.icons.Icons
Expand Down Expand Up @@ -53,7 +54,6 @@ import com.google.android.play.core.ktx.launchReview
import com.google.android.play.core.ktx.requestReview
import com.google.android.play.core.review.ReviewManagerFactory
import kotlinx.coroutines.launch
import org.ccci.gto.android.common.androidx.compose.foundation.layout.padding
import org.ccci.gto.android.common.androidx.compose.material3.ui.navigationdrawer.NavigationDrawerHeadline
import org.cru.godtools.R
import org.cru.godtools.analytics.model.AnalyticsScreenEvent
Expand Down
3 changes: 2 additions & 1 deletion app/src/main/kotlin/org/cru/godtools/ui/drawer/DrawerMenuPresenter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import javax.inject.Inject
import javax.inject.Singleton
import kotlinx.coroutines.NonCancellable
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import org.cru.godtools.account.GodToolsAccountManager
import org.cru.godtools.ui.drawer.DrawerMenuScreen.Event
import org.cru.godtools.ui.drawer.DrawerMenuScreen.State
Expand All @@ -28,7 +29,7 @@ class DrawerMenuPresenter @Inject constructor(private val accountManager: GodToo
eventSink = {
when (it) {
Event.Logout -> scope.launch {
launch(NonCancellable) { accountManager.logout() }
launch { withContext(NonCancellable) { accountManager.logout() } }
drawerState.close()
}

Expand Down
7 changes: 4 additions & 3 deletions app/src/main/kotlin/org/cru/godtools/ui/tooldetails/ToolDetailsAbout.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,12 @@ import org.cru.godtools.R
import org.cru.godtools.analytics.model.ExitLinkActionEvent
import org.cru.godtools.base.ui.compose.LocalEventBus
import org.cru.godtools.model.getDescription
import org.cru.godtools.ui.tooldetails.ToolDetailsScreen.UiState

internal const val TEST_TAG_LANGUAGES_AVAILABLE = "languages_available"

@Composable
internal fun ToolDetailsAbout(state: ToolDetailsScreen.State, modifier: Modifier = Modifier) {
internal fun ToolDetailsAbout(state: UiState, modifier: Modifier = Modifier) {
val tool by rememberUpdatedState(state.tool)
val translation by rememberUpdatedState(state.translation)

Expand Down Expand Up @@ -123,7 +124,7 @@ private enum class ToolDetailsAboutAccordionSection { OUTLINE, BIBLE_REFERENCES,
@Composable
@VisibleForTesting
internal fun ToolDetailsLanguages(
state: ToolDetailsScreen.State,
state: UiState,
expanded: Boolean,
onToggleLanguages: () -> Unit,
modifier: Modifier = Modifier,
Expand Down Expand Up @@ -188,7 +189,7 @@ private fun ToolDetailsAboutAccordionSection(
}

@Composable
private fun ToolDetailsLinkifiedText(text: String?, state: ToolDetailsScreen.State, modifier: Modifier = Modifier) {
private fun ToolDetailsLinkifiedText(text: String?, state: UiState, modifier: Modifier = Modifier) {
val eventBus = LocalEventBus.current
val uriHandler = LocalUriHandler.current
val tool by rememberUpdatedState(state.tool?.code)
Expand Down
24 changes: 12 additions & 12 deletions app/src/main/kotlin/org/cru/godtools/ui/tooldetails/ToolDetailsLayout.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ import org.cru.godtools.base.ui.theme.GodToolsTheme
import org.cru.godtools.base.ui.youtubeplayer.YouTubePlayer
import org.cru.godtools.model.getName
import org.cru.godtools.ui.drawer.DrawerMenuLayout
import org.cru.godtools.ui.tooldetails.ToolDetailsScreen.Event
import org.cru.godtools.ui.tooldetails.ToolDetailsScreen.Page
import org.cru.godtools.ui.tooldetails.ToolDetailsScreen.State
import org.cru.godtools.ui.tooldetails.ToolDetailsScreen.UiEvent
import org.cru.godtools.ui.tooldetails.ToolDetailsScreen.UiState
import org.cru.godtools.ui.tooldetails.analytics.model.ToolDetailsScreenEvent
import org.cru.godtools.ui.tools.AvailableInLanguage
import org.cru.godtools.ui.tools.DownloadProgressIndicator
Expand All @@ -87,7 +87,7 @@ internal const val TEST_TAG_ACTION_TOOL_TRAINING = "action_tool_training"
@Composable
@CircuitInject(ToolDetailsScreen::class, SingletonComponent::class)
@OptIn(ExperimentalMaterial3Api::class)
fun ToolDetailsLayout(state: State, modifier: Modifier = Modifier) = DrawerMenuLayout(state.drawerState, modifier) {
fun ToolDetailsLayout(state: UiState, modifier: Modifier = Modifier) = DrawerMenuLayout(state.drawerState, modifier) {
val hasShortcut by rememberUpdatedState(state.hasShortcut)
val eventSink by rememberUpdatedState(state.eventSink)

Expand All @@ -97,7 +97,7 @@ fun ToolDetailsLayout(state: State, modifier: Modifier = Modifier) = DrawerMenuL
title = {},
navigationIcon = {
IconButton(
onClick = { eventSink(Event.NavigateUp) },
onClick = { eventSink(UiEvent.NavigateUp) },
modifier = Modifier.testTag(TEST_TAG_ACTION_NAVIGATE_UP),
) {
Icon(Icons.AutoMirrored.Filled.ArrowBack, null)
Expand All @@ -116,7 +116,7 @@ fun ToolDetailsLayout(state: State, modifier: Modifier = Modifier) = DrawerMenuL
DropdownMenu(expanded = showOverflow, onDismissRequest = { showOverflow = false }) {
DropdownMenuItem(
text = { Text(stringResource(R.string.menu_add_to_home)) },
onClick = { eventSink(Event.PinShortcut) },
onClick = { eventSink(UiEvent.PinShortcut) },
modifier = Modifier.testTag(TEST_TAG_ACTION_PIN_SHORTCUT),
)
}
Expand All @@ -132,7 +132,7 @@ fun ToolDetailsLayout(state: State, modifier: Modifier = Modifier) = DrawerMenuL

@Composable
@OptIn(ExperimentalFoundationApi::class)
private fun ToolDetailsContent(state: State, modifier: Modifier = Modifier) {
private fun ToolDetailsContent(state: UiState, modifier: Modifier = Modifier) {
val tool by rememberUpdatedState(state.tool)
val translation by rememberUpdatedState(state.translation)
val secondLanguage by rememberUpdatedState(state.secondLanguage)
Expand Down Expand Up @@ -231,7 +231,7 @@ private fun ToolDetailsContent(state: State, modifier: Modifier = Modifier) {
}

@Composable
private fun ToolDetailsBanner(state: State, modifier: Modifier = Modifier) {
private fun ToolDetailsBanner(state: UiState, modifier: Modifier = Modifier) {
val youtubeVideo = state.tool?.detailsBannerYoutubeVideoId

when {
Expand Down Expand Up @@ -268,18 +268,18 @@ private fun ToolDetailsBanner(state: State, modifier: Modifier = Modifier) {

@Composable
@VisibleForTesting
internal fun ToolDetailsActions(state: State, modifier: Modifier = Modifier) = Column(modifier = modifier) {
internal fun ToolDetailsActions(state: UiState, modifier: Modifier = Modifier) = Column(modifier = modifier) {
val tool by rememberUpdatedState(state.tool)
val eventSink by rememberUpdatedState(state.eventSink)

Button(
onClick = { eventSink(Event.OpenTool) },
onClick = { eventSink(UiEvent.OpenTool) },
modifier = Modifier.fillMaxWidth()
) { Text(stringResource(R.string.action_tools_open_tool)) }

if (state.hasTips) {
Button(
onClick = { eventSink(Event.OpenToolTraining) },
onClick = { eventSink(UiEvent.OpenToolTraining) },
modifier = Modifier
.testTag(TEST_TAG_ACTION_TOOL_TRAINING)
.fillMaxWidth()
Expand All @@ -288,7 +288,7 @@ internal fun ToolDetailsActions(state: State, modifier: Modifier = Modifier) = C

val isFavorite by remember { derivedStateOf { tool?.isFavorite == true } }
OutlinedButton(
onClick = { eventSink(if (isFavorite) Event.UnpinTool else Event.PinTool) },
onClick = { eventSink(if (isFavorite) UiEvent.UnpinTool else UiEvent.PinTool) },
colors = ButtonDefaults.outlinedButtonColors(
contentColor = if (isFavorite) GodToolsTheme.GT_RED else MaterialTheme.colorScheme.primary
),
Expand All @@ -308,7 +308,7 @@ internal fun ToolDetailsActions(state: State, modifier: Modifier = Modifier) = C
}

@Composable
private fun ToolDetailsVariants(state: State, modifier: Modifier = Modifier) {
private fun ToolDetailsVariants(state: UiState, modifier: Modifier = Modifier) {
val currentTool by rememberUpdatedState(state.toolCode)
val variants by rememberUpdatedState(state.variants)

Expand Down
30 changes: 15 additions & 15 deletions app/src/main/kotlin/org/cru/godtools/ui/tooldetails/ToolDetailsPresenter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ import org.cru.godtools.model.Translation
import org.cru.godtools.shortcuts.GodToolsShortcutManager
import org.cru.godtools.sync.GodToolsSyncService
import org.cru.godtools.ui.drawer.DrawerMenuPresenter
import org.cru.godtools.ui.tooldetails.ToolDetailsScreen.Event
import org.cru.godtools.ui.tooldetails.ToolDetailsScreen.Page
import org.cru.godtools.ui.tooldetails.ToolDetailsScreen.State
import org.cru.godtools.ui.tooldetails.ToolDetailsScreen.UiEvent
import org.cru.godtools.ui.tooldetails.ToolDetailsScreen.UiState
import org.cru.godtools.ui.tools.ToolCard
import org.cru.godtools.ui.tools.ToolCardPresenter
import org.cru.godtools.util.createToolIntent
Expand All @@ -91,9 +91,9 @@ class ToolDetailsPresenter @AssistedInject constructor(
@Named(IS_CONNECTED_STATE_FLOW) private val isConnected: StateFlow<Boolean>,
@Assisted private val screen: ToolDetailsScreen,
@Assisted private val navigator: Navigator,
) : Presenter<State> {
) : Presenter<UiState> {
@Composable
override fun present(): State {
override fun present(): UiState {
val coroutineScope = rememberCoroutineScope()

var toolCode by rememberSaveable { mutableStateOf(screen.initialTool) }
Expand All @@ -114,12 +114,12 @@ class ToolDetailsPresenter @AssistedInject constructor(
val manifest by manifestManager.produceManifestState(translation)
val secondManifest by manifestManager.produceManifestState(secondTranslation)

val eventSink: (Event) -> Unit = remember {
val eventSink: (UiEvent) -> Unit = remember {
{
when (it) {
Event.NavigateUp -> navigator.pop()
UiEvent.NavigateUp -> navigator.pop()

Event.OpenTool -> tool?.let { tool ->
UiEvent.OpenTool -> tool?.let { tool ->
val intent = tool.createToolIntent(
context = context,
languages = when (tool.type) {
Expand All @@ -138,7 +138,7 @@ class ToolDetailsPresenter @AssistedInject constructor(
}
}

Event.OpenToolTraining -> tool?.let {
UiEvent.OpenToolTraining -> tool?.let {
// TODO: handle opening training tips and optionally showing the tutorial locally once the
// tutorial uses Circuit.
navigator.goTo(
Expand All @@ -151,20 +151,20 @@ class ToolDetailsPresenter @AssistedInject constructor(
)
}

Event.PinTool -> coroutineScope.launch {
UiEvent.PinTool -> coroutineScope.launch {
settings.setFeatureDiscovered(Settings.FEATURE_TOOL_FAVORITE)
toolsRepository.pinTool(toolCode)
syncService.syncDirtyFavoriteTools()
}

Event.UnpinTool -> coroutineScope.launch {
UiEvent.UnpinTool -> coroutineScope.launch {
toolsRepository.unpinTool(toolCode)
syncService.syncDirtyFavoriteTools()
}

is Event.SwitchVariant -> toolCode = it.variant
is UiEvent.SwitchVariant -> toolCode = it.variant

Event.PinShortcut -> pendingShortcut?.let { shortcutManager.pinShortcut(it) }
UiEvent.PinShortcut -> pendingShortcut?.let { shortcutManager.pinShortcut(it) }
}
}
}
Expand All @@ -176,7 +176,7 @@ class ToolDetailsPresenter @AssistedInject constructor(
DownloadLatestTranslation(downloadManager, toolCode, translation?.languageCode, isConnected)
DownloadLatestTranslation(downloadManager, toolCode, secondTranslation?.languageCode, isConnected)

return State(
return UiState(
toolCode = toolCode,
tool = tool,
banner = attachmentsRepository.rememberAttachmentFile(
Expand Down Expand Up @@ -210,7 +210,7 @@ class ToolDetailsPresenter @AssistedInject constructor(
private fun rememberVariants(
metaToolCode: String?,
secondLanguage: Language?,
eventSink: (Event) -> Unit,
eventSink: (UiEvent) -> Unit,
): ImmutableList<ToolCard.State> {
if (metaToolCode == null) return persistentListOf()

Expand All @@ -227,7 +227,7 @@ class ToolDetailsPresenter @AssistedInject constructor(
loadAvailableLanguages = true,
eventSink = {
when (it) {
ToolCard.Event.Click -> tool.code?.let { eventSink(Event.SwitchVariant(it)) }
ToolCard.Event.Click -> tool.code?.let { eventSink(UiEvent.SwitchVariant(it)) }
else -> Unit
}
}
Expand Down
21 changes: 10 additions & 11 deletions app/src/main/kotlin/org/cru/godtools/ui/tooldetails/ToolDetailsScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,12 @@ import org.cru.godtools.downloadmanager.DownloadProgress
import org.cru.godtools.model.Language
import org.cru.godtools.model.Tool
import org.cru.godtools.model.Translation
import org.cru.godtools.shared.tool.parser.model.Manifest
import org.cru.godtools.ui.drawer.DrawerMenuScreen
import org.cru.godtools.ui.tools.ToolCard

@Parcelize
data class ToolDetailsScreen(val initialTool: String, val secondLanguage: Locale? = null) : Screen {
data class State(
data class UiState(
val toolCode: String? = null,
val tool: Tool? = null,
val banner: File? = null,
Expand All @@ -35,21 +34,21 @@ data class ToolDetailsScreen(val initialTool: String, val secondLanguage: Locale
val availableLanguages: ImmutableList<String> = persistentListOf(),
val variants: ImmutableList<ToolCard.State> = persistentListOf(),
val drawerState: DrawerMenuScreen.State = DrawerMenuScreen.State(),
val eventSink: (Event) -> Unit = {},
val eventSink: (UiEvent) -> Unit = {},
) : CircuitUiState

enum class Page(@StringRes val tabLabel: Int) {
DESCRIPTION(R.string.label_tools_about),
VARIANTS(R.string.tool_details_section_variants_label)
}

sealed interface Event : CircuitUiEvent {
data object NavigateUp : Event
data object OpenTool : Event
data object OpenToolTraining : Event
data object PinTool : Event
data object UnpinTool : Event
data class SwitchVariant(val variant: String) : Event
data object PinShortcut : Event
sealed interface UiEvent : CircuitUiEvent {
data object NavigateUp : UiEvent
data object OpenTool : UiEvent
data object OpenToolTraining : UiEvent
data object PinTool : UiEvent
data object UnpinTool : UiEvent
data class SwitchVariant(val variant: String) : UiEvent
data object PinShortcut : UiEvent
}
}
4 changes: 2 additions & 2 deletions app/src/testDebug/kotlin/org/cru/godtools/ui/tooldetails/ToolDetailsLayoutPaparazziTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import org.cru.godtools.model.Language
import org.cru.godtools.model.randomTool
import org.cru.godtools.model.randomTranslation
import org.cru.godtools.ui.drawer.DrawerMenuScreenStateTestData
import org.cru.godtools.ui.tooldetails.ToolDetailsScreen.State
import org.cru.godtools.ui.tooldetails.ToolDetailsScreen.UiState
import org.cru.godtools.ui.tools.ToolCard
import org.cru.godtools.ui.tools.ToolCardStateTestData
import org.junit.runner.RunWith
Expand All @@ -43,7 +43,7 @@ class ToolDetailsLayoutPaparazziTest(
ToolCardStateTestData.tool.copy(secondLanguage = null)
)

private val state = State(
private val state = UiState(
tool = randomTool(
detailsBannerYoutubeVideoId = null,
shares = 123355,
Expand Down
Loading