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

Display shields for destinations in turn banner #353

Merged
merged 2 commits into from
Jul 10, 2017

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Jul 9, 2017

This PR significantly increases the frequency of route shields in the turn banner. Before, the turn banner only considered the route step’s code for displaying a shield, but that would only be relevant when merging onto a freeway and when approaching an off-ramp that happens to be part of a route. Now the turn banner also shows a shield based on destinationCode, which is populated when approaching an on-ramp and when approaching an off-ramp that isn’t part of a route.

Additionally, we try to treat the route step’s destination as a destinationCode in case it begins with a route number. This is a workaround for Project-OSRM/osrm-backend#3304 to fix #320.

RouteManeuverViewController is now responsible for fetching route shield images it needs to display. This makes it easier to avoid redundant requests to Wikimedia Commons: we only issue a request when the textual representation of the shield changes.

Guadalupe Freeway Park Avenue

/cc @bsudekum @frederoni @ericrwolfe @willwhite

RouteManeuverViewController is now responsible for fetching route shield images it needs to display. If the step has no code, try displaying a shield based on its destination code or, failing that, its destination.
@1ec5 1ec5 added bug Something isn’t working op-ex Refactoring, Tech Debt or any other operational excellence work. labels Jul 9, 2017
@1ec5 1ec5 added this to the v0.5.0 milestone Jul 9, 2017
@1ec5 1ec5 self-assigned this Jul 9, 2017
@1ec5 1ec5 requested review from frederoni and bsudekum July 9, 2017 10:28
@@ -107,7 +102,6 @@ class RouteMapViewController: UIViewController {

override func viewWillDisappear(_ animated: Bool) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Let's drop this override now when it's only calling super.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in af5fe5c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn’t working op-ex Refactoring, Tech Debt or any other operational excellence work.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display shields for destinations in turn banner
2 participants