Fix #402 - Add bikeshare support - Layers and Trip Plan #776
Conversation
…own with no details about the station yet)
|
|
| @@ -0,0 +1,108 @@ | |||
| package org.onebusaway.android.map.bike; | |||
There was a problem hiding this comment.
Could you please add Apache license header to the top of each new class?
If any code is a copy/paste/edit from other files in the project, the names of the people and orgs in the Apache license header of that file should be included here, along with your name, and the date range should be the start date in that file header with end date 2017. If all code is entirely yours in the new file, then your name would be the only one listed and 2017 would be only date listed.
So, if all code is yours, it would look like this:
/*
* Copyright (C) 2017 Rodrigo Carvalho (your.email@emailprovider.org)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
If there was copy pasted code from another file, it would look something like this:
/*
* Copyright (C) 2013-2017 University of South Florida (sjbarbeau@gmail.com), Rodrigo Carvalho (your.email@emailprovider.org)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
|
|
||
| import java.util.List; | ||
|
|
||
| /** |
There was a problem hiding this comment.
Please fill out comment headers with brief description of class.
| @@ -66,12 +69,20 @@ | |||
|
|
|||
| void showStops(List<ObaStop> stops, ObaReferences refs); | |||
|
|
|||
| void showBikeStations(List<BikeRentalStation> bikeStations); | |||
There was a problem hiding this comment.
On the topic of separating the code/logic for multiple layers - I think this will change to something like:
void showLayer(Layer layer);
And, each *Overlay class would implement a new Layer interface.
|
There are certain TODOs that we'll want to make sure we fix before merging this pull request. I'll start adding comments with checkboxes for these - as we finish them, we can check the box. Here's the first:
|
Made sure the method to move the FAB is called only once. The way it was implemented it sometimes the method got called more than once for the same action (a click on a marker). Also, removed the check to see if there was an animation already running. It would not be necessary anymore since the method will be called only once. Finally, an animation lives only inside the Runnable. Removed references to it outside since it was not necessary anymore after removing the checks to see if it was running. There was also a call to reset on the animation that was already running, seems it does not make sense anymore since the method is supposed to be called only once per click on a marker.
This reverts commit b86a0a5.
| boolean setMyLocation(boolean useDefaultZoom, boolean animateToLocation); | ||
|
|
||
| void notifyOutOfRange(); | ||
|
|
||
| // Zooms to the region bounds, if a region has been set | ||
| void zoomToRegion(); | ||
|
|
||
| LatLng getSouthWest(); |
There was a problem hiding this comment.
@carvalhorr We can't actually include the LatLng object in MapModeController, because it's proprietary to Google, and we're supporting Amazon as well. Anything that has a com.google.android.gms.* import has to be implemented in the onebusaway-android/src/google/java/org/onebusaway/android/map/googlemapsv2 package classes. In this controller interface, you could change it to Location getSouthWest();, because Location is part of the Android framework (AOSP) and not proprietary to Google Maps API v2.
Every time you push a new commit to the PR Travis CI should build the project, and it will fail when building the Amazon flavor for this type of problem - see https://travis-ci.org/OneBusAway/onebusaway-android/builds/246223817 for an example. The Github PR will have a red X for any build failures. Travis builds were temporarily all failing due to #779, but I believe that's fixed now (still confirming). So if you fix this, I believe the PR will give you a green dot for a build success.
…and "My reminders"
…d returning to OBA
… apps when the FAB speed dial is open. I could not find another solution for this issue other than closing the speed dial on the activity onStop lifecycle callback. This way the problem is avoided.
…and "My reminders" after navigating to a starred stop The issue was present after navigating to a starred stop and showing the map and returning to the the "Starred stops" list using the back button
… a region with support to bikeshare was selected
…hat affect the FAB visibility that were not treated before: - Initial app installation - Switching between regions with/without support to bikeshare and vice-versa - Showing "Starred stops", "My reminders" and switching back to the "Nearby" (map) view - Opening an starred stop > showing the map view for the starred stop > returning to initial screen
…egardless of bikeshare being active in main map
|
@themonki Would you be able to do a quick translation check for this PR? I think there have been a few changes since your last review, and maybe a few new strings too. I'd like to release a beta based on this today or tomorrow. |
We're no longer using USF Maps icons
We're no longer using USF Maps icons
|
Ok, I hope to review this today. |
|
@themonki Awesome, thanks! If it's helpful, here are the direct links to the most recent strings files in this PR:
Note that you might have to wait a second after clicking on the URL for the browser to load the PR and then scroll to the diff. Not many changes total. |
barbeau
changed the title
|
@carvalhorr Thanks, this is excellent work! We really appreciate your contribution to the project! @themonki I'm merging this so I can get a beta out today. If you have a chance to do the translation please open a PR on the main project - thanks! |
This pull request is a 2017 OSGeo Google Summer of Code project that implements bikeshare support, specifically:
This PR has been merged, so you can see instructions for building and running the project in the main project README. Note that you will need to select the "Tampa" region to see bikeshare info, as it's currently the only region providing real-time bikeshare information.
Discussion surrounding the design and implementation of the bikeshare features is available in Issue #402.
Other 2017 GSoC links for this project: