Skip to content

Commit

Permalink
fix(Android): respond to non-TalkBack P0 QA (#695)
Browse files Browse the repository at this point in the history
* route pill text vertical alignment

* align GL pills with predictions

* align departure status text

* use less obviously placeholder detour icon

* fix search placeholder

* fix route header padding

* fix error banner padding

* use shimmer loading for upcoming trip loading

* fix tests that broke

* work around Modifier.weight bug

hell is empty, and all the devils are here
  • Loading branch information
boringcactus authored Jan 31, 2025
1 parent cec1639 commit ca044d8
Show file tree
Hide file tree
Showing 21 changed files with 244 additions and 95 deletions.
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package com.mbta.tid.mbta_app.android.component

import androidx.compose.ui.semantics.ProgressBarRangeInfo
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.hasProgressBarRangeInfo
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onNodeWithContentDescription
import androidx.compose.ui.test.onNodeWithText
Expand Down Expand Up @@ -129,8 +127,6 @@ class HeadsignRowViewTest {
fun showsLoading() {
init("Headsign", RealtimePatterns.Format.Loading)

composeTestRule
.onNode(hasProgressBarRangeInfo(ProgressBarRangeInfo.Indeterminate))
.assertIsDisplayed()
composeTestRule.onNodeWithContentDescription("Loading...").assertIsDisplayed()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.mbta.tid.mbta_app.android.component
import androidx.compose.ui.test.assertContentDescriptionContains
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onNodeWithContentDescription
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.onParent
Expand Down Expand Up @@ -201,4 +202,10 @@ class UpcomingTripViewTest {
.assertContentDescriptionContains("buses arriving in 5 min", substring = true)
.assertIsDisplayed()
}

@Test
fun testUpcomingTripViewWithLoading() {
composeTestRule.setContent { UpcomingTripView(UpcomingTripViewState.Loading) }
composeTestRule.onNodeWithContentDescription("Loading...").assertIsDisplayed()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ class NearbyTransitPageTest : KoinTest {
.performSemanticsAction(SemanticsActions.Expand)

composeTestRule.onNodeWithText("Nearby Transit").assertIsDisplayed()
composeTestRule.onNodeWithText("Search by stop").assertIsDisplayed()
composeTestRule.onNodeWithText("Stops").assertIsDisplayed()

composeTestRule.onNodeWithText("Green Line Long Name").assertExists()
composeTestRule.onNodeWithText("Green Line Stop").assertExists()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ class SearchBarOverlayTest : KoinTest {

currentNavEntry.value = null

composeTestRule.waitUntilAtLeastOneExists(hasText("Search by stop"))
val searchNode = composeTestRule.onNodeWithText("Search by stop")
composeTestRule.waitUntilAtLeastOneExists(hasText("Stops"))
val searchNode = composeTestRule.onNodeWithText("Stops")
searchNode.assertExists()
searchNode.requestFocus()
composeTestRule.awaitIdle()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.mbta.tid.mbta_app.android
import androidx.compose.foundation.isSystemInDarkTheme
import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.material3.MaterialTheme
import androidx.compose.material3.ProvideTextStyle
import androidx.compose.material3.Shapes
import androidx.compose.material3.Typography
import androidx.compose.material3.darkColorScheme
Expand Down Expand Up @@ -111,7 +112,8 @@ fun MyApplicationTheme(
@Composable
fun textStyle(fontSize: TextUnit, fontWeight: FontWeight? = null): TextStyle =
TextStyle(
color = colorResource(R.color.text),
// setting to unspecified will inherit local color correctly
color = Color.Unspecified,
fontFamily = fontFamily,
fontSize = fontSize,
fontWeight = fontWeight
Expand All @@ -134,5 +136,8 @@ fun MyApplicationTheme(
large = RoundedCornerShape(0.dp)
)

MaterialTheme(colorScheme = colors, typography = typography, shapes = shapes, content = content)
MaterialTheme(colorScheme = colors, typography = typography, shapes = shapes) {
// default text style is bodyLarge
ProvideTextStyle(value = MaterialTheme.typography.bodySmall, content = content)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.PaddingValues
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.heightIn
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size
Expand Down Expand Up @@ -39,11 +38,12 @@ import kotlin.time.Duration.Companion.minutes
import kotlinx.datetime.Clock

@Composable
fun ErrorBanner(vm: ErrorBannerViewModel) {
fun ErrorBanner(vm: ErrorBannerViewModel, modifier: Modifier = Modifier) {
val state by vm.errorState.collectAsState()
when (state) {
is ErrorBannerState.DataError -> {
ErrorCard(
modifier,
details = {
Text(
stringResource(R.string.error_loading_data),
Expand All @@ -60,6 +60,7 @@ fun ErrorBanner(vm: ErrorBannerViewModel) {
}
is ErrorBannerState.NetworkError -> {
ErrorCard(
modifier,
details = {
Row(verticalAlignment = Alignment.CenterVertically) {
Icon(painterResource(R.drawable.wifi_slash), contentDescription = "")
Expand All @@ -76,7 +77,7 @@ fun ErrorBanner(vm: ErrorBannerViewModel) {
is ErrorBannerState.StalePredictions -> {
if (vm.loadingWhenPredictionsStale) {
Row(
modifier = Modifier.heightIn(60.dp),
modifier = modifier.heightIn(60.dp),
verticalAlignment = Alignment.CenterVertically
) {
Spacer(modifier = Modifier.weight(1f))
Expand All @@ -85,6 +86,7 @@ fun ErrorBanner(vm: ErrorBannerViewModel) {
}
} else {
ErrorCard(
modifier,
details = {
val minutes = (state as ErrorBannerState.StalePredictions).minutesAgo()
Text(
Expand All @@ -110,10 +112,15 @@ fun ErrorBanner(vm: ErrorBannerViewModel) {
}

@Composable
private fun ErrorCard(details: @Composable () -> Unit, button: (@Composable () -> Unit)? = null) {
private fun ErrorCard(
modifier: Modifier = Modifier,
details: @Composable () -> Unit,
button: (@Composable () -> Unit)? = null
) {
Row(
modifier =
Modifier.padding(16.dp)
modifier
.padding(horizontal = 16.dp)
.heightIn(60.dp)
.background(Color.Gray.copy(alpha = 0.1f), shape = RoundedCornerShape(15.dp)),
verticalAlignment = Alignment.CenterVertically
Expand Down Expand Up @@ -177,13 +184,13 @@ private fun ErrorBannerPreviews() {
LaunchedEffect(null) { staleVM.activate() }
LaunchedEffect(null) { staleLoadingVM.activate() }
Column(
modifier = Modifier.background(MaterialTheme.colorScheme.background),
verticalArrangement = Arrangement.SpaceEvenly
modifier =
Modifier.background(MaterialTheme.colorScheme.background).padding(vertical = 16.dp),
verticalArrangement = Arrangement.spacedBy(16.dp)
) {
ErrorBanner(networkErrorVM)
ErrorBanner(dataErrorVM)
ErrorBanner(staleVM)
ErrorBanner(staleLoadingVM)
Spacer(modifier = Modifier.height(16.dp))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,26 @@ import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.heightIn
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.widthIn
import androidx.compose.foundation.layout.wrapContentHeight
import androidx.compose.material3.Icon
import androidx.compose.material3.MaterialTheme
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.scale
import androidx.compose.ui.res.painterResource
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import com.mbta.tid.mbta_app.android.R
import com.mbta.tid.mbta_app.android.generated.drawableByName
import com.mbta.tid.mbta_app.android.util.modifiers.placeholderIfLoading
import com.mbta.tid.mbta_app.model.ObjectCollectionBuilder
import com.mbta.tid.mbta_app.model.RealtimePatterns
import com.mbta.tid.mbta_app.model.Route
import com.mbta.tid.mbta_app.model.RouteType
import com.mbta.tid.mbta_app.model.TripInstantDisplay

sealed interface PillDecoration {
data class OnRow(val route: Route) : PillDecoration
Expand Down Expand Up @@ -71,9 +77,13 @@ fun PredictionRowView(
when (predictions) {
is RealtimePatterns.Format.Some ->
predictions.trips.mapIndexed { index, prediction ->
Row(verticalAlignment = Alignment.CenterVertically) {
Row(
horizontalArrangement = Arrangement.spacedBy(2.dp),
verticalAlignment = Alignment.CenterVertically
) {
UpcomingTripView(
UpcomingTripViewState.Some(prediction.format),
modifier = Modifier.weight(1f, fill = false),
isFirst = index == 0,
isOnly = index == 0 && predictions.trips.count() == 1
)
Expand All @@ -83,7 +93,9 @@ fun PredictionRowView(
route,
null,
RoutePillType.Flex,
modifier = Modifier.scale(0.75f).padding(start = 2.dp)
modifier =
Modifier.wrapContentHeight(Alignment.CenterVertically)
.scale(0.75f)
)
}
}
Expand All @@ -109,3 +121,91 @@ fun PredictionRowView(
}
}
}

@Preview
@Composable
private fun PredictionRowViewPreview() {
val objects = ObjectCollectionBuilder()
val green = objects.line()
val greenB =
objects.route {
lineId = green.id
color = "00843D"
textColor = "FFFFFF"
shortName = "B"
longName = "Green Line B"
type = RouteType.LIGHT_RAIL
}
val trip = objects.trip()

Column {
PredictionRowView(
predictions =
RealtimePatterns.Format.Some(
listOf(
RealtimePatterns.Format.Some.FormatWithId(
trip.id,
RouteType.LIGHT_RAIL,
TripInstantDisplay.Boarding
)
),
null
),
pillDecoration = PillDecoration.OnPrediction(mapOf(trip.id to greenB))
) {
Text("Destination")
}

PredictionRowView(
RealtimePatterns.Format.Some(
listOf(
RealtimePatterns.Format.Some.FormatWithId(
trip.id,
RouteType.LIGHT_RAIL,
TripInstantDisplay.Overridden("Stopped 10 stops away")
)
),
null
),
pillDecoration = PillDecoration.OnRow(greenB)
) {
Text("Destination")
}

PredictionRowView(
RealtimePatterns.Format.Some(
listOf(
RealtimePatterns.Format.Some.FormatWithId(
trip.id,
RouteType.LIGHT_RAIL,
TripInstantDisplay.Overridden("Stopped 10 stops away")
)
),
null
),
pillDecoration = PillDecoration.OnPrediction(mapOf(trip.id to greenB))
) {
Text("Destination")
}

PredictionRowView(
RealtimePatterns.Format.Some(
listOf(
RealtimePatterns.Format.Some.FormatWithId(
"a",
RouteType.BUS,
TripInstantDisplay.ScheduleMinutes(6)
),
RealtimePatterns.Format.Some.FormatWithId(
"b",
RouteType.BUS,
TripInstantDisplay.ScheduleMinutes(15)
),
),
null
)
) {
Text("Destination")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.layout.widthIn
import androidx.compose.foundation.layout.wrapContentHeight
import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.material3.Icon
import androidx.compose.material3.LocalContentColor
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.platform.LocalContext
Expand Down Expand Up @@ -68,13 +70,6 @@ fun RoutePill(
else -> 24.dp
}

val lineHeight =
when (spec.size) {
RoutePillSpec.Size.CircleSmall,
RoutePillSpec.Size.FlexPillSmall -> 16.sp
else -> 24.sp
}

fun Modifier.withSizePadding() =
when (spec.size) {
RoutePillSpec.Size.FixedPill -> size(width = 50.dp, height = 24.dp)
Expand All @@ -96,9 +91,14 @@ fun RoutePill(
val typeText = route?.type?.typeText(LocalContext.current, true)

val finalModifier =
modifier.placeholderIfLoading().withColor().withSizePadding().semantics {
contentDescription = "${route?.label ?: line?.longName ?: ""} ${typeText ?: ""}"
}
modifier
.placeholderIfLoading()
.withColor()
.withSizePadding()
.wrapContentHeight(Alignment.CenterVertically)
.semantics {
contentDescription = "${route?.label ?: line?.longName ?: ""} ${typeText ?: ""}"
}

when (pillContent) {
RoutePillSpec.Content.Empty -> {}
Expand All @@ -111,7 +111,6 @@ fun RoutePill(
fontWeight = FontWeight.Bold,
letterSpacing = 0.5.sp,
textAlign = TextAlign.Center,
lineHeight = lineHeight,
maxLines = 1
)
is RoutePillSpec.Content.ModeImage -> {
Expand Down
Loading

0 comments on commit ca044d8

Please sign in to comment.