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

Hide portions of the route that the user has passed #2377

Merged
merged 27 commits into from
Jun 25, 2020

Conversation

captainbarbosa
Copy link
Contributor

@captainbarbosa captainbarbosa commented May 28, 2020

Fixes #1307
Fixes #1434

⚠️ Breaking changes expected to land in v1.0

Adds the option to fade the route line as the user navigates along a route. A custom fade out color can also be specified. the default is clear, to give the appearance of a "vanishing" line as the puck travels along a route.

vanish-route

This also fixes #1434 because I've broken out the main route / alternate routes into different casing layers:

Screen Shot 2020-06-17 at 5 19 17 PM

The vanishing route color can also be customized by overriding NavigationMapView.appearance().vanishingRouteColor:

Screen Shot 2020-06-18 at 6 53 05 PM

Within this change, NavigationMapViewDelegate.navigationMapView(_:routeStyleLayerWithIdentifier:source:) and NavigationMapViewDelegate.navigationMapView(_:routeCasingStyleLayerWithIdentifier:source:) are removed in favor of four new delegate methods to customize the route styling:

  • NavigationMapViewDelegate.navigationMapView(_:mainRouteStyleLayerWithIdentifier:source:) to style the main route.
  • NavigationMapViewDelegate.navigationMapView(_:mainRouteCasingStyleLayerWithIdentifier:source:) to style the casing of the main route.
  • NavigationMapViewDelegate.navigationMapView(_:alternativeRouteStyleLayerWithIdentifier:source:) to style alternative routes.
  • NavigationMapViewDelegate.navigationMapView(_:alternativeRouteCasingStyleLayerWithIdentifier:source:) to style the casing of alternative routes.

cc @1ec5 @fabian-guerra

@captainbarbosa captainbarbosa added feature New feature request. UI Work related to visual components, Android Auto, Camera, 3D, voice, etc. labels May 28, 2020
@captainbarbosa captainbarbosa self-assigned this May 28, 2020
MapboxNavigation/NavigationMapView.swift Outdated Show resolved Hide resolved
MapboxNavigation/NavigationMapView.swift Outdated Show resolved Hide resolved
@captainbarbosa
Copy link
Contributor Author

I'm working on nailing down the implementation in a side project first as a minimal test case before porting back to here - stay tuned.

@captainbarbosa
Copy link
Contributor Author

This is the current state of the prototype - the gradients need to be fine-tuned still but I'm hopeful - seems like just a matter of nailing down the right numbers for the stops.

@captainbarbosa
Copy link
Contributor Author

captainbarbosa commented Jun 10, 2020

Looking better - here's a fake route toggling between traffic styled with two difference sources and layers. The difference is subtle, and even more indistinguishable when you end up calculating the gradient stops dictionary with CGFloat's nextUp/nextDown as @1ec5 suggested in #2377 (comment).

One layer contains individual congestion segments styled with an MGL_MATCH expression. The other layer is a singular line feature styled with a line gradient. Prototype for reference.

The next step is to dynamically transform the stops dictionary the gradient is using to simulate the disappearance of traveled-over parts of the line.

@LukasPaczos
Copy link

/cc @cafesilencio for visibility, in case there are any improvements that iOS could borrow from Android or vice versa

@captainbarbosa
Copy link
Contributor Author

I got the gradient to fade today over time. Here's that same fake route line fading from 0-100% on a timer, leaving a white trail in its path:

Next up, I'll work on integrating the code for this prototype into this PR.

@captainbarbosa
Copy link
Contributor Author

As of 25fcd74 the route line's traffic is now expressed with a single line gradient. Next, I need to work on how to only display this traffic only for the main route and not the alternate routes:

Screen Shot 2020-06-15 at 5 35 01 PM

@captainbarbosa
Copy link
Contributor Author

This might also take care of #1434...

@captainbarbosa
Copy link
Contributor Author

Things are looking good (see new updated PR description). There's some additional refactoring I need to do before this PR is ready for review, but the visual implementation looks about right.

@captainbarbosa captainbarbosa marked this pull request as ready for review June 23, 2020 03:56
@captainbarbosa captainbarbosa changed the base branch from master to release-v1.0-pre-registry June 23, 2020 03:57
@captainbarbosa
Copy link
Contributor Author

Opt-in setting moving in #2418 - that should be merged into this PR first.

@captainbarbosa
Copy link
Contributor Author

#2418 has merged into this PR so now this feature is opt-in. I did some additional manual testing today to make sure the vanishing feature still works as expected when traffic isn’t enabled 👍

Copy link
Contributor

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

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

It’s cool to see this come together and with less churn than I had expected. Thanks for including updated screenshots at the top; that gives more confidence that the feature works as expected in the absence of UI tests for this feature.

CHANGELOG.md Show resolved Hide resolved
MapboxNavigation/DayStyle.swift Outdated Show resolved Hide resolved
MapboxNavigation/NavigationMapView.swift Outdated Show resolved Hide resolved
MapboxNavigation/NavigationMapView.swift Outdated Show resolved Hide resolved
MapboxNavigation/NavigationMapView.swift Show resolved Hide resolved
MapboxNavigation/NavigationMapView.swift Show resolved Hide resolved
MapboxNavigation/NavigationMapView.swift Show resolved Hide resolved
MapboxNavigation/NavigationMapView.swift Outdated Show resolved Hide resolved
MapboxNavigation/NavigationMapView.swift Outdated Show resolved Hide resolved
MapboxNavigation/NavigationMapViewDelegate.swift Outdated Show resolved Hide resolved
@captainbarbosa
Copy link
Contributor Author

I've integrated all feedback, minus this weird behavior which I've noted for later and is not a blocker for this PR. The CI failure for the ci/circleci: Xcode_10.2_iOS_12.2_CP_install job is expected (caused by #2394), so we're good to merge assuming everything else passes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards incompatible changes that break backwards compatibility of public API feature New feature request. UI Work related to visual components, Android Auto, Camera, 3D, voice, etc.
Projects
None yet
4 participants