-
Notifications
You must be signed in to change notification settings - Fork 93
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
Intersections indicate preferred lane usage #529
Conversation
The build is currently erroring on type checking:
Looks like the tests just need to be updated to account for a new argument to |
I had one assertion that was failing, so I didn't push those updated tests. |
05463ba
to
92880c5
Compare
CHANGELOG.md
Outdated
@@ -6,6 +6,9 @@ | |||
* This library requires Turf v2.0.0-alpha.3. ([#525](https://github.com/mapbox/mapbox-directions-swift/pull/525)) | |||
* The `Incident.impact` property is now an `Incident.Impact` value instead of a string. ([#519](https://github.com/mapbox/mapbox-directions-swift/pull/519)) | |||
|
|||
## v2.0.0 | |||
* Added the `Intersection.preferredApproachLanes`, `Intersection.usableLaneIndication`, `Lane.isActive`, and `Lane.validIndication` properties that indicate preferred lane usage. ([#529](https://github.com/mapbox/mapbox-directions-swift/pull/529)) |
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.
The Lane
struct is internal, so we shouldn’t mention it in the changelog; the additions to Intersection
are what concern developers. However, we should also mention that the VisualInstruction.Component.lane(indications:isUsable:)
case has been renamed to VisualInstruction.Component.lane(indications:isUsable:preferredDirection:)
.
92880c5
to
ee8d778
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.
There’s a little bit of fallout from the original merge in the readme. Otherwise, there are a few minor nits; feel free to merge once they’re addressed.
@@ -6,6 +6,9 @@ | |||
* This library requires Turf v2.0.0-alpha.3. ([#525](https://github.com/mapbox/mapbox-directions-swift/pull/525)) | |||
* The `Incident.impact` property is now an `Incident.Impact` value instead of a string. ([#519](https://github.com/mapbox/mapbox-directions-swift/pull/519)) | |||
|
|||
## v2.0.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.
Even the whole section above is for v2.0.0, but we can clean it up after merging the PR.
README.md
Outdated
|
||
## Pricing | ||
|
||
API calls made to the Directions API are individually billed by request. Review the [pricing information](https://docs.mapbox.com/api/navigation/directions/#directions-api-pricing) and the [pricing page](https://www.mapbox.com/pricing/#directions) for current rates. |
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 part shouldn’t be removed. We expect it to no longer be part of the diff after rebasing, but it should remain in the codebase because it’s now in release-v2.0.
var usableIndications = [LaneIndication]() | ||
lanes.forEach { lane in | ||
if lane.validIndication != nil { | ||
usableIndications.append(lane.validIndication!) | ||
} | ||
} |
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: This can be written more succinctly and without an implicitly unwrapped optional:
var usableIndications = [LaneIndication]() | |
lanes.forEach { lane in | |
if lane.validIndication != nil { | |
usableIndications.append(lane.validIndication!) | |
} | |
} | |
let usableIndications = lanes.compactMap { lane.validIndication } |
CHANGELOG.md
Outdated
@@ -6,6 +6,9 @@ | |||
* This library requires Turf v2.0.0-alpha.3. ([#525](https://github.com/mapbox/mapbox-directions-swift/pull/525)) | |||
* The `Incident.impact` property is now an `Incident.Impact` value instead of a string. ([#519](https://github.com/mapbox/mapbox-directions-swift/pull/519)) | |||
|
|||
## v2.0.0 | |||
* Added the `Intersection.preferredApproachLanes` and `Intersection.usableLaneIndication` properties that indicate preferred lane usage. `VisualInstruction.Component.lane(indications:isUsable:)` has been renamed to `VisualInstruction.Component.lane(indications:isUsable:preferredDirection:)`. Note that comparison for `Intersection`s is now stricter around road classes, regions, and rest stops. ([#529](https://github.com/mapbox/mapbox-directions-swift/pull/529)) |
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.
* Added the `Intersection.preferredApproachLanes` and `Intersection.usableLaneIndication` properties that indicate preferred lane usage. `VisualInstruction.Component.lane(indications:isUsable:)` has been renamed to `VisualInstruction.Component.lane(indications:isUsable:preferredDirection:)`. Note that comparison for `Intersection`s is now stricter around road classes, regions, and rest stops. ([#529](https://github.com/mapbox/mapbox-directions-swift/pull/529)) | |
* Added the `Intersection.preferredApproachLanes` and `Intersection.usableLaneIndication` properties that indicate preferred lane usage. `VisualInstruction.Component.lane(indications:isUsable:)` has been renamed to `VisualInstruction.Component.lane(indications:isUsable:preferredDirection:)`. ([#529](https://github.com/mapbox/mapbox-directions-swift/pull/529)) | |
* Comparing two `Intersection`s with `==` now considers whether the `Intersection.restStop`, `Intersection.regionCode`, and `Intersection.outletMapboxStreetsRoadClass` properties are equal. ([#529](https://github.com/mapbox/mapbox-directions-swift/pull/529)) |
Sources/MapboxDirections/Lane.swift
Outdated
let str = validIndication?.descriptions.first | ||
try container.encodeIfPresent(str, forKey: .preferred) |
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: Use a more descriptive constant name or inline it into the next line:
let str = validIndication?.descriptions.first | |
try container.encodeIfPresent(str, forKey: .preferred) | |
try container.encodeIfPresent(validIndication?.descriptions.first, forKey: .preferred) |
Sources/MapboxDirections/Lane.swift
Outdated
if let str = try container.decodeIfPresent(String.self, forKey: .preferred) { | ||
validIndication = LaneIndication(descriptions: [str]) | ||
} |
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 is a bit of a workaround for the fact that LaneIndication always corresponds to an array of values in JSON, whereas we’re using the type here out of convenience for roundtripping one “description” element within such an array. To make the use of lane indication descriptions more apparent, we can name the variable a bit more verbosely:
if let str = try container.decodeIfPresent(String.self, forKey: .preferred) { | |
validIndication = LaneIndication(descriptions: [str]) | |
} | |
if let validIndicationDescription = try container.decodeIfPresent(String.self, forKey: .preferred) { | |
validIndication = LaneIndication(descriptions: [validIndicationDescription]) | |
} |
Description
This PR adds new Directions API response properties that indicate preferred lane usage #516.
Implementation
For the banner instruction component object property
active_direction
: addedpreferredDirection
argument (of typeLaneIndication?
) associated value of theVisualInstruction.Component.lane(indications:isUsable:)
enumeration case.For the lane object properties
active
andvalid_indication
: AddedIntersection.preferredApproachLanes
property (of typeIndexSet?
) modeled afterIntersection.usableApproachLanes
, addedIntersection.usableLaneIndication
property (of typeLaneIndication?
).