feat: add prominent 'hide all' alerts banner above active alerts (#1534)#45
Open
bmander wants to merge 30 commits into
Open
feat: add prominent 'hide all' alerts banner above active alerts (#1534)#45bmander wants to merge 30 commits into
bmander wants to merge 30 commits into
Conversation
…neBusAway#1604) The OneBusAway#1594 fix wired TripState.toServerClock() through the trajectory now line, but its tests only guarded TripTrajectory.nowMs. The overlay now line -- ExtrapolationSeries.nowMs at TripTrajectory.kt:181, the operand of the user-facing schedule-deviation label -- had no coverage: both existing cases built state with no anchor/history, so extrapolate() returned NoData and the overlay was null, letting a revert of line 181 pass every test. Drive a real mid-route anchor through withStatus on a grade-separated trip so schedule replay succeeds and the overlay is non-null, then: - assert the overlay now line is lifted by the anchor skew (a revert to nowMs = nowMs now fails), - assert the lifted now line lands within trajectory.bounds (guards the now-line-off-axis symptom from OneBusAway#1583), - add a negative-skew case (server behind local) to exercise the subtraction branch of toServerClock the +5s cases never reached.
…n-clock-skew-tests Add test coverage for the trajectory extrapolation-overlay clock-skew lift (OneBusAway#1604)
Bus stop icons were driven directly by each viewport API fetch, causing two problems: every stop blinked at the end of every pan/zoom (the renderers clear-and-redrew the whole stop layer on each snapshot), and since stops-for-location caps and returns an arbitrary subset, panning showed a shifting random subset. Drive icons from an access-ordered LRU cache that the API fetch updates additively, with two layers: - StopsMapController accumulates stops in an access-ordered LinkedHashMap LRU; a re-seen stop bumps to most-recently-used keeping the same instance (so an unchanged fetch conflates and never redraws). Eviction walks eldest-first via trimStopCache, never drops the focused stop, and is skipped in route mode (the whole route is fed in one batch). - Both flavor renderers diff stop markers by id (the reconcileVehicleMarkers pattern): add new, remove gone, re-icon only on a focus flip. Unchanged markers survive a static redraw, so they no longer blink. Also request maxCount = cache size from stops-for-location so a dense viewport fills the cache in fewer pans (server still clamps to its hard ceiling), and expose the cache size as an Advanced settings option (10-2000, default 200) behind PreferencesRepository, read per-load so a change applies on the next pan.
…ops" banner At distant (zoomed-out) zoom the map gets crowded, so transition the full directional bus-stop icons to small dots, and surface a banner when the API truncates the stop list. - A pure zoom-band classifier (stopZoomBand / stopIconKind) in map/render decides each stop's icon: full icon, focused full icon, dot, or accent (focused) dot. JVM-unit-tested. - The zoom band is a first-class MapRenderState snapshot field, derived from the camera by StopsMapController and published via distinctUntilChanged. So a pure zoom re-fires the renderer like any other snapshot change and the renderers stay pure functions of the snapshot (no live camera reads for stops). Always-on, so it keeps updating in route mode. - Both flavor renderers reconcile stop markers in place, re-iconing only the markers whose icon kind changed (focus flip or band crossing) — no blink. The dot bitmap is a shared StopBitmaps.dot() (src/main); the focused dot uses a new map_stop_focus color shared with selected_map_stop_icon.xml. - Perf: StopIconFactory / MapLibreStopIcons now build their BitmapDescriptor / Icon caches once instead of re-wrapping per call, so re-iconing ~1000 markers on a band crossing reuses cached descriptors (fixes the transition stutter on dense viewports). - "Zoom in to see more stops" banner: a translucent, theme-error-container pill that slides down/up at the top of the map, shown when the stops API reports limitExceeded (more stops match the viewport than were returned). Driven by a MapHost.moreStopsAvailable flag mirroring the progress flow.
Drive map stop icons from an LRU cache, not each API fetch
…neBusAway#1602) Non-blocking follow-ups from OneBusAway#1584: 1. Surface RegionState.Failed to the user. A catastrophic region load now logs Log.e + a Crashlytics non-fatal at the holder.failed() site, and RegionPickerViewModel exposes a `failed` flow + `retry()` that RegionPickerHost renders as a dismissible, retryable "Couldn't load regions — check your connection" dialog. 2. Log the dropped partial stop-reveal. HomeNavHost's stop-reveal effect now records a Log.w + non-fatal when STOP_ID is present but lat/lon are missing (only reachable on a corrupted/half-restored SavedStateHandle). 3. Bound the deferred OTP-reset await. resetOtpVersionOnRegionChange's NeedsManualSelection branch now awaits a terminal state (Active OR Failed) and resets only on Active, so a retried refresh that fails no longer suspends forever. 4. Polish: MapReveal gains a typed SavedStateHandle.consumeStopReveal() helper (+ StopReveal) so the RESULT_MAP_STOP_* keys/types live in one file; TripInfoLauncher/NightLightLauncher add FLAG_ACTIVITY_NEW_TASK when launched from a non-Activity context; ReminderEditorArgs requires non-blank tripId/stopId; HomeDestinationDeps KDoc updated. Adds JVM unit tests for the picker failure/retry path, the bounded OTP await, consumeStopReveal, and the ReminderEditorArgs invariant.
…re-map-reveal-otp Surface region-load failures and harden map-reveal/OTP-reset edges (OneBusAway#1602)
Dynamic stop sizes: dots at distant zoom + "more stops" banner
…or-residue [codex] Remove travel-behavior residue
The modernization campaign re-introduced an identical private isInDarkMode(context) in both GoogleComposeAdapter and MapLibreComposeAdapter. Hoist the single copy into ThemeUtils.isInDarkMode(context) and call it from both adapters, dropping the now-unused AppCompatDelegate/Configuration imports. Rebased onto current main from the original PR OneBusAway#1545, whose target files (BaseMapFragment/MapLibreMapFragment/UIUtils) no longer exist.
Tapping "show vehicles on map" from a stop's arrival row put the map into
route mode showing the whole route: every live vehicle (including
opposite-direction buses that had already passed the stop) and every stop
across all direction variants. The user tapped an arrival at a specific
stop, so only that stop's direction is relevant.
Now the map shows only the stops and vehicles heading in the stop-relevant
direction. stops-for-route groups stops by direction, each group carrying
the GTFS direction id ("0"/"1"), so the map data source tags each stop with
the direction(s) it serves (RouteMapStop). The RouteMapRepository narrows
the stops to the tapped stop's direction and resolves the direction id; the
vehicle sampler filters live vehicles to trips with that directionId. An
ambiguous stop (in zero or several groups, e.g. a shared/loop stop) falls
back to the whole route. Every other "show route on map" entry point (My
Routes, route search, RouteInfo) is unchanged.
To avoid flashing all vehicles for the seconds before the (slower)
stops-for-route load resolves the direction, the controller holds the
vehicle layer back in a direction launch until the load lands
(DirectionState.Pending -> Resolved), so vehicles appear already-filtered
with the stops; whole-route launches are unaffected.
The "show route on map" intent is carried as a single ShowRouteRequest
payload: every launcher builds it once and both UI transports (the
navigation reveal and the home MapDirective.ShowRoute) carry it opaquely,
unwrapped once at MapViewModel.toRoute, so adding a future map-intent
parameter is a change to that one type rather than every hop in between.
Unit tests cover the direction focus and the vehicle direction filter.
…into Room Retire the hand-rolled `ObaProvider` SQLite ContentProvider (and `ObaContract`) and unify all local persistence into the existing Room `AppDatabase`, behind Hilt-injected repositories with reactive `Flow` reads. Behaviour-preserving; a one-time importer migrates existing users' data with no loss. - One Room database (`app_database`) subsumes the legacy provider DB, the separate `study-survey-db`, and the `survey_pref` SharedPreferences blob. - One-time, crash-immune importer copies existing user data (favourites, custom stop names, reminders, region cache, route filters, alert-hidden state) from the legacy provider DB on first launch, then deletes it. Idempotent, and a failed import can never wedge the app (it opens without the data and retries). - ContentObserver + cursor reads become Room `@Query` Flows: the My-tab recent/ starred lists are reactive and region-scoped. `<provider>` removed from the manifest; `ObaProvider` deleted. - Backup rewritten to target `app_database` (WAL checkpoint before copy; old legacy-format backups are routed through the importer on restore). - Survey + wide-alert repositories are Hilt-injected (no static DB access). - `ObaContract` dissolved: the surviving intent/URI + reminder date-math vocabulary moved to `ui/nav/DeepLinkUris` + `database/oba/TripDepartureTime`. Tested: JVM unit tests for the pure logic and repositories; instrumented Room tests for DAO merge/query semantics, the v2->v3 migration, and the legacy import. The full on-device upgrade path (legacy provider data -> Room) was verified end-to-end on a Pixel 7 Pro with no data loss.
The show-route path now carries a ShowRouteRequest instead of a bare routeId String; update the test's MapDirective.ShowRoute accessor and requestShowRouteOnMap call to match, fixing the unit-test compile failure.
…neBusAway#1612) Arrival predictions, starred-stop badges, vehicle 'data updated N ago' age, and service-alert active-window checks computed durations as (server time - System.currentTimeMillis()), so a skewed device clock shifted every number. Route 'now' through the response's server clock instead, so the prediction and the 'now' it's measured against share one clock and skew cancels out. - ArrivalsRepository: use StopArrivals.currentTime for ETAs + alert windows - MyListRepository: badge ETAs use the fetch's StopArrivals.currentTime - VehicleInfoWindow: project RouteTrips.currentTimeMs forward by elapsed device time (rememberServerNowMs/serverNowMs) for a skew-free live age - GtfsAlertsHelper: use the GTFS-RT feed header timestamp for the 24h window - SituationUtils: make the active-window check a pure function of its input (drop the helper-level System.currentTimeMillis()) Guardrail: CLAUDE.md 'Time domains' section + SituationUtilsTest / ServerNowMsTest. Device-local timers (poll scheduling, cache TTLs, 'updated Ns ago' vs a locally-stamped write, extrapolation's paired local clock) are left as-is.
OneBusAway#1612) - GtfsAlerts: hoist the device-clock fallback to the fetch boundary and pass a guaranteed-positive nowMs down, so isStartDateWithin24Hours is a pure function of its inputs (matches the SituationUtils treatment). - VehicleInfoWindow: trim the duplicated serverNowMs KDoc; correct the nowMs comment to name lastLocationUpdateTime (the field the age is actually measured against, with lastUpdateTime as fallback). - GtfsAlertsHelper: Javadoc {@code nowMs} instead of KDoc-style [nowMs]. - SituationUtilsTest: drop the redundant purity test (the boundary tests already demonstrate it).
Review fixes on the legacy-ContentProvider -> Room cutover: - Backup restore (Room-format) now restarts the process after swapping the database file. The Hilt-cached AppDatabase singleton (and every repository that captured a DAO from it) otherwise kept using the old, closed instance and threw on the next query. - Gate the My-list remove/clearAll writes: a remove/clear racing the one-time importer's clear-then-insert would otherwise be silently wiped. - Gate SurveyRepository reads/writes against the same one-time import so a query can't return an empty completed-set (re-showing an answered survey) or write a row the import then collides with. - Extract ImportGate to an interface (DefaultImportGate impl + Hilt binding) so gated repositories are unit-testable; add a SurveyRepository unit test with a no-op fake gate.
…by-direction Filter "show vehicles on map" to the stop-relevant direction
…dupe refactor: extract duplicated inDarkMode() helper to ThemeUtils
Resolve conflict in ArrivalActionHandlers.onShowVehiclesOnMap: keep this branch's recordRoute() (DBUtil was retired) and take main's ShowRouteRequest direction-focus call — the two edits landed on adjacent lines of one hunk.
…guards (OneBusAway#1612) Adversarial review of the server-clock fix surfaced four real issues: - ArrivalsRepository stale fallback: deriving now from lastGood.currentTime froze it, so stale ETAs/countdowns stopped advancing across repeated failures (a regression from the legacy fresh-device-now behavior). Pair lastGood with the device elapsed-realtime at receipt and project the server clock forward by the elapsed device time on the stale path — advancing again, still skew-free. - SituationUtils.isActiveWindowForSituation: the units heuristic inferred seconds-vs-millis from 'now', which misclassified a future seconds-based open-ended (to==0) window as active. Detect units by fixed magnitude per timestamp and require the start to have passed. - GtfsAlertsHelper.isStartDateWithin24Hours: a future-dated start yielded a negative elapsed that slipped past the <=24h upper bound. Require elapsed >= 0. Tests: add the open-ended before-start + future-start-no-end cases to SituationUtilsTest, and a new GtfsAlertsHelperTest pinning the 24h cutoff, boundary, and future-start guard.
…in this PR (OneBusAway#1612) - New 'No unsanctioned heuristics' section: heuristics (magnitude guesses, unit/type inference, fuzzy matching for something the data should state) are a human-sign-off gate, not an agent judgment call; prefer resolving the fact at its source. Includes a registry of known heuristics. - Document SituationUtils.toEpochMillis explicitly as a heuristic with its assumption and failure mode, and note the deeper fix (normalize units at the wire boundary).
…BusAway#1612) Investigating the 'upstream is inconsistent about units' claim showed the opposite: OBA active-window from/to are epoch SECONDS per the REST contract (SituationWindow in ObaApiModels) and every response fixture (DART/MTS/HART/…, 2015-2019 all ~1.4-1.5e9), while currentTime is epoch MILLIS. That's a fixed, known field mismatch, not upstream inconsistency — the magnitude 'is this seconds or millis' guess came from a single 2025 commit with no issue/test/data. Delete the heuristic; convert now to seconds deterministically and compare. Keeps the start-passed / open-ended (to==0) semantics and all tests. Updates the CLAUDE.md heuristics registry to empty, with this as the worked example of 'check the contract before guessing.'
…r OBA server source (OneBusAway#1612) Correcting an earlier over-confident 'deterministic seconds' change. Reading the OBA server source settles the unit question the previous commit got wrong: - GtfsRealtimeAlertLibrary.toMillis converts GTFS-RT active_period (seconds per spec) to millis on ingestion using a magnitude rule (threshold 1e12), and BeanFactoryV2.getTimeRange passes from/to through unchanged. So a MODERN server emits activeWindows in millis; older servers/fixtures emit seconds. The field is genuinely polymorphic across the ecosystem. - currentTime and creationTime are always millis. Treating from/to as fixed seconds (prior commit) would make every alert read inactive on a modern millis-emitting server. Restore a magnitude normalization that mirrors the server's own toMillis (threshold 1e12), now evidence-backed rather than an invented guess. Document the polymorphism on SituationWindow (the wire contract) and in CLAUDE.md's worked example; add a millis-form test.
…ization altitude, repo race, monotonic vehicle age (OneBusAway#1612) From the high-effort review of this branch: - currentTime==0 no longer yields garbage: guard server currentTime at the two data-source boundaries (StopArrivals, RouteTrips) via serverNowOrDeviceClock, falling back to the device clock only in the degenerate missing-field case. - GtfsAlertsHelper.isStartDateWithin24Hours: guard getActivePeriod(0) — a GTFS-RT alert with no active_period is optional/'always active'; return false instead of throwing IndexOutOfBounds (which aborted the whole feed). - ArrivalsRepository: hold lastGood + its receipt time in one @volatile pair so a concurrent getArrivals (user refresh over the poll loop) can't mix a new snapshot with an old receipt time. - VehicleInfoWindow: project the vehicle age on the monotonic clock (SystemClock.elapsedRealtime) and clamp the delta at 0, so an NTP/clock change or the sync-vs-lagged ticker can't corrupt or reverse the age. - Altitude: move the seconds/millis normalization to the wire→domain adapter (situationEpochToMillis), so ObaSituation.ActiveWindow is unambiguously millis and isActiveWindowForSituation is a plain comparison. Documents the failure mode (satisfies the CLAUDE.md heuristics rule). - Tests: normalization + no-active-period + clamp cases; SituationUtilsTest now uses millis windows; drop dead dayMs fixture.
Retire the legacy SQLite ContentProvider; unify local storage into Room
…seline Conflicts in ArrivalsRepository.kt and MyListRepository.kt, resolved by keeping both sides' intent: - Take main's Room-DAO data access + importGate + Firebase + the new convertArrivals isFavorite lambda; keep OneBusAway#1612's server-clock 'now' (snapshot.currentTime), the @volatile LastGood(snapshot, elapsedMs) pairing, and the stale-path forward projection. - MyList: main moved arrivalsPoll to a top-level fn; drop the stale device-clock nowMs threading and use snapshot.currentTime for the badge ETA baseline, with main's constant-false favorite lambda. Main + full JVM unit suite compile and pass.
…-eta-baseline Use the server clock as the ETA baseline everywhere (OneBusAway#1612)
…BusAway#1534) The 'hide alerts' action was only reachable from the toolbar overflow menu, so users reported alerts felt impossible to dismiss. Add a banner above the stop's active-alert list showing the count and a 'Hide all' button, wired to the existing ArrivalsViewModel.hideAllAlerts(). Hiding reveals the existing 'N hidden alerts -> show' footnote as the recovery path. Reimplements OneBusAway#1553 (by @Hitanshi7556) against the post-modernization Compose UI; the original patch targeted the now-deleted View-based ArrivalsListHeader.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses issue OneBusAway#1534 — make the "hide all alerts" action discoverable.
The "hide alerts" action was only reachable from the toolbar overflow menu, so users reported alerts felt impossible to dismiss. This adds a banner directly above a stop's active-alert list showing the count and a Hide all button.
Details
HideAllAlertsBannerabove the alert list inArrivalsList(ArrivalsScreen.kt): aN active alerts showncount label plus a distinct Hide all button.ArrivalsListcall sites — the standalone arrivals screen and the home bottom-sheet panel (ArrivalsPanel.kt) — to the existingArrivalsViewModel.hideAllAlerts().N hidden alerts → showfootnote as the recovery path (no new snackbar needed).alert_shown_text; reuses the existinghide_allstring for the button.Design notes
alert_filter_textplural — that string is still used by the hidden-alerts footnote, so repurposing it would mis-word that banner.Context
This reimplements the intent of OneBusAway#1553 (by @Hitanshi7556) against the current post-modernization Compose UI. That PR targeted the now-deleted View-based
ArrivalsListHeader/AlertListand can no longer merge. Opening this on my fork so the original author has the chance to reimplement upstream.Built and installed on device (
installObaGoogleDebug);compileObaGoogleDebugKotlinpasses.