GNOME Bugzilla – Bug 777559
Add shortcut to reverse a route
Last modified: 2017-02-16 20:31:04 UTC
When doing a route search it could be convenient with a button that reverses the list. I think one of the interview persons in Andreas' public transportation survey had one such usecase (search for a route to go to a pub in town, and then change direction and set a late time to see when the last bus home goes), but now I couldn't find it. It is possible now also, by dragging the points, but it is a bit awkward, and especially with extra stops inbetween. One option might be to have a "switch direction" button next to the last route entry (where there is a + button at the top and a - button for intermediate entries to add and remove them).
Created attachment 344004 [details] [review] routing: Add button to quickly reverse a route search Adds a button to reverse a route search.
NOTE: the above patch is kinda WIP, it uses a "temporary icon" (I think it comes from Evolution, it sports up and down arrows, and seemed good enough to "prove the point"). I cooked this up more as a quick evening proof-of-concept. And it probably makes sense to revise this after landing the transit-routing branch, as this will probably need a bit of rebasing after that (and ofcourse a more permanent solution with the icon would be needed).
Created attachment 344005 [details] [review] routing: Add button to quickly reverse a route search Adds a button to reverse a route search.
Created attachment 344025 [details] screenshot
Seems to work well. Looks good to me, UI-wise.
Review of attachment 344005 [details] [review]: Nice! Some questions ::: src/routeEntry.js @@ +77,3 @@ this._icon.icon_name = 'maps-point-end-symbolic'; + /* TODO: only use this icon temporarily */ + this._buttonImage.icon_name = 'mail-send-receive-symbolic'; How temporarily, should this be committed to master? What is the plan for this icon? ::: src/sidebar.js @@ +146,3 @@ }); + } else if (type === RouteEntry.Type.TO) { + routeEntry.button.connect('clicked', function() { could this be routeEntry.button.connect('clicked', this._reverseRoute.bind(this)); Is it prettier? @@ -248,0 +252,12 @@ + _reverseRoutePoints: function() { + let query = Application.routeService.query; + let points = query.points; ... 9 more ... We do not have the array.reverse() javascript function ?
(In reply to Jonas Danielsson from comment #6) > Review of attachment 344005 [details] [review] [review]: > > Nice! > > Some questions > > ::: src/routeEntry.js > @@ +77,3 @@ > this._icon.icon_name = 'maps-point-end-symbolic'; > + /* TODO: only use this icon temporarily */ > + this._buttonImage.icon_name = 'mail-send-receive-symbolic'; > > How temporarily, should this be committed to master? What is the plan for > this icon? > Not sure, the icon actually comes from adwaita-icon-theme when I took a closer look. Though, I was kinda thinking about waiting with this patch, since it would need some rebase gymnastics in the transit-routing branch (as the route query was moved out from routeService.js, which is for GraphHopper, to application.js to be used by the OpenTripPlanner module as well. > ::: src/sidebar.js > @@ +146,3 @@ > }); > + } else if (type === RouteEntry.Type.TO) { > + routeEntry.button.connect('clicked', function() { > > could this be routeEntry.button.connect('clicked', > this._reverseRoute.bind(this)); > > Is it prettier? > Makes sense. > @@ -248,0 +252,12 @@ > + _reverseRoutePoints: function() { > + let query = Application.routeService.query; > + let points = query.points; > ... 9 more ... > > We do not have the array.reverse() javascript function ? Yes, however in RouteQuery, the objects in the "points" array are RouteQueryPoints, on which the "place" parameter is connected a notification handler. Something similar is done in _reorderRoutePoints() which is called when dragging an entry. So, in essense the place properties on the RouteQuery objects are exchanged.
(In reply to Jonas Danielsson from comment #6) > ::: src/routeEntry.js > @@ +77,3 @@ > this._icon.icon_name = 'maps-point-end-symbolic'; > + /* TODO: only use this icon temporarily */ > + this._buttonImage.icon_name = 'mail-send-receive-symbolic'; > > How temporarily, should this be committed to master? What is the plan for > this icon? Yeah, this icon name seems a bit risky under other icon themes. Adding Jimmac and Lapo to cc. What's the best approach here?
Maybe it's easiest to ship our own copy of the icon under the name route-reverse-symbolic
(In reply to Andreas Nilsson from comment #9) > Maybe it's easiest to ship our own copy of the icon under the name > route-reverse-symbolic Yeah, maybe that is the easiest option.
Created attachment 345731 [details] icon this is just the mail icon renamed.
Created attachment 345880 [details] [review] Add icon for reversing a route This icon is just a copy of the "mail send/receive" icon from the Adwaita icon theme.
Created attachment 345881 [details] [review] routing: Add button to quickly reverse a route search Adds a button to reverse a route search.
Created attachment 345882 [details] [review] routing: Add button to quickly reverse a route search Adds a button to reverse a route search.
Added a patch installing the copy of the icon. And rebased the main patch to the new "post transit merge" era :) Also added a comment about the rational behind the array shuffling when reversing.
Review of attachment 345880 [details] [review]: yes
Review of attachment 345882 [details] [review]: lgtm ::: src/routeEntry.js @@ +76,3 @@ case Type.TO: this._icon.icon_name = 'maps-point-end-symbolic'; + this._buttonImage.icon_name = 'route-reverse-symbolic'; before you push, you _could_ change the order of these assignments to match FROM and VIA which do buttomImage then icon_name.
Attachment 345880 [details] pushed as 770d4db - Add icon for reversing a route Attachment 345882 [details] pushed as 62d6188 - routing: Add button to quickly reverse a route search