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

Routing #716

Merged
merged 1 commit into from Feb 16, 2015
Merged

Routing #716

merged 1 commit into from Feb 16, 2015

Conversation

systemed
Copy link
Contributor

@systemed systemed commented Mar 8, 2014

This is a basic routing interface for osm.org implemented in JavaScript. Routing is provided by external services (in this commit, OSRM, GraphHopper, and MapQuest Open).

The initial aim is to provide A-B routing functionality with a coherent UI. There is much more that could be implemented, of course, but the focus has been on the simplest possible service for now.

There are two obvious omissions:

  • Routes do not have permalinks and the URL history is never changed.
  • Directions are not translated.

Both of these are some way outside my knowledge / comfort zone and I suspect others would be more efficient at coding them! Translation is expected to be done by the external service, so should be largely a matter of the routing engine passing the current locale.

Otherwise this should largely be ready. Comments/suggestions within the existing scope welcome: "wouldn't it be nice if it did this..." requests are probably best left for after an initial launch.

Thanks hugely to @apmon, @danstowell and @karussell for patches and suggestions so far.

@RobJN
Copy link

RobJN commented Mar 8, 2014

Hi Richard and other contributors - great work on this. Looking forward to it going live.

Is this the same thing that is running on http://jsrouting.apis.dev.openstreetmap.org/ ? If yes, the I've been testing it and have identified a small bug:

  • Find a route.
  • Change the destination address (but do not click go)
  • On map control (pan) the destination marker moves to the new address but the route is not updated.

Where should I file issues/bugs. Here on on your guthub page?

@tomhughes
Copy link
Member

Please don't file separate bugs (other than this PR) for routing against this tree - only code which is merged should get bugs here.

@mvl22
Copy link

mvl22 commented Mar 9, 2014

The current implementation seems to imply that only those organisations with the resources to run worldwide routing are eligible to be included. This means that smaller organisations doing interesting/pioneering routing work or providing more specialist routing which might not be expected to be worldwide (e.g. horse routing, skiing routing?) are excluded.

Yet one of the key benefits of OSM is that it facilitates specialist routing beyond that found on well-known large consumer-facing company sites, something osm.org should perhaps be demonstrating.

Therefore has the possibility that interested organisations might be able to provide a GeoJSON file covering their routeable area (plus driver engine code to translate results, and a service level guarantee), and the drop-down GUI modified so that this appears dynamically as an option when the user is suitably zoomed in, been considered?

@systemed
Copy link
Contributor Author

systemed commented Mar 9, 2014

The implementation has been built to reflect the "New Routing Services Guidelines" at http://wiki.openstreetmap.org/wiki/Strategic_working_group/New_Routing_services_Guidelines_Proposal , which are an adaptation of the widely-accepted and long-standing http://wiki.openstreetmap.org/wiki/Strategic_working_group/New_Tile_Layer_Guidelines . I guess if someone could devise a non-obtrusive UI that permitted localised routers to be included, and submitted a good-quality patch to do this, then it would be up to the relevant WG to consider revisiting these guidelines (SWG having died a largely unlamented death).

@pnorman
Copy link
Contributor

pnorman commented Mar 9, 2014

I can't see regional routing being part of the initial release - there's too many issues to work out and it would delay worldwide routing.

@tomhughes
Copy link
Member

Yes the key target right now is to get what we have up to scratch and get it merged before we start worrying about adding other engines.

Long term we will need some sort of guidelines - we really don't want to be supporting hundreds of different engines, especially as adding extra routing engines is significantly more complicated than adding extra tile layers.

continue_on: "Weiter auf "
slight_right: "Rechts halten auf "
turn_right: "Rechts abbiegen auf "
sharp_right: "Hart rechts auf "

Choose a reason for hiding this comment

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

This is an odd translation. The proper phrase is "scharf rechts auf".

Copy link
Member

Choose a reason for hiding this comment

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

Well we shouldn't be adding strings to any translations (other than English of course) anyway - translations are done via TranslateWiki!

Copy link
Member

Choose a reason for hiding this comment

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

I added those strings to de.yml in order to test the i18n functions of the interface and check that all strings are translated.

But yes, "Scharf rechts auf" is a better translation. There are a couple of instructions I didn't know how best to translate to German. As we can always fix things via translatewiki later, I didn't put too much effort into finding the optimal translations.

@samanpwbb
Copy link
Member

There are two big usability issues that should be addressed:

Form design:

screen shot 2014-03-10 at 12 32 15 pm

Both "From" and "To" fields need to be placed before the "Go" action. Right now, it feels like I'm supposed to enter a "From" location, click go, then enter a "To" location.

Trap:

  1. Search for a location, click "go".
  2. Click "get directions"
  3. Things break, don't work as expected.

