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(DebugView): trip, stop, vehicle ids #538

Merged
merged 3 commits into from
Nov 20, 2024
Merged

feat(DebugView): trip, stop, vehicle ids #538

merged 3 commits into from
Nov 20, 2024

Conversation

KaylaBrady
Copy link
Collaborator

@KaylaBrady KaylaBrady commented Nov 13, 2024

Summary

Ticket: Debug setting to show (trip, stop, vehicle, etc) IDs

What is this PR for?

This PR adds the low hanging fruit ids - trip, stop, and vehicle. I opted to add each element in the detail view (stop in stop details, trip & vehicle in trip details).

I didn't add anything around route patterns since our route pattern grouping logic is in flux - if we find it will be valuable, we can add that separately. I did take an initial pass at it and it would involve quite a bit of prop drilling unless we switch to using an EnvironmentObject to represent debug mode.

Testing

What testing have you done?

Added some unit tests & ran locally. Also checked formatting to address the DebugView expanding unexpectedly as raised in slack

@KaylaBrady KaylaBrady requested a review from a team as a code owner November 13, 2024 20:28
@KaylaBrady KaylaBrady requested a review from EmmaSimon November 13, 2024 20:28
@KaylaBrady KaylaBrady force-pushed the kb-debug-2 branch 2 times, most recently from 27e95ea to 320ef8f Compare November 13, 2024 20:35
@@ -51,6 +51,7 @@ struct ContentView: View {
.onReceive(inspection.notice) { inspection.visit(self, $0) }
.onAppear {
Task { await contentVM.loadOnboardingScreens() }
Task { await nearbyVM.loadDebugSetting() }
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not seeing the debug info if I just toggle the setting on without restarting the app (and vice versa). I think the nearby VM needs to reload when the setting changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you haven't looked at this yet, I based the combined stop and trip details changes off of this branch (partly to use the debug view, partly to use the existing debug loading in nearby VM), and I just fixed it as part of that, so don't worry about getting it working in this PR, it's fine as an interim state on a debug feature flag only we'll see.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay great, thank you!!

if nearbyVM.showDebugMessages {
DebugView {
VStack {
Text(verbatim: "trip id \(tripId)")
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: Add a : after trip id 😛

@KaylaBrady KaylaBrady merged commit 540ff3f into main Nov 20, 2024
7 checks passed
@KaylaBrady KaylaBrady deleted the kb-debug-2 branch November 20, 2024 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants