Make Origin Destination Working Marker View #32
Conversation
| Text("Add Map Location") | ||
| }) | ||
| .padding(.all) | ||
| .background(Color.gray) |
There was a problem hiding this comment.
Hi @aaronbrethorst, I wonder do you have any suggestion on this color? Seems like the contrast is not good for our texts...
There was a problem hiding this comment.
I recommend a white background and a gray border for the button.
OTPKit/Features/OriginDestination/Sheets/OriginDestinationSheetView.swift
Show resolved
Hide resolved
| import MapKit | ||
| import SwiftUI | ||
|
|
||
| public final class LocationManagerService: NSObject, ObservableObject { |
There was a problem hiding this comment.
I move all the location-services based to one LocationManagerService. What do you think of this approach? I'm kinda confused about this since I'm afraid it will be bloated one day, but if I separate this into many classes I'm afraid it will make an unnecessary abstraction? What do you think? @aaronbrethorst
There was a problem hiding this comment.
Let's not worry about over-bloating eventually. For now, I think this is a totally reasonable approach. https://understandlegacycode.com/blog/refactoring-rule-of-three/#premature-refactoring-is-the-root-of-many-bad-things
There was a problem hiding this comment.
Got it. Thanks @aaronbrethorst!
There was a problem hiding this comment.
I think this looks like great progress! I have a couple thoughts about the UI:
Change Stop
Move "Select based on map" to this page. Also, please rename that option to "Choose on Map" (which is the same thing Google Maps calls that option).
MapMarkingView
After some considerations, I've made some changes to this UI. Check out the changes I made and tell me what you think. Here are a few things to note in particular about my changes:
- Buttons wrapped in HStack and given equal width
- Helper text above the buttons to explain how to use the feature
- Button styles driven entirely by the
.buttonStyle()modifier instead of custom styling.
| Text("Add Map Location") | ||
| }) | ||
| .padding(.all) | ||
| .background(Color.gray) |
There was a problem hiding this comment.
I recommend a white background and a gray border for the button.
About moving the "choose on map option", I assume that you expect something like this? (I'm not really sure) |
…the choose on map
Make Origin Destination Working Marker View
Related Issues
#22
Description
This PR scope is adding marker on origin and destination map
Media
abcdef.mov
Test Instructions