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

"Continue onto" --> "Continue to stay on" #142

Merged
merged 5 commits into from
Sep 6, 2017
Merged

"Continue onto" --> "Continue to stay on" #142

merged 5 commits into from
Sep 6, 2017

Conversation

lyzidiamond
Copy link
Contributor

@lyzidiamond lyzidiamond commented Sep 6, 2017

Issue

Closes #41 -- rephrases instruction from "continue onto" to "continue to stay on" when continuing. Note that I added a special condition for exits.

Tasklist

  • update language in en.json
  • Add changelog entry
  • Test with osrm-frontend (?)
  • Review by @willwhite and/or @bsudekum and/or @1ec5? (not sure who to tag here)

Copy link
Contributor

@mcwhittemore mcwhittemore left a comment

Choose a reason for hiding this comment

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

This is looking good to me. I'm a bit intrigued to know if we need both to stay on and on from a reducing chattiness pov.

@lyzidiamond - can you write up the changlog and also explain continue.default.exit a bit. I'm not seeing how continue and exiting are the same thing.

@lyzidiamond
Copy link
Contributor Author

@mcwhittemore Per #41 (comment) + the OSRM docs, "Continue" means "Turn in direction of modifier to stay on the same road." With that in mind, consider these scenarios:

  • You are driving on Main Street. In order to stay on Main Street, you need to turn left. Prior to this PR, you would get: "Continue left onto Main Street." But you're already on Main Street. With this PR, you would get: "Continue left to stay on Main Street." It sounds like you're asking if we can drop "to stay", which leaves: "Continue left on Main Street." This sounds confusing to me -- it's ambiguous about whether you're staying on the same street or turning onto a new street.

  • The one place this doesn't make sense is when taking a highway exit. Per the tests, going from a ramp onto a road is considered a continue maneuver, but you are actually going from one path (the ramp) to another (the road). In that case, instead of hearing "Continue straight to stay on Main Street" (because you're not yet on Main Street when you get to that maneuver) you will hear "Continue straight onto Main Street" which makes more sense.

Copy link
Contributor

@mcwhittemore mcwhittemore left a comment

Choose a reason for hiding this comment

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

Thanks for the clarification @lyzidiamond. Lets merge and release this.

@mcwhittemore mcwhittemore merged commit fa5014d into master Sep 6, 2017
@mcwhittemore mcwhittemore deleted the stay-on branch September 6, 2017 22:29
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.

2 participants