Skip to content
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

Enable night style when inside tunnel #1127

Merged
merged 62 commits into from
Mar 13, 2018
Merged
Show file tree
Hide file tree
Changes from 58 commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
d2581fc
Added ability to detect commuter is in a tunnel and automatically ena…
vincethecoder Feb 17, 2018
0a67127
Minor code refactor for tunnel intersection.
vincethecoder Feb 17, 2018
52564d6
Minor grammatical updates to comments on route step tunnel intersection.
vincethecoder Feb 17, 2018
2f67660
Minor text updates.
vincethecoder Feb 17, 2018
72fee83
Added comments to explain preconditions to capture all locations that…
vincethecoder Feb 17, 2018
1b0e577
Removed console logs for debug purposes.
vincethecoder Feb 17, 2018
14c5b8e
Removed resetTimeOfDayTimer method, as styles are automatically adjus…
vincethecoder Feb 17, 2018
d764421
Updated style manager to apply custom styles.
vincethecoder Feb 17, 2018
dc482f7
Set style to night style when in tunnel.
vincethecoder Feb 17, 2018
1b1152a
A few enhancement/refsctor for routestep and routecontroller discover…
vincethecoder Feb 18, 2018
19ac74d
Updated logic for setting a custom style.
vincethecoder Feb 18, 2018
f81319b
Modified delegate method to enable tunnel dark mode when commuter ent…
vincethecoder Feb 18, 2018
aadadf4
Added function to apply a specific style type.
vincethecoder Feb 18, 2018
181b428
Added a break to the for-loop for retrieving the desired style type.
vincethecoder Feb 18, 2018
5198ce4
corrected grammatical error
vincethecoder Feb 18, 2018
b2b0186
Removed extra spaces
vincethecoder Feb 18, 2018
05ef98f
Merge branch 'tunnel/dark_mode_style' of github.com:mapbox/mapbox-nav…
vincethecoder Feb 18, 2018
bb89fff
Refactored condition limit to 2
vincethecoder Feb 18, 2018
613fdfc
A few updates for when a tunnel is detected.
vincethecoder Feb 19, 2018
8caa01a
Refactored function - applyStyle()
vincethecoder Feb 19, 2018
c1d95d5
Minor text updates
vincethecoder Feb 19, 2018
efc7527
Removed break in style apply function.
vincethecoder Feb 19, 2018
256e154
Updated tunnelIntersectionBounds to check index bounds before accessi…
vincethecoder Feb 19, 2018
7686d52
Updated the check for intersection count.
vincethecoder Feb 19, 2018
b8f8a9e
Revised the absolute tunnel distance calculation to use Polyline meth…
vincethecoder Feb 19, 2018
cf306ca
Reversed variables around for distance to entrance.
vincethecoder Feb 19, 2018
c7513bf
Updated route controller delegate methods to detect when tunnel was e…
vincethecoder Feb 21, 2018
fb959c8
Updated changelog file with new delegate methods that detects when a …
vincethecoder Feb 21, 2018
6915af5
Revised logic to accommodate a single intersection on tunnel route.
vincethecoder Feb 21, 2018
e5058e6
Added logic to handle navigation through multiple tunnels on a route.
vincethecoder Feb 22, 2018
f7463d4
Removed extra lines of code to enhance readability
vincethecoder Feb 22, 2018
232baa4
Updated change log to reference link to PR
vincethecoder Feb 22, 2018
64a7897
Merge branch 'master' into tunnel/dark_mode_style
vincethecoder Feb 22, 2018
4e5356a
Minor refactor for array to use the defined typealias - IntersectionB…
vincethecoder Feb 22, 2018
be3cf6b
Updated the function to detect intersection bounds on a route.
vincethecoder Feb 26, 2018
18160da
Updates to cache and calculate the intersection bounds for detecting …
vincethecoder Feb 27, 2018
af050d4
Moved variable up to the top of the extension.
vincethecoder Feb 27, 2018
e48ad83
Made updates based on PR feedback.
vincethecoder Feb 28, 2018
3d1ef3f
Readded deleted function updateSpokenInstructionProgress(for:)
vincethecoder Feb 28, 2018
da0ab08
Updated the change log to reflect new changes.
vincethecoder Feb 28, 2018
82c8310
Updates to set intersectionDistances on RouteProgress class. Modifed …
vincethecoder Feb 28, 2018
4518a12
Updated the bounds check on currentIntersection and upcomingIntersection
vincethecoder Feb 28, 2018
49a5817
Restrict the intersection index to 0 base.
vincethecoder Mar 2, 2018
2b5cd86
Merge branch 'master' into tunnel/dark_mode_style
vincethecoder Mar 2, 2018
f25448f
Added a restriction for the minimum distance of a tunnel intersection…
vincethecoder Mar 2, 2018
59b25b3
Turf import is unnecessary.
vincethecoder Mar 2, 2018
0b93231
Removed valid adjective
vincethecoder Mar 3, 2018
f71f649
Updates to tunnel work change log file
vincethecoder Mar 5, 2018
8f24774
Added function to force refresh of appearance.
vincethecoder Mar 5, 2018
0b44f03
Added a constant to MBNavigation constants file. Removed irrelevant p…
vincethecoder Mar 6, 2018
3675974
Added a feature flag to detect when to switch styles for tunnels.
vincethecoder Mar 6, 2018
2acde88
Deleted extra white space.
vincethecoder Mar 6, 2018
241bba6
Minor refactor
vincethecoder Mar 7, 2018
4ad24d0
Updated the change logs with a note regarding new fields for current …
vincethecoder Mar 7, 2018
6ef815c
Updates to use a CountableClosedRange on the conditional check on int…
vincethecoder Mar 8, 2018
f0411b8
Updates to use a CountableClosedRange on the conditional check on int…
vincethecoder Mar 8, 2018
5ecb164
Reverted updates to use a CountableClosedRange on the conditional che…
vincethecoder Mar 8, 2018
36a374a
Updates to use a CountableClosedRange on the conditional check on int…
vincethecoder Mar 8, 2018
cb8a46b
Reverted updates to use a CountableClosedRange on the conditional che…
vincethecoder Mar 8, 2018
709231e
Added dead reckoning flag to restrict intersection calculations on th…
vincethecoder Mar 8, 2018
70c5e76
Refactored code into functions and renamed feature flag for switching…
vincethecoder Mar 8, 2018
9c878c3
Merge branch 'master' into tunnel/dark_mode_style
vincethecoder Mar 12, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,15 @@
* Exposes `setOverheadCameraView(from:along:for:)` which is useful for fitting the camera to an overhead view for the remaining route coordinates.
* Changed the heuristics needed for a the users location to unsnap from the route line. [#1110](https://github.com/mapbox/mapbox-navigation-ios/pull/1122)
* Changes `routeController(:didDiscardLocation:)` to `routeController(:shouldDiscardLocation:)`. Now if implemented, developers can choose to keep a location when RouteController deems a location unqualified. [#1095](https://github.com/mapbox/mapbox-navigation-ios/pull/1095/)
* Exposes `RouteStepProgress.intersectionDistances` of each step which can be used to determine the distance between `RouteStepProgress.currentIntersection` and `RouteStepProgress.upcomingIntersection` for unique use cases such as detecting tunnels, tolls, motorway etc. of a given route. [#1127](https://github.com/mapbox/mapbox-navigation-ios/pull/1127)

## User Interface

* Added a `NavigationMapView.localizeLabels()` method that should be called within `MGLMapViewDelegate.mapView(_:didFinishLoading:)` for standalone `NavigationMapView`s to ensure that map labels are in the correct language. [#1111](https://github.com/mapbox/mapbox-navigation-ios/pull/1122)
* The `/` delimiter is longer shown when a shield is shown on either side of the delimiter. This also removes the dependency SDWebImage. [#1046](https://github.com/mapbox/mapbox-navigation-ios/pull/1046)
* Exposes constants used for styling the route line. [#1124](https://github.com/mapbox/mapbox-navigation-ios/pull/1124/)
* Exposes `update(for:)` on `InstructionBannerView`. This is helpful for developers creating a custom user interface. [#1085](https://github.com/mapbox/mapbox-navigation-ios/pull/1085/)
* Updates to set current `NavigationViewController.styleManager.applyStyle(type:)` to `.nightStyle` when the current step intersection contains a tunnel. Also, we reset the step style when we advance to the next intersection and the upcoming intersection doesn't contain a tunnel. [#1127](https://github.com/mapbox/mapbox-navigation-ios/pull/1127)

## Voice Guidance

Expand Down
12 changes: 11 additions & 1 deletion MapboxCoreNavigation/RouteController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public protocol RouteControllerDelegate: class {
optional func routeController(_ routeController: RouteController, willRerouteFrom location: CLLocation)

/**
Called when a location has been idenetified as unqualified to navigate on.
Called when a location has been identified as unqualified to navigate on.

See `CLLocation.isQualified` for more information about what qualifies a location.

Expand Down Expand Up @@ -628,6 +628,10 @@ extension RouteController: CLLocationManagerDelegate {
let currentStepProgress = routeProgress.currentLegProgress.currentStepProgress
let currentStep = currentStepProgress.step

let intersectionDistances = routeProgress.currentLegProgress.currentStepProgress.intersectionDistances
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the user has set NavigationViewController.automaticallyChangeStyleForTunnel to false, is it necessary to perform these calculations?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bsudekum FYI

  • It shouldn't be restricted to automaticallyChangeStyleForTunnel. It should be wrapped within a isDeadReckoningEnabled flag.
  • This calculation updates the intersectionIndex - fix for issue Intersection index is always 0 #1170. @1ec5 do you need to use this fix for a task unrelated to dead reckoning?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vincethecoder I don't think this code should be intertwined with isDeadReckoningEnabled. Let's keep these as separate ideas for now.

IMO, the fix here is to move NavigationViewController.automaticallyChangeStyleForTunnel to RouteController and rename it to something more appropriate.

Copy link
Contributor Author

@vincethecoder vincethecoder Mar 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bsudekum I believe it was mentioned in one of my PR comments to separate the UI-related things in NavigationViewController from RouteController. Now, I'm totally thrown for loop 🤔
cc @1ec5 - do you recall this discussion?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, in #1127 (comment) I recommended keeping UI-related stuff out of Core Navigation. I stand by that recommendation.

let upcomingIntersectionIndex = intersectionDistances.index { $0 > currentStepProgress.distanceTraveled } ?? intersectionDistances.endIndex
currentStepProgress.intersectionIndex = upcomingIntersectionIndex > 0 ? intersectionDistances.index(before: upcomingIntersectionIndex) : 0

// Notify observers if the step’s remaining distance has changed.
if let closestCoordinate = polyline.closestCoordinate(to: location.coordinate) {
let remainingDistance = polyline.distance(from: closestCoordinate.coordinate)
Expand Down Expand Up @@ -942,6 +946,12 @@ extension RouteController: CLLocationManagerDelegate {
} else {
routeProgress.currentLegProgress.stepIndex += 1
}

if let coordinates = routeProgress.currentLegProgress.currentStep.coordinates, let intersections = routeProgress.currentLegProgress.currentStep.intersections {
let polyline = Polyline(coordinates)
let distances = intersections.map { polyline.distance(from: coordinates.first, to: $0.location) }
routeProgress.currentLegProgress.currentStepProgress.intersectionDistances = distances
}
}
}

Expand Down
24 changes: 21 additions & 3 deletions MapboxCoreNavigation/RouteProgress.swift
Original file line number Diff line number Diff line change
Expand Up @@ -388,21 +388,39 @@ open class RouteStepProgress: NSObject {
/**
The next intersection the user will travel through.

The step must contains `Intersections` for this value not be `nil`.
The step must contain `intersectionsIncludingUpcomingManeuverIntersection` otherwise this property will be `nil`.
*/
@objc public var upcomingIntersection: Intersection? {
guard let intersections = intersectionsIncludingUpcomingManeuverIntersection, intersectionIndex + 1 < intersections.endIndex else {
guard let intersections = intersectionsIncludingUpcomingManeuverIntersection, intersections.startIndex...intersections.endIndex-2 ~= intersectionIndex else {
return nil
}

return intersections[intersectionIndex]
return intersections[intersections.index(after: intersectionIndex)]
}

/**
Index representing the current intersection.
*/
@objc public var intersectionIndex: Int = 0

/**
The current intersection the user will travel through.

The step must contain `intersectionsIncludingUpcomingManeuverIntersection` otherwise this property will be `nil`.
*/
@objc public var currentIntersection: Intersection? {
guard let intersections = intersectionsIncludingUpcomingManeuverIntersection, intersections.startIndex..<intersections.endIndex ~= intersectionIndex else {
return nil
}

return intersections[intersectionIndex]
}

/**
Returns an array of the calculated distances from the current intersection to the next intersection on the current step.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this is a property, not a method, so it is set to the array rather than returning the array.

*/
@objc public var intersectionDistances = [CLLocationDistance]()

/**
The distance in meters the user is to the next intersection they will pass through.
*/
Expand Down
15 changes: 15 additions & 0 deletions MapboxNavigation/NavigationViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,11 @@ public class NavigationViewController: UIViewController {
*/
@objc public var annotatesSpokenInstructions = false

/**
A Boolean value that indicates whether the dark style should apply when a route controller enters a tunnel.
*/
@objc public var enableNightStyleInsideTunnelFeatureEnabled: Bool = false

let progressBar = ProgressBar()
var styleManager: StyleManager!

Expand Down Expand Up @@ -403,6 +408,16 @@ public class NavigationViewController: UIViewController {
let secondsRemaining = routeProgress.currentLegProgress.currentStepProgress.durationRemaining

mapViewController?.notifyDidChange(routeProgress: routeProgress, location: location, secondsRemaining: secondsRemaining)

if enableNightStyleInsideTunnelFeatureEnabled,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This property has the word enable(d) in it twice. I too am a fan of suggestion - automaticallySwitchesStyleInsideTunnels.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per our discussion, this variable will be usesNightStyleInsideTunnels.

cc @bsudekum @1ec5

let currentIntersection = routeProgress.currentLegProgress.currentStepProgress.currentIntersection,
let classes = currentIntersection.outletRoadClasses {
if classes.contains(.tunnel) {
styleManager.applyStyle(type:.nightStyle)
} else {
styleManager.timeOfDayChanged()
}
}

progressBar.setProgress(routeProgress.currentLegProgress.userHasArrivedAtWaypoint ? 1 : CGFloat(routeProgress.fractionTraveled), animated: true)
}
Expand Down
30 changes: 21 additions & 9 deletions MapboxNavigation/StyleManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,22 @@ open class StyleManager: NSObject {
resetTimeOfDayTimer()
}

func applyStyle(type styleType: StyleType) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this func be combined with the applyStyle below?

Copy link
Contributor Author

@vincethecoder vincethecoder Feb 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I considered it. However, this func applyStyle(type:) is cleaner, since it requires the styleType. The other func applyStyle scope is getting a bit bigger and complex with more conditions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UPDATES: I also refactored applyStyle so that we have a function applyStyle(type:) to contain styleType related logic.

guard currentStyleType != styleType else { return }

NSObject.cancelPreviousPerformRequests(withTarget: self, selector: #selector(timeOfDayChanged), object: nil)

for style in styles {
if style.styleType == styleType {
style.apply()
currentStyleType = styleType
delegate?.styleManager?(self, didApply: style)
}
}

forceRefreshAppearance()
}

func applyStyle() {
guard let location = delegate?.locationFor(styleManager: self) else {
// We can't calculate sunset or sunrise w/o a location so just apply the first style
Expand All @@ -116,14 +132,7 @@ open class StyleManager: NSObject {
}

let styleTypeForTimeOfDay = styleType(for: location)

for style in styles {
if style.styleType == styleTypeForTimeOfDay {
style.apply()
currentStyleType = style.styleType
delegate?.styleManager?(self, didApply: style)
}
}
applyStyle(type: styleTypeForTimeOfDay)
}

func styleType(for location: CLLocation) -> StyleType {
Expand All @@ -144,7 +153,10 @@ open class StyleManager: NSObject {
}

applyStyle()

forceRefreshAppearance()
}

func forceRefreshAppearance() {
for window in UIApplication.shared.windows {
for view in window.subviews {
view.removeFromSuperview()
Expand Down