Progress view current road #307
Conversation
android/composeui/src/main/java/com/stadiamaps/ferrostar/composeui/theme/RoadNameViewTheme.kt
Show resolved
Hide resolved
android/composeui/src/main/java/com/stadiamaps/ferrostar/composeui/views/TripProgressView.kt
Show resolved
Hide resolved
apple/Sources/FerrostarMapLibreUI/Views/Overylays/LandscapeNavigationOverlayView.swift
Outdated
Show resolved
Hide resolved
android/composeui/src/main/java/com/stadiamaps/ferrostar/composeui/theme/RoadNameViewTheme.kt
Show resolved
Hide resolved
android/composeui/src/main/java/com/stadiamaps/ferrostar/composeui/views/CurrentRoadView.kt
Outdated
Show resolved
Hide resolved
android/composeui/src/main/java/com/stadiamaps/ferrostar/composeui/views/TripProgressView.kt
Show resolved
Hide resolved
...rc/main/java/com/stadiamaps/ferrostar/maplibreui/views/DynamicallyOrientingNavigationView.kt
Show resolved
Hide resolved
| @ViewBuilder currentRoadNameViewBuilder: (String?) -> AnyView = { name in | ||
| AnyView(CurrentRoadNameView(currentRoadName: name, theme: DefaultRoadNameViewTheme())) | ||
| } |
There was a problem hiding this comment.
This is probably fine as it's an internal view. No need to convert to view modifier.
| @ViewBuilder currentRoadNameViewBuilder: (String?) -> AnyView = { name in | ||
| AnyView(CurrentRoadNameView(currentRoadName: name, theme: DefaultRoadNameViewTheme())) | ||
| } |
There was a problem hiding this comment.
This is probably fine as it's an internal view. No need to convert to view modifier.
| @ViewBuilder currentRoadNameViewBuilder: @escaping (String?) -> AnyView = { name in | ||
| AnyView(CurrentRoadNameView(currentRoadName: name, theme: DefaultRoadNameViewTheme())) | ||
| } |
There was a problem hiding this comment.
Use view modifier pattern?
There was a problem hiding this comment.
Yeah; that might actually be the right move here...
| Text(currentRoadName) | ||
| .font(theme.textFont) | ||
| .foregroundStyle(theme.textColor) | ||
| .padding(.leading, 12) | ||
| .padding(.trailing, 12) | ||
| .padding(.vertical, 8) | ||
| .background(theme.backgroundColor) | ||
| .clipShape(.rect(cornerRadius: 48)) | ||
| .overlay(RoundedRectangle(cornerRadius: 48).stroke(theme.borderColor, lineWidth: 1)) |
There was a problem hiding this comment.
Would be nice to have the & clip shape in the init or theme (similar to android comment). To allow easy customization.
|
|
||
| public extension CurrentRoadNameView { | ||
| /// Sets the interior padding (expanding the buffer around the text). | ||
| func padding(_ padding: EdgeInsets) -> Self { |
There was a problem hiding this comment.
Nice! I expect with these modifiers. We can get rid of all of the init args?
@ViewBuilder currentRoadNameViewBuilder: @escaping (String?) -> AnyView = { name in
AnyView(CurrentRoadNameView(currentRoadName: name, theme: DefaultRoadNameViewTheme()))
}There was a problem hiding this comment.
Ahh that's an interesting idea! Let me try that...
There was a problem hiding this comment.
Like the view modifier methodology 👍. Added a comment about all the init @Viewbuilders now that the modifiers exist.
Secondarily, I expect we'd have a some snapshot tests of the new view? Or to update the progress view tests to include it in a bundled test?
Beyond those two things, looks good to me.
Closes #245. Closes close #244.
ArrivalView toProgressView`Android screenshot (simulator; sorry for the puck bugs:
iPhone 16:
iPhone SE 3rd gen:
Out of scope for now: generic interface for road names. I expect eventually someone will ask for this so they can do local map matching etc. for free roam experiences. We should support this eventually but doesn't need to be now.
For a future (but soon-ish) PR: Landscape layouts. These are... a bit trickier. Confirmed with the user asking for this that they are only blocked by portrait. I'll split up issues.