Extend Example with ability to edit waypoints #566
Conversation
S2Ler
force-pushed
the
s2ler/extend-example
branch
from
b9545d0
to
d8d0cb6
Compare
This comment was marked as resolved.
This comment was marked as resolved.
S2Ler
force-pushed
the
s2ler/extend-example
branch
5 times, most recently
from
567d6cd
to
b14c769
Compare
S2Ler
changed the title
S2Ler
force-pushed
the
s2ler/extend-example
branch
from
b14c769
to
f8c5ca7
Compare
S2Ler
force-pushed
the
s2ler/extend-example
branch
from
f8c5ca7
to
7e42222
Compare
S2Ler
requested review from
a team,
MaximAlien,
kried and
1ec5
and removed request for
a team
|
Given that DirectionsPlayground(Former Example) app is separate project, we can bring back integration with Maps for better visualisation. |
Codecov Report
@@ Coverage Diff @@
## main #566 +/- ##
==========================================
- Coverage 85.30% 85.22% -0.08%
==========================================
Files 52 52
Lines 4545 4549 +4
==========================================
Hits 3877 3877
- Misses 668 672 +4
|
| @@ -110,7 +110,12 @@ public enum DirectionsError: LocalizedError { | |||
| */ | |||
|
|
|||
| case unknown(response: URLResponse?, underlying: Error?, code: String?, message: String?) | |||
|
|
|||
|
|
|||
| public var errorDescription: String? { | |||
There was a problem hiding this comment.
This change looks like it shouldn't be in this pull request. It extends the public API of Directions and it is not used in the example
There was a problem hiding this comment.
It can be separated, yes, but it is still used in the new code when presenting error alert: .alert(isPresented: .constant(error != nil), error: error)
There was a problem hiding this comment.
I can remove this code, but I think it is a net win instead of something that is bad to have.
|
|
||
| struct QueriesList: View { | ||
| @State | ||
| var queries: [Query] = [] { |
There was a problem hiding this comment.
Should we add the first Query to the screen or should we add text explaining that you need to add a query to see how Directions work? It might be confusing to see the empty list
DirectionsPlayground/DirectionsPlayground/Sources/WaypointsEditor.swift
Outdated
Show resolved
Hide resolved
| var urlString: String { | ||
| switch self { | ||
| case .waypointAllowsArrivingOnOppositeSide: | ||
| return "https://docs.mapbox.com/ios/directions/api/2.3.0/Classes/Waypoint.html#/s:16MapboxDirections8WaypointC28allowsArrivingOnOppositeSideSbvp" |
There was a problem hiding this comment.
It looks like we would have to update the version here with each Directions SDK release.
Should we use 2.8.0 here for now?
| import Combine | ||
| import MapboxDirections | ||
|
|
||
| struct QueryEditor: View { |
There was a problem hiding this comment.
As a suggestion, names like these sound as if it was a model, not a view. Would it be possible to use names like QueryEditView?
|
It is a great idea to rewrite the example to use SPM and SwiftUI! |
|
New example still not in a good shape. Editing query is buggy. |
S2Ler
force-pushed
the
s2ler/extend-example
branch
from
068d697
to
ed5e46e
Compare
localizedDescriptionto DirectionsError.swift. Without it, Xcode shows runtime issue saying thatlocalizedDescriptionisn't available.