Skip to content

Commit af1833d

Browse files
bmanderclaude
bmander
and
claude
authored
Unify stop and route focus banners (#1897)
* Unify stop and route focus banners * Simplify focus banner rendering * Address focus-banner review feedback - Document the createComposeRule() @Suppress("DEPRECATION") in the two new test files with the standard #1792 rationale, matching the repo convention. - Keep focusBannerBottomPx unkeyed so switching between two equal-height banners doesn't reset it to 0 (onSizeChanged only fires on size change), which framed the map as if no banner showed. - Clear the pending map-focus latches in clearMapFocus() so a stop closed before its arrivals load can't leak a recenter onto the next stop. - Serialize route-favorite toggles via an optimistic override so rapid double-taps build on the last intent instead of a stale store snapshot; the override is reconciled away once the store catches up. Bike-station focus (the rest of #1894) is deferred to a follow-up; the PR description now references the issue without auto-closing it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 0f9bfa6 commit af1833d

22 files changed

Lines changed: 1303 additions & 909 deletions

File tree

‎onebusaway-android/src/androidTest/java/org/onebusaway/android/ui/arrivals/ArrivalsPanelHeaderTest.kt‎

Lines changed: 0 additions & 124 deletions
This file was deleted.

‎onebusaway-android/src/androidTest/java/org/onebusaway/android/ui/home/arrivals/ServiceAlertsDialogTest.kt‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/*
2+
* Copyright (C) 2026 Open Transit Software Foundation
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*/
10+
package org.onebusaway.android.ui.home.arrivals
11+
12+
import androidx.compose.ui.test.assertIsDisplayed
13+
import androidx.compose.ui.test.junit4.createComposeRule
14+
import androidx.compose.ui.test.onNodeWithTag
15+
import androidx.compose.ui.test.onNodeWithText
16+
import androidx.compose.ui.test.performClick
17+
import org.junit.Assert.assertEquals
18+
import org.junit.Rule
19+
import org.junit.Test
20+
import org.onebusaway.android.ui.arrivals.AlertItem
21+
import org.onebusaway.android.ui.arrivals.AlertSeverity
22+
import org.onebusaway.android.ui.arrivals.ArrivalsUiState
23+
import org.onebusaway.android.ui.arrivals.StopHeader
24+
import org.onebusaway.android.time.ServerTime
25+
26+
class ServiceAlertsDialogTest {
27+
28+
// See EtaStripJustifyTest for why the v1 (Unconfined) rule is used here (issue #1792).
29+
@Suppress("DEPRECATION")
30+
@get:Rule
31+
val composeRule = createComposeRule()
32+
33+
@Test
34+
fun summaryOpensTheSelectedAlert() {
35+
var selected: String? = null
36+
val alert = AlertItem("content", "situation", setOf("situation"), "Route detour", AlertSeverity.WARNING)
37+
val content = ArrivalsUiState.Content(
38+
header = StopHeader("stop", "Main St", "N", false),
39+
arrivals = emptyList(),
40+
routeGroups = emptyList(),
41+
minutesAfter = 60,
42+
windowEnd = ServerTime(0),
43+
isStale = false,
44+
alerts = listOf(alert),
45+
)
46+
composeRule.setContent {
47+
ServiceAlertsDialog(
48+
content = content,
49+
onShowAlert = { selected = it },
50+
onHideAlert = {},
51+
onShowHiddenAlerts = {},
52+
onDismiss = {},
53+
)
54+
}
55+
56+
composeRule.onNodeWithTag("focus_alert_panel").assertIsDisplayed()
57+
composeRule.onNodeWithText("Route detour").performClick()
58+
composeRule.runOnIdle { assertEquals("situation", selected) }
59+
}
60+
}

‎onebusaway-android/src/androidTest/java/org/onebusaway/android/ui/home/map/FocusBannerTest.kt‎

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
/*
2+
* Copyright (C) 2026 Open Transit Software Foundation
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.onebusaway.android.ui.home.map
17+
18+
import androidx.compose.ui.test.assertHasClickAction
19+
import androidx.compose.ui.test.assertIsNotEnabled
20+
import androidx.compose.ui.test.assertIsDisplayed
21+
import androidx.compose.ui.test.performClick
22+
import androidx.compose.ui.test.getUnclippedBoundsInRoot
23+
import androidx.compose.ui.test.junit4.createComposeRule
24+
import androidx.compose.ui.test.onNodeWithContentDescription
25+
import androidx.compose.ui.test.onNodeWithText
26+
import androidx.test.platform.app.InstrumentationRegistry
27+
import kotlin.math.abs
28+
import org.junit.Assert.assertTrue
29+
import org.junit.Rule
30+
import org.junit.Test
31+
import org.onebusaway.android.R
32+
33+
class FocusBannerTest {
34+
35+
// See EtaStripJustifyTest for why the v1 (Unconfined) rule is used here (issue #1792).
36+
@Suppress("DEPRECATION")
37+
@get:Rule
38+
val composeRule = createComposeRule()
39+
40+
private val context = InstrumentationRegistry.getInstrumentation().targetContext
41+
42+
private fun setStopBanner(
43+
hasAlerts: Boolean = true,
44+
onClearSubordinateRoute: () -> Unit = {},
45+
) {
46+
composeRule.setContent {
47+
FocusBanner(
48+
state = FocusBannerState.Stop(
49+
title = "Pine St & 3rd Ave",
50+
direction = "N",
51+
isFavorite = false,
52+
favoriteEnabled = true,
53+
hasAlerts = hasAlerts,
54+
subordinateRoutes = listOf(
55+
FocusBannerState.SubordinateRoute("65"),
56+
FocusBannerState.SubordinateRoute("75"),
57+
FocusBannerState.SubordinateRoute("40"),
58+
),
59+
subordinateHeadsign = "Downtown",
60+
),
61+
onClose = {},
62+
onToggleFavorite = {},
63+
onShowAlerts = {},
64+
onClearSubordinateRoute = onClearSubordinateRoute,
65+
onRecenterStop = {},
66+
onSelectDirection = {},
67+
onFrameRoute = {},
68+
onHeight = {},
69+
)
70+
}
71+
}
72+
73+
@Test
74+
fun stopChromeUsesCompactStarAndAccessibleAlertTarget() {
75+
setStopBanner()
76+
composeRule.onNodeWithContentDescription(
77+
context.getString(R.string.stop_shortcut)
78+
).assertIsDisplayed()
79+
val starBounds = composeRule.onNodeWithContentDescription(
80+
context.getString(R.string.bus_options_menu_add_star)
81+
).assertHasClickAction().getUnclippedBoundsInRoot()
82+
assertTrue((starBounds.right - starBounds.left).value in 25.9f..26.9f)
83+
assertTrue((starBounds.bottom - starBounds.top).value in 25.9f..26.9f)
84+
85+
val alertBounds = composeRule.onNodeWithContentDescription(
86+
context.getString(R.string.stop_info_show_alerts)
87+
).assertHasClickAction().getUnclippedBoundsInRoot()
88+
assertTrue((alertBounds.right - alertBounds.left).value >= 47.5f)
89+
assertTrue((alertBounds.bottom - alertBounds.top).value >= 47.5f)
90+
}
91+
92+
@Test
93+
fun routeBannerUsesRouteOrientationIcon() {
94+
composeRule.setContent {
95+
FocusBanner(
96+
state = FocusBannerState.Route(
97+
header = org.onebusaway.android.map.RouteHeader(
98+
loading = false,
99+
shortName = "40",
100+
longName = "Downtown - Northgate",
101+
agency = "Metro",
102+
routeId = "1_40",
103+
),
104+
isFavorite = false,
105+
),
106+
onClose = {},
107+
onToggleFavorite = {},
108+
onShowAlerts = {},
109+
onClearSubordinateRoute = {},
110+
onRecenterStop = {},
111+
onSelectDirection = {},
112+
onFrameRoute = {},
113+
onHeight = {},
114+
)
115+
}
116+
117+
composeRule.onNodeWithContentDescription(
118+
context.getString(R.string.route_shortcut)
119+
).assertIsDisplayed()
120+
composeRule.onNodeWithContentDescription(
121+
context.getString(R.string.bus_options_menu_add_star)
122+
).assertIsDisplayed().assertHasClickAction()
123+
}
124+
125+
@Test
126+
fun stopBannerShowsIdentityAndFullSubordinateRouteChain() {
127+
setStopBanner()
128+
composeRule.onNodeWithText("Pine St & 3rd Ave (N)").assertIsDisplayed()
129+
listOf("65", "75", "40", "Downtown").forEach {
130+
composeRule.onNodeWithText(it).assertIsDisplayed()
131+
}
132+
}
133+
134+
@Test
135+
fun subordinateRouteDismissIsACompactExactSizeTarget() {
136+
var cleared = false
137+
setStopBanner(onClearSubordinateRoute = { cleared = true })
138+
139+
val dismiss = composeRule.onNodeWithContentDescription(
140+
context.getString(R.string.stop_info_unselect_route)
141+
).assertIsDisplayed().assertHasClickAction()
142+
val bounds = dismiss.getUnclippedBoundsInRoot()
143+
assertTrue((bounds.right - bounds.left).value in 21.5f..22.5f)
144+
assertTrue((bounds.bottom - bounds.top).value in 21.5f..22.5f)
145+
dismiss.performClick()
146+
assertTrue(cleared)
147+
}
148+
149+
@Test
150+
fun loadingStopBannerReservesTheFavoriteStar() {
151+
composeRule.setContent {
152+
FocusBanner(
153+
state = FocusBannerState.Stop(
154+
title = "Pine St & 3rd Ave",
155+
direction = null,
156+
isFavorite = false,
157+
favoriteEnabled = false,
158+
hasAlerts = false,
159+
),
160+
onClose = {},
161+
onToggleFavorite = {},
162+
onShowAlerts = {},
163+
onClearSubordinateRoute = {},
164+
onRecenterStop = {},
165+
onSelectDirection = {},
166+
onFrameRoute = {},
167+
onHeight = {},
168+
)
169+
}
170+
171+
val star = composeRule.onNodeWithContentDescription(
172+
context.getString(R.string.bus_options_menu_add_star)
173+
).assertIsDisplayed().assertIsNotEnabled().getUnclippedBoundsInRoot()
174+
val typeIcon = composeRule.onNodeWithContentDescription(
175+
context.getString(R.string.stop_shortcut)
176+
).getUnclippedBoundsInRoot()
177+
val stopName = composeRule.onNodeWithText("Pine St & 3rd Ave")
178+
.getUnclippedBoundsInRoot()
179+
180+
val typeIconCenter = (typeIcon.top.value + typeIcon.bottom.value) / 2f
181+
val starCenter = (star.top.value + star.bottom.value) / 2f
182+
val stopNameCenter = (stopName.top.value + stopName.bottom.value) / 2f
183+
val railCenter = (typeIconCenter + starCenter) / 2f
184+
assertTrue(abs(stopNameCenter - railCenter) < 1f)
185+
}
186+
}

‎onebusaway-android/src/main/AndroidManifest.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@
6464
Services' Maps dynamite module (loaded in-process on the API-33 emulator) references
6565
org.apache.http.* and crashes the instrumentation process with a NoClassDefFoundError
6666
for org.apache.http.ProtocolVersion when it isn't present, flaking the connected tests
67-
(e.g. ArrivalsPanelHeaderTest, EtaStripJustifyTest). required="false" so it's a no-op on
67+
(e.g. FocusBannerTest, EtaStripJustifyTest). required="false" so it's a no-op on
6868
devices/images that lack it. See #1819 CI flakiness. -->
6969
<uses-library
7070
android:name="org.apache.http.legacy"

‎onebusaway-android/src/main/java/org/onebusaway/android/map/MapHost.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,12 +189,12 @@ class MapHost(
189189
// ----- Padding + camera commands -----
190190

191191
// Map content padding: the floating top chrome (status bar + FAB-row clearance) supplies a baseline;
192-
// route focus may extend that edge farther down; the arrivals sheet sets the bottom. Declarative state
192+
// a focus banner may extend that edge farther down; the arrivals sheet sets the bottom. Declarative state
193193
// the renderer applies (Google: GoogleMap contentPadding), replacing the old imperative
194194
// mapView.setPadding(...) relay through HomeActivity.
195195
fun setTopChromeInset(px: Int) = renderState.setTopChromeInset(px)
196196

197-
fun setRouteFocusTopEdge(px: Int) = renderState.setRouteFocusTopEdge(px)
197+
fun setFocusBannerBottomEdge(px: Int) = renderState.setFocusBannerBottomEdge(px)
198198

199199
fun setBottomPadding(px: Int) = renderState.setBottomPadding(px)
200200

0 commit comments

Comments
 (0)