Solutions: a) Hide 'get directions' button as soon as you enter anything in search bar. b) automatically populate first routing field with result from search

@tomhughes
Copy link
Member

Thanks for having a look at the design @samanpwbb . I was wondering if you had any thoughts on the moving of "Where am I?" above the search box and the addition of the routing activation link there, or if you had any ideas of how better to handle that because I'm not a great fan of the current solution...

@systemed
Copy link
Contributor Author

Good spots - thanks!

First issue - I've moved the 'Go' button in systemed@9d8ab7d (and may I be struck down for using !important).

Second issue - populating the 'From' field with the 'Search' value is an appealing solution, but has the complication that the fields have different behaviour. 'Search' brings up all possible Nominatim results, whereas 'From' just chooses the top one. So if you were to search for Boston, choose the one in Lincolnshire, then click 'Get directions' and have the text copied across, your route would probably start with the one in Massachusetts.

I've discussed with @tomhughes the notion that the search interface could be ultimately rewritten in JS rather than going through Rails as at present, which would make integrating the two easier, but that's not a "right now" option. For now, populating 'From' with the lat/long of the chosen 'Search' result would work, though I'm not familiar enough with the internals to do that without breaking large amounts of stuff.

@vshcherb
Copy link

I would like to thank for this great piece of functionality and suggest some directions to improve

  1. Double click could select on the map in "Get directions" mode .
  2. Context menu (right click) on the map could give options like directions from and to, what is very handy
  3. When you select address in Get directions from , you need to click "Enter" twice and also you don't see any search options

@jfirebaugh
Copy link
Member

I've sketched out an alternate UI for a directions flow.

image

In this variant, clicks on the map result in a "search" operation for the clicked coordinate. The results page for coordinate searches is pared down to a single nominatim result and buttons to switch to a directions search starting or ending at the chosen coordinate.

Not implemented yet, but how I envision subsequent interactions to work is that once you click "to here" or "from here", the search sidebar disappears, leaving just the directions search form. At that point, clicks on the map fill in the second search parameter and submit the form.

Code in my fork.

@danstowell
Copy link
Contributor

@jfirebaugh I like the idea workflow-wise. Visually, my first reaction was:

"Hmm, so there are 3 buttons shown, or maybe they're tabs: 'get directions', 'to here', and 'from here'. I wonder why the first of them is selected."

Notice the visual similarity with Edit/History/Export at the top of your screenshot, which are indeed buttons. I suggest taking the border off "Get directions", and maybe even making the two buttons have similar styling as the top row actions.

@systemed
Copy link
Contributor Author

@jfirebaugh Thanks for that - have now had the chance to have a look!

I like the "to here"/"from here" buttons with the geocoding results; that works well. I also like the idea of single-clicking the map bringing up contextual UI in the LH panel - this might be especially relevant to @tomhughes' work at https://github.com/tomhughes/openstreetmap-website/tree/overpass .

That said, I think we do still need a visible "Get directions" link of some sort, rather than completely hiding it behind geocoding results, to make it obvious to the user that OSM does routing. Google, Bing, Yahoo and Mapquest all have a button or link saying "Get Directions" - well, strictly speaking Bing says "Directio...", but I guess it's truncating the text because my browser window is only 1850px wide or something.

I take @tomhughes' point that the current superscript-ish links aren't the most elegant in the world, but OTOH they're less clutter than a new button would be (and there's a danger that we're reaching Peak Button on the right-hand side).

Changing the colours to grey->black (as in the GPS Traces, User Diaries &c. links) would make them a little more consistent with the rest of the UI, but we'd then have grey-on-grey text. Suggestions welcome. Patches even more welcome!

On balance, I'd suggest we go forward with the current routing UI; look at implementing the "to here"/"from here" buttons as a v1.1 (@apmon's idea of splitting the dropdown into "mode" and "provider" would also be worth discussing at that stage); and consider single-click on the map as a wider enhancement to the osm.org UI in conjunction with the Overpass/Inspector branch.

@Zverik
Copy link
Contributor

Zverik commented Mar 31, 2014

From #osm-dev, I propose three improvements, keeping the "get directions" tiny link:

  1. open directions panel with /directions url
  2. when opening the panel (either by clicking "get directions" link or following the url, prefill "from" field with the center of the map
  3. add "reverse route" button

@tomhughes
Copy link
Member

We need to decide where we're going with this, and how it is going to get merged...

Personally I like @jfirebaugh's approach, and I don't really want to launch as it is if we're then going to quickly redesign it. I'm not saying it might not need some tweaking for improved discoverability as @systemed suggests but as a general concept I prefer it.

As I see it as this point we have three options:

  • Decide we prefer the current UI and launch as is.
  • Decide we prefer the new UI but will launch as is while waiting for it to be finished.
  • Decide we prefer the new UI and find a way to (hopefully quickly) drive it to completion.

If we can make a decision on the basic strategy we want to follow then we can work to drive this forward and get it merged.

@danstowell
Copy link
Contributor

My personal opinion: definitely launch as-is. I like @jfirebaugh's approach workflow-wise but not UI-wise: discoverability issue, as mentioned; plus adding the "get directions" block to every search result would reduce the number of results that fit on a page quite dramatically. So - I don't think that either of the visual designs hits perfection, but I don't think we're going to hit perfection by waiting here...

If I'm forced to say which of your three options to plump for then it's the first, "prefer current UI".

An early launch of the feature will draw lots of comments (of course) which will be food for thought for a future rethink of the interaction design of it. So IMHO there's very little cost-without-benefit to going ahead.

@tomhughes
Copy link
Member

There are two major costs (assuming we then change) one of which is forcing the community to relearn how to use it, and the other of which is the cost to me of having to do two big reviews instead of one.

@RobJN
Copy link

RobJN commented Apr 24, 2014

Clicks on the map bringing up search results in the left pane would be new behaviour on osm.org. I'm open to giving it a go myself (if there's a version running on a dev server), however I'm just wondering what the rest of the osm community will think. I can see many people raising the concern that accidental clicks (when wanting to pan the map) keeps brining the side pane up.

Whatever you choose I would recommend a bit of comms on the "talk" mailing list so that it doesn't come as a surprise.

@pnorman
Copy link
Contributor

pnorman commented Apr 25, 2014

Whatever you choose I would recommend a bit of comms on the "talk" mailing list so that it doesn't come as a surprise.

I'll be doing some outreach, but takes time itself, so I'd like it to be a bit more solid which branch.

@tomhughes
Copy link
Member

I've put up a live demo of @jfirebaugh's branch at http://jsrouting2.apis.dev.openstreetmap.org/.

@Zverik
Copy link
Contributor

Zverik commented Apr 25, 2014

Click → «From here» → Another click → Expected: a route from the first point to the selected; received: another suggestion «from here / to here».

Actually, it doesn't work at all. «to here / from here» buttons clear both fields.

@Zverik
Copy link
Contributor

Zverik commented Apr 25, 2014

Also, all markers are "marker red" instead of "routing green/red"; they are not draggable; routes are not displayed. In other words, nothing works except for the "from here/to here" dialog, which I like.

@tomhughes
Copy link
Member

@Zverik We know it's in no way complete - it's just a quick prototype to show the outline of the concept.

@simonpoole
Copy link
Contributor

I think there maybe a simple way to use jfirebaugh's concept and at the same time fulfill RichardFs point that we need to clearly show that you can get a route from the UI So what about replacing the the "Search" placeholder with "Search and Directions". Small eye sore nit: I would make the selector fo the routing engines align with the Start/End fields and make it full size from a height pov.

@apmon
Copy link
Member

apmon commented Apr 25, 2014

There are some nice ideas in jfirebaugh's concept and some aspects I like less.

E.g. I am not sure if I like that clicking on the map immediately sets a marker. That seems like a behaviour users wouldn't expect and is not consistent with how other maps like google's or bing appear to behave. Also, as others have mentioned, the non discoverability of the routing feature might be an issue with what is currently in jfirebaugh's branch.

On the other hand, I like the prominent directions buttons on the search result, as they are clear and intuitive. A bit problematic might be that you can only use them currently for either from or to and not both destinations. So there might be a need for some additional tweeking of the interface.

So my suggestion would be to use RichardF's branch and simply add the get directions buttons to the normal search result.

However, as this seems like a mostly incremental addition, rather than a rewrite, I am not sure that should or needs to block the deployment of RichardF's current work? I would therefore probably vote for "we prefer the current UI and launch as is".

@jfirebaugh
Copy link
Member

I will be devoting some time this week to working on this.

@jfirebaugh
Copy link
Member

PR sent to @systemed's fork: https://github.com/systemed/openstreetmap-website/pull/21

I'm not sure precisely how the history should work when the user updates the route via the various means of doing so (dragging a marker, changing the engine, editing the text field and resubmitting). Right now switching to the directions form pushes a single history entry (/directions) and then all other changes use replaceState. That feels pretty ok, although maybe resubmitting the form should push a separate history entry, like search does.

@systemed
Copy link
Contributor Author

This looks great at first glance - will have a proper look asap.

@danstowell
Copy link
Contributor

Hi - is there anything more we can do to help on getting this live? I think it will catalyse lots of improvements to our routing data so I'm quite hopeful for it!

@tomhughes
Copy link
Member

The GO button is on the next line because it's a separate div. That div probably needs removing and the drop and button putting in a single div class=line (not that I like a class with a name that generic...) or something. The align=right on that and other stuff should be CSS as well.

@danstowell
Copy link
Contributor

Re the GO button: a quick HTML hack shows me that simply removing that div wrapper almost works, except that I also had to edit the CSS for input[type="submit"]and decrease min-width from 120px to 80px. Then, they sit nicely on a line together with no div wrapper needed. Tested in firefox at all zoom levels including the ones which trigger the small stylesheet. But I didn't check who else uses that CSS rule to check for unintended consequences.

@danstowell
Copy link
Contributor

Oops just found another glitch: if you shrink your screen so that the "small" stylesheet is in operation, then the routing uses the wrong search text, because it grabs the text from the copy of the form which is shown to big-screeners!

This will fail silently if there's no text in the big-screener form: it simply looks like the small-screen version silently decides not to give you any routing. But if you start in big-screen mode, enter text, then shrink to small-screen mode and try a different search, you'll see it return results for the old one.

I believe the reason is related to the fact that the site outputs two separate HTML blocks for the form in the two modes. I'd imagine the JS didn't expect that.

@zerebubuth
Copy link
Contributor

Ah yes, I'd seen the two separate copies of the sidebar content tree. How annoying. I'll fix that.

@zerebubuth
Copy link
Contributor

@danstowell: I think I fixed this in 1b2bfb6 - could you have another go and let me know if it's still an issue for you, please?

@danstowell
Copy link
Contributor

@zerebubuth yes, fix works here, thanks!

@danstowell
Copy link
Contributor

There's a typo in 1b2bfb6 however which prevents the directions close-box from working. Fixed in PR 32

@systemed
Copy link
Contributor Author

systemed commented Feb 2, 2015

This is looking great now, thanks to @zerebubuth, @tomhughes and @danstowell. Are there any blockers remaining?

@tomhughes
Copy link
Member

There's a couple of things I want to look at still - can hopefully get them done this week.

Other than that, should we speak to the operators of the engines we are using before we go live?

@karussell
Copy link
Contributor

From the GraphHopper operator +1 ;)

@DennisOSRM
Copy link

+1

@pnorman
Copy link
Contributor

pnorman commented Feb 2, 2015

I believe we've either gotten explicit okays or we're meeting the terms of service for every routing engine we're launching with. A note in advance to those who haven't commented here would probably be good, more to make them aware if they want to do anything for their own communications channels than for load reasons.

Note: Not writing this as a MQ employee.

@HolgerJeromin
Copy link
Contributor

surprisingly usable from mobile :-)
Problems:
if a routing is presented i can drag both markers.
but on mobile the direction list is closed, so the position of my finger at the starting ondrag is not at the point of the ontouch.

If i enter the start point name and push enter on the soft keyboard the div with start and end is closed. I have to reopen it. When i leave the input (without enter) everything works nice.

@Zverik
Copy link
Contributor

Zverik commented Feb 8, 2015

Are we discussing jsrouting2 here? Follow this link to a route, then try to get completely new directions for another city.

@tomhughes
Copy link
Member

No, http://jsrouting.apis.dev.openstreetmap.org/ is the current code.

@Zverik
Copy link
Contributor

Zverik commented Feb 8, 2015

Ah, sorry. That one works fine.

jsrouting: don't close the smallscreen header until both geocodes entered
@danstowell
Copy link
Contributor

ok, fixed @HolgerJeromin 's second issue with that latest commit. I can't reproduce the first issue (neither on my shrunken web browser, nor on my firefoxos phone). It's possible that the commit also coincidentally affected the first one as well (i.e. the one about dragging markers with your finger)...

@HolgerJeromin
Copy link
Contributor

The fist issue is still valid for my opera mobile classic (old presto engine). While dragging the markers the routing is updated live. So the number of entries in the direction view is changing. Thus the map view is moved under the finger. But this is no real blocker for me.

@openstreetmap-mirror openstreetmap-mirror merged commit f576984 into openstreetmap:master Feb 16, 2015
@danstowell
Copy link
Contributor

:D

@ximex
Copy link

ximex commented Feb 16, 2015

👍

@matkoniecz
Copy link
Contributor

Thanks to all involved!

@ximex
Copy link

ximex commented Feb 16, 2015

are there intermediate stops planed to support?
and where should i leave an issue if i found some strange routing with GraphHopper Cycle?

@tomhughes
Copy link
Member

Well you should start by checking that the data is right - the main point of having routing there is to find errors in the data.

If you think the data is right and the engine is wrong then you should contact the Graphopper developers.

@karussell
Copy link
Contributor

Nice! Thanks to @tomhughes & @systemed and others for all the coding work!

@devemux86
Copy link

👍

@JeanFred
Copy link

This is great, thanks to everyone involved.

@openstreetmap openstreetmap locked and limited conversation to collaborators Feb 17, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet