-
Notifications
You must be signed in to change notification settings - Fork 88
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
Add SubType
to VisualInstruction.Component
.
#778
Add SubType
to VisualInstruction.Component
.
#778
Conversation
Codecov Report
@@ Coverage Diff @@
## main #778 +/- ##
=======================================
Coverage 85.29% 85.30%
=======================================
Files 52 52
Lines 4542 4545 +3
=======================================
+ Hits 3874 3877 +3
Misses 668 668
|
aa19788
to
7fe8012
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some naming-related feedback. (@abhishek1508, not sure if this is coming too late for the corresponding APIs on Android.)
Subtype that provides more context about the component guidance view that may help in visual | ||
markup and display choices. | ||
*/ | ||
public enum SubType: String, Codable, CaseIterable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We’re avoiding the term “Type” in favor of “Kind”, as in VisualInstruction.Component.Kind
, so “SubType” is a bit incongruous. If only a guidanceView
can have a subtype, then call it GuidanceViewKind
. Ideally, we’d make this an associated value of Kind.guidanceView
, but that’s difficult because Kind
is string-backed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's hard to find the best option here because this option is not yet public. At the same time it seems that SubType
is only related to guidance views, so I'll rename it to GuidanceViewKind
.
@@ -1,9 +1,5 @@ | |||
# Changes to Mapbox Directions for Swift | |||
|
|||
## v2.9.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This entry should not be public just yet.
Junction view. Bird’s-eye artist’s rendition view of the overhead signage and | ||
preferred road lane arrow on motorways where the road bifurcates into 2 or | ||
more motorway trunk roads. | ||
*/ | ||
case fork = "jct" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: imo, we should make these doc comments more clear since the naming has changed. Some of @1ec5's suggestions provide some clarity as to what each of these GuidanceViewKind
s mean, and it would be great to let developers know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit hard to provide more clarity at this point, maybe in future we'll be able to extend these docs. Current info regarding sub types was taken from private docs and unfortunately they do not have enough info either.
This reverts commit 94244ec. This feature contains breaking change and should be reimplemented.
Add
SubType
toVisualInstruction.Component
.