Fixed #851 - added filter by arrivals/departures option to ArrivalsListFragment #1290
Conversation
…for readability
…ng the to address
…ng the to address
Signed-off-by: Amr Hossam <[email protected]>
Signed-off-by: Amr Hossam <[email protected]>
Signed-off-by: Amr Hossam <[email protected]>
|
Hi @viditpawar0, thanks for your contribution! Could you please rebase it against the main branch? |
|
@amrhossamdev Sure. But the branch is up to date with the main branch. |
|
@viditpawar0 These commits are already part of the main branch but are being shown again because your branch's history diverged from main before those commits were merged. Rebase Your Branch on main To clean up the commit history, and always start a new branch from main. Before starting new work, ensure your branch is up-to-date. Start with a fresh branch each time to avoid inheriting old commits. |
| @@ -41,5 +42,9 @@ public void setTripsForStop(ContentQueryMap tripsForStop) { | |||
| notifyDataSetChanged(); | |||
| } | |||
|
|
|||
| abstract public void setData(ObaArrivalInfo[] arrivals, ArrayList<String> routesFilter, long currentTime); | |||
| abstract public void setData(ObaArrivalInfo[] arrivals, ArrayList<String> routesFilter, long currentTime, ArrivalFilter arrivalFilter); | |||
| public void setData(ObaArrivalInfo[] arrivals, ArrayList<String> routesFilter, long currentTime) { | |||
| @@ -227,4 +227,10 @@ | |||
| <item>@string/trip_plan_arriving</item> | |||
| </string-array> | |||
|
|
|||
| <string-array name="stop_info_arrival_filter_options"> | |||
There was a problem hiding this comment.
We are using localization to support multiple languages so make sure to translate those strings to all supported languages
| @@ -1280,4 +1280,5 @@ | |||
| <string name="display_test_wide_alerts_for_regions">Display test-wide alerts for regions</string> | |||
| <string name="do_you_want_to_plan_the_trip_now">Do you want to plan the trip now?</string> | |||
| <string name="plan_trip">Plan Trip?</string> | |||
| <string name="stop_info_option_filter_arrivals_departures">Filter arrivals/departures</string> | |||
There was a problem hiding this comment.
Translate that to multiple languages
| @@ -106,6 +146,15 @@ private static boolean shouldAddEta(ArrivalInfo info) { | |||
| return false; | |||
| } | |||
|
|
|||
| private static boolean shouldAddArrivalFilter(ArrivalInfo info, @NonNull ArrivalFilter arrivalFilter) { | |||
There was a problem hiding this comment.
Could you please write comments explaining this function?
| this.arrayResourceIndex = arrayResourceIndex; | ||
| } | ||
|
|
||
| public String getOptionString(Resources appResources) { |
| @@ -61,7 +64,7 @@ public static ArrayList<ArrivalInfo> convertObaArrivalInfo(Context context, | |||
| if (filter.contains(arrival.getRouteId())) { | |||
| ArrivalInfo info = new ArrivalInfo(context, arrival, ms, | |||
| includeArrivalDepartureInStatusLabel); | |||
| if (shouldAddEta(info)) { | |||
| if (shouldAddEta(info) && shouldAddArrivalFilter(info, arrivalFilter)) { | |||
There was a problem hiding this comment.
Could you please explain your logic here ?
| @@ -71,7 +74,7 @@ public static ArrayList<ArrivalInfo> convertObaArrivalInfo(Context context, | |||
| for (ObaArrivalInfo obaArrivalInfo : arrivalInfo) { | |||
| ArrivalInfo info = new ArrivalInfo(context, obaArrivalInfo, ms, | |||
| includeArrivalDepartureInStatusLabel); | |||
| if (shouldAddEta(info)) { | |||
| if (shouldAddEta(info) && shouldAddArrivalFilter(info, arrivalFilter)) { | |||
There was a problem hiding this comment.
and here can you explain the logic
Please make sure these boxes are checked before submitting your pull request - thanks!
Apply the
AndroidStyle.xmlstyle template to your code in Android Studio.Run the unit tests with
gradlew connectedObaGoogleDebugAndroidTestto make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them for the initial submission of the pull request. When addressing comments on a pull request, please push a new commit per comment when possible (reviewers will squash and merge using GitHub merge tool)
Fixed #851
Screenshot:
