Skip to content

Commit

Permalink
Remove intermediate event sinks (#810)
Browse files Browse the repository at this point in the history
  • Loading branch information
ZacSweers authored Jul 13, 2023
1 parent d42e44e commit 14519f9
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import com.alorma.compose.settings.storage.base.getValue
import com.alorma.compose.settings.storage.base.setValue
import com.slack.circuit.codegen.annotations.CircuitInject
import com.slack.circuit.overlay.LocalOverlayHost
import com.slack.circuit.runtime.CircuitUiEvent
import com.slack.circuit.runtime.CircuitUiState
import com.slack.circuit.runtime.Navigator
import com.slack.circuit.runtime.Screen
Expand Down Expand Up @@ -229,12 +230,12 @@ object DebugSettingsScreen : Screen {
val eventSink: (Event) -> Unit,
) : CircuitUiState

sealed interface Event {
sealed interface Event : CircuitUiEvent {
data class NavigateTo(val screen: Screen) : Event

data class ToggleLogs(val show: Boolean) : Event

object ShareLogs : Event
data object ShareLogs : Event
}
}

Expand Down Expand Up @@ -321,8 +322,6 @@ constructor(
Ui<DebugSettingsScreen.State>, BaseSettingsUi by RealBaseSettingsUi(debugPreferences.datastore) {
@Composable
override fun Content(state: DebugSettingsScreen.State, modifier: Modifier) {
val eventSink = state.eventSink

if (state.logsToShow.isNotEmpty()) {
val overlayHost = LocalOverlayHost.current
LaunchedEffect(overlayHost) {
Expand All @@ -336,10 +335,10 @@ constructor(
)
when (result) {
LogsShareResult.SHARE -> {
eventSink(DebugSettingsScreen.Event.ShareLogs)
state.eventSink(DebugSettingsScreen.Event.ShareLogs)
}
LogsShareResult.DISMISS -> {
eventSink(DebugSettingsScreen.Event.ToggleLogs(show = false))
state.eventSink(DebugSettingsScreen.Event.ToggleLogs(show = false))
}
}
}
Expand Down Expand Up @@ -382,7 +381,7 @@ constructor(
val scope = rememberCoroutineScope()
DebugElementContent(
item,
{ screen -> eventSink(DebugSettingsScreen.Event.NavigateTo(screen)) }
{ screen -> state.eventSink(DebugSettingsScreen.Event.NavigateTo(screen)) }
) { key, value ->
scope.launch {
debugPreferences.edit { prefs ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,15 +226,14 @@ constructor(
@CircuitInject(ServiceScreen::class, AppScope::class)
@Composable
fun Service(state: ServiceScreen.State, modifier: Modifier = Modifier) {
val eventSink = state.eventSink
val lazyItems: LazyPagingItems<CatchUpItem> = state.items.collectAsLazyPagingItems()
var refreshing by remember { mutableStateOf(false) }
val pullRefreshState = rememberPullRefreshState(refreshing, onRefresh = lazyItems::refresh)
Box(modifier.pullRefresh(pullRefreshState)) {
if (state is VisualState) {
VisualServiceUi(lazyItems, state.themeColor, { refreshing = it }, eventSink)
VisualServiceUi(lazyItems, state.themeColor, { refreshing = it }, state.eventSink)
} else {
TextServiceUi(lazyItems, state.themeColor, { refreshing = it }, eventSink)
TextServiceUi(lazyItems, state.themeColor, { refreshing = it }, state.eventSink)
}

PullRefreshIndicator(
Expand Down
11 changes: 5 additions & 6 deletions app/src/main/kotlin/io/sweers/catchup/home/HomeScreen.kt
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,9 @@ object HomeScreen : Screen {
) : CircuitUiState

sealed interface Event : CircuitUiEvent {
object OpenSettings : Event
data object OpenSettings : Event

object ShowChangelog : Event
data object ShowChangelog : Event

data class NestedNavEvent(val navEvent: NavEvent) : Event
}
Expand Down Expand Up @@ -185,7 +185,6 @@ constructor(
@CircuitInject(HomeScreen::class, AppScope::class)
fun Home(state: HomeScreen.State, modifier: Modifier = Modifier) {
if (state.serviceMetas.isEmpty()) return // Not loaded yet
val eventSink = state.eventSink
val pagerState = key(state.serviceMetas) { rememberPagerState { state.serviceMetas.size } }
val currentServiceMeta = state.serviceMetas[pagerState.currentPage]
val title = stringResource(currentServiceMeta.name)
Expand Down Expand Up @@ -261,7 +260,7 @@ fun Home(state: HomeScreen.State, modifier: Modifier = Modifier) {
// TODO wire with Syllabus
if (state.changelogAvailable) {
IconButton(
onClick = { eventSink(HomeScreen.Event.ShowChangelog) },
onClick = { state.eventSink(HomeScreen.Event.ShowChangelog) },
) {
Icon(
imageVector = ImageVector.vectorResource(R.drawable.tips_and_updates),
Expand All @@ -270,7 +269,7 @@ fun Home(state: HomeScreen.State, modifier: Modifier = Modifier) {
}
}
IconButton(
onClick = { eventSink(HomeScreen.Event.OpenSettings) },
onClick = { state.eventSink(HomeScreen.Event.OpenSettings) },
) {
Icon(
imageVector = Icons.Default.Settings,
Expand Down Expand Up @@ -349,7 +348,7 @@ fun Home(state: HomeScreen.State, modifier: Modifier = Modifier) {
) {
CircuitContent(
screen = ServiceScreen(state.serviceMetas[page].id),
onNavEvent = { eventSink(HomeScreen.Event.NestedNavEvent(it)) }
onNavEvent = { state.eventSink(HomeScreen.Event.NestedNavEvent(it)) }
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ constructor(
@CircuitInject(ChangelogScreen::class, AppScope::class)
@Composable
fun Changelog(state: ChangelogScreen.State, modifier: Modifier = Modifier) {
val sink = state.eventSink
LazyColumn(modifier = modifier) {
val items = state.items
if (items == null) {
Expand All @@ -107,7 +106,7 @@ fun Changelog(state: ChangelogScreen.State, modifier: Modifier = Modifier) {
val item = items[index]
ClickableItem(
modifier = Modifier.animateItemPlacement(),
onClick = { sink(ChangelogScreen.Event.Click(item.clickUrl!!)) },
onClick = { state.eventSink(ChangelogScreen.Event.Click(item.clickUrl!!)) },
) {
TextItem(item, colorResource(R.color.colorAccent))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ private fun OssItem.toCatchUpItem(): CatchUpItem {
@CircuitInject(LicensesScreen::class, AppScope::class)
@Composable
internal fun Licenses(state: LicensesScreen.State, modifier: Modifier = Modifier) {
val sink = state.eventSink
LazyColumn(modifier = modifier) {
val items = state.items
if (items == null) {
Expand Down Expand Up @@ -172,7 +171,7 @@ internal fun Licenses(state: LicensesScreen.State, modifier: Modifier = Modifier
val catchUpItem = ossItem.toCatchUpItem()
ClickableItem(
modifier = Modifier.animateItemPlacement(),
onClick = { sink(LicensesScreen.Event.Click(catchUpItem.clickUrl!!)) },
onClick = { state.eventSink(LicensesScreen.Event.Click(catchUpItem.clickUrl!!)) },
) {
Row {
Spacer(Modifier.width(50.dp))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,15 @@ data class ImageViewerScreen(
) : CircuitUiState

sealed interface Event : CircuitUiEvent {
object Close : Event
data object Close : Event

object NoOp : Event // Weird but necessary because of the reuse in bottom sheet
data object NoOp : Event // Weird but necessary because of the reuse in bottom sheet

object ShareImage : Event
data object ShareImage : Event

object CopyImage : Event
data object CopyImage : Event

object SaveImage : Event
data object SaveImage : Event

data class OpenInBrowser(val url: String) : Event
}
Expand Down Expand Up @@ -144,7 +144,6 @@ constructor(
@CircuitInject(ImageViewerScreen::class, AppScope::class)
@Composable
fun ImageViewer(state: ImageViewerScreen.State, modifier: Modifier = Modifier) {
val sink = state.eventSink
var showChrome by remember { mutableStateOf(true) }
val systemUiController = rememberSystemUiController()
systemUiController.isSystemBarsVisible = showChrome
Expand Down Expand Up @@ -173,7 +172,7 @@ fun ImageViewer(state: ImageViewerScreen.State, modifier: Modifier = Modifier) {

val dismissState = rememberFlickToDismissState()
if (dismissState.gestureState is Dismissed) {
sink(ImageViewerScreen.Event.Close)
state.eventSink(ImageViewerScreen.Event.Close)
}
// TODO bind scrim with flick. animate scrim out after flick finishes? Or with flick?
FlickToDismiss(state = dismissState) {
Expand All @@ -192,7 +191,7 @@ fun ImageViewer(state: ImageViewerScreen.State, modifier: Modifier = Modifier) {
modifier = Modifier.fillMaxSize(),
state = imageState,
onClick = { showChrome = !showChrome },
onLongClick = { launchShareSheet(scope, overlayHost, state, sink) },
onLongClick = { launchShareSheet(scope, overlayHost, state) },
)
}

Expand All @@ -206,7 +205,7 @@ fun ImageViewer(state: ImageViewerScreen.State, modifier: Modifier = Modifier) {
Modifier.align(Alignment.TopStart).padding(16.dp).statusBarsPadding(),
NavButtonType.CLOSE,
) {
sink(ImageViewerScreen.Event.Close)
state.eventSink(ImageViewerScreen.Event.Close)
}
}
}
Expand All @@ -218,7 +217,6 @@ private fun launchShareSheet(
scope: CoroutineScope,
overlayHost: OverlayHost,
state: ImageViewerScreen.State,
sink: (ImageViewerScreen.Event) -> Unit,
) =
scope.launch {
val result =
Expand Down Expand Up @@ -260,7 +258,7 @@ private fun launchShareSheet(
}
}
)
sink(result)
state.eventSink(result)
}

// TODO
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ import androidx.compose.ui.res.stringResource
import androidx.compose.ui.unit.dp
import com.slack.circuit.codegen.annotations.CircuitInject
import com.slack.circuit.overlay.LocalOverlayHost
import com.slack.circuit.runtime.CircuitUiEvent
import com.slack.circuit.runtime.CircuitUiState
import com.slack.circuit.runtime.Navigator
import com.slack.circuit.runtime.Screen
Expand Down Expand Up @@ -102,14 +103,14 @@ object OrderServicesScreen : Screen {
val eventSink: (Event) -> Unit = {},
) : CircuitUiState

sealed interface Event {
object Shuffle : Event
sealed interface Event : CircuitUiEvent {
data object Shuffle : Event

data class Reorder(val from: Int, val to: Int) : Event

object BackPress : Event
data object BackPress : Event

object Save : Event
data object Save : Event

data class DismissConfirmation(val save: Boolean, val pop: Boolean) : Event
}
Expand Down Expand Up @@ -211,7 +212,6 @@ constructor(
@OptIn(ExperimentalMaterial3Api::class)
@Composable
fun OrderServices(state: OrderServicesScreen.State, modifier: Modifier = Modifier) {
val eventSink = state.eventSink
if (state.showConfirmation) {
val overlayHost = LocalOverlayHost.current
LaunchedEffect(Unit) {
Expand All @@ -226,13 +226,13 @@ fun OrderServices(state: OrderServicesScreen.State, modifier: Modifier = Modifie
)
when (result) {
DialogResult.Cancel -> {
eventSink(OrderServicesScreen.Event.DismissConfirmation(save = false, pop = true))
state.eventSink(OrderServicesScreen.Event.DismissConfirmation(save = false, pop = true))
}
DialogResult.Confirm -> {
eventSink(OrderServicesScreen.Event.DismissConfirmation(save = true, pop = false))
state.eventSink(OrderServicesScreen.Event.DismissConfirmation(save = true, pop = false))
}
DialogResult.Dismiss -> {
eventSink(OrderServicesScreen.Event.DismissConfirmation(save = false, pop = false))
state.eventSink(OrderServicesScreen.Event.DismissConfirmation(save = false, pop = false))
}
}
}
Expand All @@ -245,7 +245,7 @@ fun OrderServices(state: OrderServicesScreen.State, modifier: Modifier = Modifie
navigationIcon = { BackPressNavButton() },
actions = {
IconButton(
onClick = { eventSink(OrderServicesScreen.Event.Shuffle) },
onClick = { state.eventSink(OrderServicesScreen.Event.Shuffle) },
content = {
Icon(
painter = painterResource(R.drawable.ic_shuffle_black_24dp),
Expand Down Expand Up @@ -278,11 +278,11 @@ fun OrderServices(state: OrderServicesScreen.State, modifier: Modifier = Modifie
indication = rememberRipple(color = Color.White)
),
// TODO show syllabus on fab
// .onGloballyPositioned { coordinates ->
// val (x, y) = coordinates.positionInRoot()
// },
// .onGloballyPositioned { coordinates ->
// val (x, y) = coordinates.positionInRoot()
// },
containerColor = colorResource(R.color.colorAccent),
onClick = { scope.launch { eventSink(OrderServicesScreen.Event.Save) } },
onClick = { scope.launch { state.eventSink(OrderServicesScreen.Event.Save) } },
content = {
Image(
painterResource(R.drawable.ic_save_black_24dp),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,6 @@ constructor(
@OptIn(ExperimentalFoundationApi::class)
@Composable
override fun Content(state: SettingsScreen.State, modifier: Modifier) {
val eventSink = state.eventSink
Scaffold(
modifier = modifier,
contentWindowInsets = WindowInsets(0, 0, 0, 0),
Expand Down Expand Up @@ -245,7 +244,7 @@ constructor(
title = stringResource(R.string.pref_reorder_services),
subtitle = stringResource(R.string.pref_order_services_description)
) {
eventSink(SettingsScreen.Event.NavToScreen(OrderServicesScreen))
state.eventSink(SettingsScreen.Event.NavToScreen(OrderServicesScreen))
}
}

Expand All @@ -254,7 +253,7 @@ constructor(
title = stringResource(R.string.pref_clear_cache),
subtitle = stringResource(R.string.pref_clear_cache_summary),
) {
eventSink(SettingsScreen.Event.ClearCache)
state.eventSink(SettingsScreen.Event.ClearCache)
}
}

Expand Down Expand Up @@ -310,7 +309,7 @@ constructor(
ClickablePreference(
title = stringResource(R.string.about),
) {
eventSink(SettingsScreen.Event.NavToScreen(AboutScreen))
state.eventSink(SettingsScreen.Event.NavToScreen(AboutScreen))
}
}

Expand Down

0 comments on commit 14519f9

Please sign in to comment.