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

Expose listeners in the NavigationView #614

Merged
merged 29 commits into from
Dec 20, 2017
Merged

Conversation

devotaaabel
Copy link
Contributor

@devotaaabel devotaaabel commented Dec 18, 2017

NavigationListener and RouteListener were created with methods for listening for navigation/routing events and for influencing reroutes. NavigationViewListener was replaced by OnNavigationReadyCallback, and onNavigationFinished was moved to NavigationListener. Eventually, the plan is to break up these two listeners into individual listeners for each method, so that people don't have to implement more methods than necessary (i.e. see empty onNavigationRunning method in NavigationActivity).

TODO:

NavigationListener

  • void onCancelNavigation()
  • void onNavigationFinished()
  • void onNavigationRunning()

RouteListener

  • boolean allowRerouteFrom(Point offRoutePoint)
  • void onOffRoute(Point offRoutePoint)
  • void onRerouteAlong(DirectionsRoute directionsRoute)
  • void onFailedReroute()

FeedbackListener

  • onFeedbackOpened()
  • onFeedbackCancelled()
  • onFeedbackSent(FeedbackItem item)

cc @ericrwolfe

Devota Aabel and others added 22 commits December 13, 2017 13:51
 - Started marking TODOs for dispatcher firing points
@danesfeder
Copy link
Contributor

Noting that this PR will expose the ability to add ProgressChangeListeners and MilestoneEventListeners directly to the MapboxNavigation initialized by NavigationView. This way they can listen to progress / milestones updates as needed

@danesfeder
Copy link
Contributor

danesfeder commented Dec 19, 2017

Lookin great and verified all listeners are firing correctly- fired reroute listeners using fake GPS updates provided by Pathfinder. @ericrwolfe if you don't have any comments concerns I say let's get this merged.

*
* @since 0.8.0
*/
void onFailedReroute();
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to return the error here?

Copy link
Contributor

@danesfeder danesfeder left a comment

Choose a reason for hiding this comment

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

Updated the failed reroute listener to provide an error message now. @devotaaabel great PR - feel free to merge once CI passes ✅

@devotaaabel devotaaabel merged commit 630b38b into master Dec 20, 2017
@devotaaabel devotaaabel deleted the devota-expose-listeners branch December 20, 2017 15:52
@danesfeder danesfeder mentioned this pull request Dec 20, 2017
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants