OneBusAway / onebusaway-android

The official Android app for OneBusAway
http://www.onebusaway.org/
Other
469 stars 290 forks source link

Add "Starred routes" list #354

Open barbeau opened 8 years ago

barbeau commented 8 years ago

We now allow users to star a route/headsign/stop combination (see https://github.com/OneBusAway/onebusaway-android/issues/155#issuecomment-131239723). However, currently this is only used to pin favorite routes to the sliding panel header for particular stops.

We should add a list of "starred routes", similar to how we currently have "starred stops". This could be implemented as another navigation drawer item, or starred stops/routes could become part of the same tabbed interface accessible from the current "starred stops" navigation drawer item (which would be renamed "Starred", or "Favorites".

adingler commented 6 years ago

Over the past couple of weeks I have been working on implementing this feature in my spare time, and it is nearly finished. I am hoping to complete this over the long weekend, time allowing. However, I have one quick question. The way I am implementing this is, essentially, to reproduce what was done for starred stops. So, there is a "Starred routes" item in the navigation drawer that launches a fragment with similar (well, nearly identical) functionality to MyStarredStopsFragment. The other option, per the original comment above, would be to create a "starred/favorites" item in the navigation drawer. My question is, is there a preference for one implementation over the other? If the latter option is preferred, I can work on that before I submit a pull request.

barbeau commented 6 years ago

@adingler Awesome, thanks for working on this! I've been leaning towards a single "Starred/Favorites" entry in the nav drawer, and then a tabbed/swipeable layout so you can easily toggle back and forth. I think this works better in the mental model than favorites being broken out into two at the top navigation level. Let me know your thoughts though.

I think there is some UI design TBD around exact layout presentation of route/headsign in the list, and also UI/logic TBD surrounding removing a starred route. If you have some existing mockups or implementation/thoughts to share on this I'd welcome them. You might want to take a look at iOS as well just to see how routes/headsigns are shown in a list there.

adingler commented 6 years ago

For the UI design for routes in a list, I have the new fragment extending MyRouteListFragmentBase. This means the starred route list looks the same as the recent routes list. Here is a screenshot of what the list looks like currently: starred_routes_scaled

(edit: sorry, didn't scale the screenshot)

The logic for removing starred routes seemed to be in place, with behavior like that for starred stops, because of the recent route list implementation. Are you suggesting that some of the UI and/or logic be overhauled? In any case, I'll try to get my hands on an iPhone (or see if there is a decent Linux or Windows emulator) to check out the iOS version.

As for the single entry in the nav drawer, I agree that makes sense, so I will work toward that. To maintain consistency, I will plan to make this look and behave like the recent route list, with tabs for stops and routes -- but let me know if you have something else in mind!

barbeau commented 6 years ago

I'd prefer not to use the styling from the recent stops/routes list - this doesn't adhere to the material design specs and is just a holdover from v1 until I can integrate Recents into Search (https://github.com/OneBusAway/onebusaway-android/issues/309). I'd strongly prefer using the styling from the Starred Stops Fragment.

The main open UI question IMHO is whether we present Routes as a single entity (no matter the direction), as in current Recents, or break them down to the level of route + headsign. I'm leaning towards headsign level. One reason is that I'd like to integrate the ETAs directly into the favorite stops/routes lists to allow you to see these without drilling down an extra tap - iOS currently has something similar. And for routes you'd need the headsign to really make sense of this. Maybe throw a quick mockup together in Google Drawings or something else (doesn't need to be pretty) and we can talk over that?

For deleting logic - If you go through the process of starring a route you'll see it asks for the stop level as well - so a user can choose to star that route/headsign for just certain stops. This take take some time to set up, so I think we need an extra warning if the user has done this stop inclusion/exclusion saying that they can't get them back if they continue deleting it.

barbeau commented 6 years ago

Also, feel free to throw what you currently have implemented into a WIP PR so I and others can start looking at the code.

adingler commented 6 years ago

I understand now, it makes much more sense to break it down by route/headsign. In looking at the iOS version -- I completely forgot we have an iPad :) -- I don't see any place where there is a route list, but the "Bookmarks" list breaks things out by stop + route/headsign. Is this what you mean? Were you thinking of moving to something like this for favorites?

In any case, here is an (ugly) sketch as a starting point for further discussion, let me know if this is along the lines of what you were thinking for the favorite routes.

Regarding the deletion logic, I have been working my way through the code and have figured out how to do this. I have a better grasp of things now, and (once I have more time) I should be able to make some progress.

Sorry for the long delay in responses, I have had less time than I'd hoped to work on this -- hopefully I'll have some more time over winter break (I work at a university).

barbeau commented 6 years ago

I don't see any place where there is a route list, but the "Bookmarks" list breaks things out by stop + route/headsign. Is this what you mean? Were you thinking of moving to something like this for favorites?

Yes, that's what I was referring to. I think we should approach this with baby steps, though. Let's just do the route + headsign for now as route favorites, and we can tackle ETAs later. There is also grouping stops to think through too, but that's a lot to bite off in one PR. Talking to @aaronbrethorst to get his thought process behind the iOS implementation would be good at some point too (for me as well).

Thanks for the mockup! This is great, I left some comments in the Google Drawing.

Sorry for the delayed response, I work at a university too :) , and been slammed lately. No rush, just keep me posted on progress. Feel free to open WIP PR as you go.

aaronbrethorst commented 6 years ago

I think this will be a great first step. The one thing I'd recommend changing on the Starred Routes UI mockup is to change the subtitle to something more like "[CARDINAL ABBR] to [ROUTE DESTINATION]" or "[DESTINATION] ([ABBR])" e.g.

10
S to Downtown Seattle

or

10
Downtown Seattle (S)
barbeau commented 6 years ago

Thanks @aaronbrethorst! I agree, it should mirror the main ETA presentation of headsign: screenshot_20171125-173905