Skip to content
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.

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

feat(Android): add basic onboarding UX #592

Merged
merged 10 commits into from
Dec 17, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ class ContentViewTests : KoinTest {
val koinApplication = koinApplication {
modules(
repositoriesModule(MockRepositories.buildWithDefaults()),
module { single<PhoenixSocket> { MockPhoenixSocket() } }
MainApplication.koinViewModelModule,
module { single<PhoenixSocket> { MockPhoenixSocket() } },
)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
package com.mbta.tid.mbta_app.android.onboarding

import android.location.Location
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.test.rule.GrantPermissionRule
import com.mbta.tid.mbta_app.android.location.MockLocationDataManager
import com.mbta.tid.mbta_app.model.OnboardingScreen
import com.mbta.tid.mbta_app.repositories.MockSettingsRepository
import com.mbta.tid.mbta_app.repositories.Settings
import kotlin.test.assertEquals
import kotlin.test.assertTrue
import org.junit.Rule
import org.junit.Test

class OnboardingScreenViewTest {
@get:Rule val composeTestRule = createComposeRule()

// We can't easily mock the permission request, so we grant the permission eagerly.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 nice

@get:Rule
val runtimePermissionRule =
GrantPermissionRule.grant(android.Manifest.permission.ACCESS_FINE_LOCATION)

@Test
fun testLocationFlow() {
var advanced = false
val locationDataManager = MockLocationDataManager(Location("mock"))
composeTestRule.setContent {
OnboardingScreenView(
screen = OnboardingScreen.Location,
advance = { advanced = true },
locationDataManager = locationDataManager,
)
}

composeTestRule
.onNodeWithText(
"We use your location to show you nearby transit options.",
substring = true
)
.assertIsDisplayed()
composeTestRule.onNodeWithText("Continue").performClick()

composeTestRule.waitForIdle()
assertTrue(advanced)
}

@Test
fun testHideMapsFlow() {
var savedSetting = false
val settingsRepo =
MockSettingsRepository(
settings = mapOf(Settings.HideMaps to false),
onSaveSettings = {
assertEquals(mapOf(Settings.HideMaps to true), it)
savedSetting = true
}
)
var advanced = false
composeTestRule.setContent {
OnboardingScreenView(
screen = OnboardingScreen.HideMaps,
advance = { advanced = true },
locationDataManager = MockLocationDataManager(Location("mock")),
settingsRepository = settingsRepo
)
}
composeTestRule
.onNodeWithText(
"When using TalkBack, we can skip reading out maps to keep you focused on transit information."
)
.assertIsDisplayed()
composeTestRule.onNodeWithText("Show maps").assertIsDisplayed()
composeTestRule.onNodeWithText("Hide maps").performClick()

composeTestRule.waitForIdle()
assertTrue(savedSetting)
assertTrue(advanced)
}

@Test
fun testFeedbackFlow() {
var advanced = false
composeTestRule.setContent {
OnboardingScreenView(
screen = OnboardingScreen.Feedback,
advance = { advanced = true },
locationDataManager = MockLocationDataManager(Location("mock")),
)
}
composeTestRule
.onNodeWithText(
"MBTA Go is in the early stages! We want your feedback" +
" as we continue making improvements and adding new features."
)
.assertIsDisplayed()
composeTestRule.onNodeWithText("Get started").performClick()

composeTestRule.waitForIdle()
assertTrue(advanced)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package com.mbta.tid.mbta_app.android.pages

import android.location.Location
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import com.mbta.tid.mbta_app.android.location.MockLocationDataManager
import com.mbta.tid.mbta_app.model.OnboardingScreen
import com.mbta.tid.mbta_app.repositories.MockOnboardingRepository
import kotlin.test.assertEquals
import kotlin.test.assertTrue
import org.junit.Rule
import org.junit.Test

class OnboardingPageTest {
@get:Rule val composeTestRule = createComposeRule()

@Test
fun testFlow() {
val completedScreens = mutableSetOf<OnboardingScreen>()
val onboardingRepository =
MockOnboardingRepository(
pendingOnboarding = OnboardingScreen.entries,
onMarkComplete = completedScreens::add
)
var finished = false

composeTestRule.setContent {
OnboardingPage(
screens = OnboardingScreen.entries,
locationDataManager = MockLocationDataManager(Location("mock")),
onFinish = { finished = true },
onboardingRepository = onboardingRepository,
skipLocationDialogue = true,
)
}

composeTestRule.onNodeWithText("Continue").performClick()
composeTestRule.waitUntil { completedScreens.size == 1 }
assertEquals(1, completedScreens.size)

composeTestRule.onNodeWithText("Show maps").performClick()
composeTestRule.waitUntil { completedScreens.size == 2 }
assertEquals(2, completedScreens.size)

composeTestRule.onNodeWithText("Get started").performClick()
composeTestRule.waitUntil { completedScreens.size == 3 }
assertEquals(OnboardingScreen.entries.toSet(), completedScreens)
assertTrue(finished)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import androidx.compose.material3.rememberBottomSheetScaffoldState
import androidx.compose.material3.rememberStandardBottomSheetState
import androidx.compose.runtime.Composable
import androidx.compose.runtime.DisposableEffect
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
Expand All @@ -24,22 +25,26 @@ import com.mbta.tid.mbta_app.android.location.rememberLocationDataManager
import com.mbta.tid.mbta_app.android.pages.MorePage
import com.mbta.tid.mbta_app.android.pages.NearbyTransit
import com.mbta.tid.mbta_app.android.pages.NearbyTransitPage
import com.mbta.tid.mbta_app.android.pages.OnboardingPage
import com.mbta.tid.mbta_app.android.phoenix.PhoenixSocketWrapper
import com.mbta.tid.mbta_app.android.state.getGlobalData
import com.mbta.tid.mbta_app.android.state.subscribeToAlerts
import com.mbta.tid.mbta_app.model.response.AlertsStreamDataResponse
import com.mbta.tid.mbta_app.network.PhoenixSocket
import io.github.dellisd.spatialk.geojson.Position
import org.koin.androidx.compose.koinViewModel
import org.koin.compose.koinInject

@OptIn(MapboxExperimental::class, ExperimentalMaterial3Api::class)
@Composable
fun ContentView(
socket: PhoenixSocket = koinInject(),
viewModel: ContentViewModel = koinViewModel(),
) {
val navController = rememberNavController()
var alertData: AlertsStreamDataResponse? = subscribeToAlerts()
val alertData: AlertsStreamDataResponse? = subscribeToAlerts()
val globalResponse = getGlobalData()
val pendingOnboarding = viewModel.pendingOnboarding.collectAsState().value
val locationDataManager = rememberLocationDataManager()
val mapViewportState = rememberMapViewportState {
setCameraOptions {
Expand All @@ -62,6 +67,16 @@ fun ContentView(
(socket as? PhoenixSocketWrapper)?.attachLogging()
onDispose { socket.detach() }
}

if (!pendingOnboarding.isNullOrEmpty()) {
OnboardingPage(
pendingOnboarding,
onFinish = { viewModel.clearPendingOnboarding() },
locationDataManager = locationDataManager
)
return
}

val sheetModifier = Modifier.fillMaxSize().background(colorResource(id = R.color.fill1))
NavHost(navController = navController, startDestination = Routes.NearbyTransit) {
composable<Routes.NearbyTransit> {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package com.mbta.tid.mbta_app.android

import androidx.lifecycle.ViewModel
import com.mbta.tid.mbta_app.model.OnboardingScreen
import com.mbta.tid.mbta_app.repositories.IOnboardingRepository
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.launch

class ContentViewModel(private val onboardingRepository: IOnboardingRepository) : ViewModel() {
private val _pendingOnboarding: MutableStateFlow<List<OnboardingScreen>?> =
MutableStateFlow(null)
val pendingOnboarding: StateFlow<List<OnboardingScreen>?> = _pendingOnboarding

init {
loadPendingOnboarding()
}

fun loadPendingOnboarding() {
CoroutineScope(Dispatchers.IO).launch {
val data = onboardingRepository.getPendingOnboarding()
_pendingOnboarding.value = data
}
}

fun clearPendingOnboarding() {
_pendingOnboarding.value = emptyList()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import com.mbta.tid.mbta_app.android.util.decodeMessage
import com.mbta.tid.mbta_app.dependencyInjection.makeNativeModule
import com.mbta.tid.mbta_app.initKoin
import com.mbta.tid.mbta_app.repositories.AccessibilityStatusRepository
import org.koin.androidx.viewmodel.dsl.viewModelOf
import org.koin.dsl.module
import org.phoenixframework.Socket

// unfortunately, expect/actual only works in multiplatform projects, so we can't
Expand All @@ -19,7 +21,12 @@ class MainApplication : Application() {
initKoin(
appVariant,
makeNativeModule(AccessibilityStatusRepository(applicationContext), socket.wrapped()),
koinViewModelModule,
this
)
}

companion object {
val koinViewModelModule = module { viewModelOf(::ContentViewModel) }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ open class LocationDataManager {
if (hasPermission) {
LaunchedEffect(Unit) {
locationClient.lastLocation.addOnSuccessListener { location: Location? ->
_currentLocation.tryEmit(location)
_currentLocation.value = location
}
}

Expand All @@ -106,7 +106,7 @@ open class LocationDataManager {
if (hasPermission && settingsCorrect) {
DisposableEffect(locationRequest, lifecycleOwner) {
val locationCallback = LocationListener { location ->
_currentLocation.tryEmit(location)
_currentLocation.value = location
}
val lifecycleObserver = LifecycleEventObserver { _, event ->
if (event == Lifecycle.Event.ON_START) {
Expand Down Expand Up @@ -134,12 +134,13 @@ open class LocationDataManager {

@OptIn(ExperimentalPermissionsApi::class)
@Composable
fun rememberPermissions() =
fun rememberPermissions(onPermissionsResult: (Map<String, Boolean>) -> Unit = {}) =
rememberMultiplePermissionsState(
listOf(
Manifest.permission.ACCESS_FINE_LOCATION,
Manifest.permission.ACCESS_COARSE_LOCATION
)
),
onPermissionsResult
)

open val currentLocation = _currentLocation.asStateFlow()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,11 @@ fun HomeMapView(

val locationProvider = remember { PassthroughLocationProvider() }

MapEffect(true) { map ->
map.mapboxMap.addOnMapClickListener { point -> handleStopClick(map, point) }
map.location.setLocationProvider(locationProvider)
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After completing the onboarding flow, the LocationDataManager.currentLocation already has the current location when the HomeMapView is being initialized, so if we don't setLocationProvider until after we've already collected, the first location will be completely lost, the puck won't show, and recentering will not work. (This was not fun to track down.)

LaunchedEffect(locationDataManager) {
locationDataManager.currentLocation.collect { location ->
if (location != null) {
Expand All @@ -268,11 +273,6 @@ fun HomeMapView(
}
}

MapEffect(true) { map ->
map.mapboxMap.addOnMapClickListener { point -> handleStopClick(map, point) }
map.location.setLocationProvider(locationProvider)
}

MapEffect(locationDataManager.hasPermission) { map ->
if (locationDataManager.hasPermission && viewportProvider.isDefault()) {
viewportProvider.follow(
Expand Down
Loading
Loading