Add example test Activity for Navigation Test Application #1317

danesfeder · 2018-09-26T15:51:03Z

This PR aims to create a more robust test application. When launching the test app now, you will be viewing ExampleActivity, which serves as an example of search + finding route + navigation. It's currently pretty "watered down", but serves as a good starting point for our navigation driving custom user interfaces.

TODO:

Long press to select destination (Example activity long press destination selection #1328)
Logo + attribution adjustment for Bottomsheet (Example activity logo and attribution #1324)
Fix ManeuverView not showing correctly
ExampleActivity handle back press (Example activity onbackpressed #1323)

The MainActivity list of examples is still accessible via settings:

tobrun

Just a couple of comments that I noticed while working on some follow up PRs

tobrun · 2018-09-27T09:03:48Z

app/src/main/res/layout/activity_example.xml

+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                android:layout_marginBottom="@dimen/fab_margin_bottom"
+                android:layout_marginEnd="@dimen/fab_margin_end"


missing marginRight for backwards compatibility

tobrun · 2018-09-27T09:04:16Z

app/src/main/res/layout/activity_example.xml

+                android:layout_height="wrap_content"
+                android:layout_marginBottom="@dimen/fab_margin_bottom"
+                android:layout_marginEnd="@dimen/fab_margin_end"
+                app:srcCompat="@drawable/ic_my_location"/>


missing gradle configuration for vectorDrawables.useSupportLibrary=true

tobrun · 2018-09-27T09:04:57Z

app/src/main/res/layout/activity_example.xml

+            android:id="@+id/settingsFab"
+            android:layout_width="wrap_content"
+            android:layout_height="wrap_content"
+            android:layout_marginStart="@dimen/fab_margin_default"


missing marginLeft for backwards compatibility (applicable to FAB below)

tobrun · 2018-09-27T09:05:32Z

app/src/main/res/layout/activity_example.xml

+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                android:layout_gravity="center_vertical"
+                android:layout_marginStart="8dp"


missing marginLeft for backwards compatibility

tobrun · 2018-09-27T11:11:19Z

One additional comment is that keyboard shows when opening the app, this is a strange UX as you can't see the edittext and when typing you can see the results coming from it. I would suggest not showing it at all on startup. update: done in 9404856.

Guardiola31337

This looks good so far. Great work!

I've left some comments code-related. I've also noticed some "behavioral" issues when playing with the example 👀 (probably some of them are known issues)

Let me know if you have any questions on ☝️ or if further clarification / explanation is needed.

Guardiola31337 · 2018-09-28T07:51:18Z

app/src/main/AndroidManifest.xml

          package="com.mapbox.services.android.navigation.testapp">

    <uses-permission android:name="android.permission.VIBRATE"/>
+    <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"/>


I believe these are needed because of

mapbox-navigation-android/app/build.gradle

Line 53 in 6eb307a

implementation dependenciesList.searchSdk

right? If so, we should request the permissions to the user as we do with location permissions.

Guardiola31337 · 2018-09-28T08:50:24Z

app/src/main/java/com/mapbox/services/android/navigation/testapp/NavigationApplication.kt

+
+  companion object {
+    var instance: NavigationApplication by DelegatesExt.notNullSingleValue()
+    const val DEFAULT_MAPBOX_ACCESS_TOKEN = "YOUR_MAPBOX_ACCESS_TOKEN_GOES_HERE"


We should place DEFAULT_MAPBOX_ACCESS_TOKEN as private const in the file scope instead of inside the companion object especially because this is only used here in NavigationApplication (there's no other classes accessing to it) and also because the companion object is a new type that is created from the file.

Guardiola31337 · 2018-09-28T08:51:59Z

app/src/main/java/com/mapbox/services/android/navigation/testapp/example/ui/ExampleActivity.kt

+  private var map: NavigationMapboxMap? = null
+
+  companion object {
+    const val ZERO_PADDING = 0


Same here ☝️ - the companion object could be removed completely.

Guardiola31337 · 2018-09-28T08:53:02Z

app/src/main/java/com/mapbox/services/android/navigation/testapp/example/ui/ExamplePresenter.kt

+class ExamplePresenter(private val view: ExampleView, private val viewModel: ExampleViewModel) {
+
+  companion object {
+    const val DEFAULT_ZOOM = 12.0


Same here ☝️

Guardiola31337 · 2018-09-28T08:54:10Z

app/src/main/java/com/mapbox/services/android/navigation/testapp/example/ui/ExamplePresenter.kt

+      val top = resources.getDimension(R.dimen.route_overview_padding_top).toInt()
+      val right = resources.getDimension(R.dimen.route_overview_padding_right).toInt()
+      val bottom = resources.getDimension(R.dimen.route_overview_padding_bottom).toInt()
+      // left top right bottom


NIT - unnecessary comment

Guardiola31337 · 2018-09-28T08:54:40Z

app/src/main/java/com/mapbox/services/android/navigation/testapp/example/ui/ExampleViewModel.kt

+class ExampleViewModel(application: Application) : AndroidViewModel(application) {
+
+  companion object {
+    const val ONE_SECOND_INTERVAL = 1000


Same here ☝️

Guardiola31337 · 2018-09-28T09:07:27Z

...a/com/mapbox/services/android/navigation/testapp/example/ui/navigation/ExampleRouteFinder.kt

+                         private val accessToken: String) : Callback<DirectionsResponse> {
+
+  companion object {
+    const val BEARING_TOLERANCE = 90.0


Same here ☝️

Guardiola31337 · 2018-09-28T10:25:08Z

app/src/main/java/com/mapbox/services/android/navigation/testapp/example/ui/ExampleViewModel.kt

+  private val routeFinder: ExampleRouteFinder
+  private val accessToken: String = instance.resources.getString(R.string.mapbox_access_token)
+
+  init {


We should explore the option of adding constructor properties and factory functions to make init shorter.

@Guardiola31337 can you provide an example of this? I'm not entirely sure what you mean here.

I was referring to extract "file" functions to initialize the different variables and make the init block shorter.

Guardiola31337 · 2018-09-28T10:38:55Z

app/src/main/java/com/mapbox/services/android/navigation/testapp/example/ui/ExampleViewModel.kt

+    const val ONE_SECOND_INTERVAL = 1000
+  }
+
+  val location: MutableLiveData<Location> = MutableLiveData()


We should explore the option of using custom LiveDatas and postValue instead. Right now, because we're exposing MutableLiveData any other outside class could change mistakenly the values with setValue. The same thing applies to the rest of variables 👇

@Guardiola31337 so like a custom class that extends from MutableLiveData and only exposes the postValue method?

MutableLiveData exposes postValue. I was referring to using that one instead.

Guardiola31337 · 2018-10-17T12:12:07Z

@danesfeder is this still WIP? Just need to address the comments? I'm excited to see this new 💯 example land!

danesfeder · 2018-10-17T13:19:33Z

@Guardiola31337 nope, I'd say it's in a good spot to merge / iterate on in the future. I'll look into addressing the comments today, thanks!

danesfeder · 2018-10-17T15:22:18Z

@Guardiola31337 I addressed most of your feedback here, thanks!

I'd like to follow up on #1317 (comment) and #1317 (comment). Imo, they aren't blocking this PR and I'd like to learn more about how to implement these comments before the changes are made.

Guardiola31337

I'd like to follow up on #1317 (comment) and #1317 (comment). Imo, they aren't blocking this PR and I'd like to learn more about how to implement these comments before the changes are made.

I've replied to your questions (we can discuss internally if you have any follow up questions) and I agree those don't block the PR (sorry if the comments meant that to you, my bad), let's

* Offline Routing Integration * add offline route activity, fix offline integration / apis and fix voice instruction milestones being fired off twice

* Add example test Activity for Navigation Test Application * Add ability to display / select alternative routes

* Offline Routing Integration * add offline route activity, fix offline integration / apis and fix voice instruction milestones being fired off twice

* Add example test Activity for Navigation Test Application * Add ability to display / select alternative routes

danesfeder · 2018-11-02T20:32:51Z

danesfeder added ⚠️ DO NOT MERGE PR should not be merged! navigation-core labels Sep 26, 2018

danesfeder added this to the 0.20.0 milestone Sep 26, 2018

danesfeder self-assigned this Sep 26, 2018

danesfeder requested review from Guardiola31337 and devotaaabel September 26, 2018 15:51

danesfeder force-pushed the example-activity branch from ebe1398 to c42c3ad Compare September 26, 2018 17:52

This was referenced Sep 27, 2018

Example activity onbackpressed #1323

Merged

Example activity logo and attribution #1324

Merged

tobrun reviewed Sep 27, 2018

View reviewed changes

This was referenced Sep 27, 2018

Example activity long press destination selection #1328

Merged

Example activity hide keyboard on launch #1329

Merged

danesfeder force-pushed the example-activity branch from 90b2a47 to 404002f Compare September 27, 2018 13:52

danesfeder mentioned this pull request Sep 27, 2018

Fixup activity_example XML for lower Android API #1334

Closed

danesfeder force-pushed the example-activity branch from 48b4c8d to 3984572 Compare September 27, 2018 15:53

danesfeder force-pushed the navigation-native branch from 51f5ea1 to 9a97a80 Compare September 27, 2018 17:04

danesfeder force-pushed the example-activity branch 2 times, most recently from d35be46 to 8fea674 Compare September 27, 2018 17:27

danesfeder changed the base branch from navigation-native to master September 27, 2018 18:16

danesfeder force-pushed the example-activity branch 3 times, most recently from 8121397 to d5e2a82 Compare September 27, 2018 18:52

danesfeder changed the title ~~[WIP] Add example test Activity for Navigation Test Application~~ Add example test Activity for Navigation Test Application Sep 27, 2018

danesfeder added ✓ ready for review and removed ⚠️ DO NOT MERGE PR should not be merged! labels Sep 27, 2018

danesfeder force-pushed the example-activity branch from d5e2a82 to 6eb307a Compare September 27, 2018 19:01

Guardiola31337 reviewed Sep 28, 2018

View reviewed changes

danesfeder force-pushed the example-activity branch 2 times, most recently from e24f9a7 to 53e0339 Compare October 17, 2018 15:20

danesfeder modified the milestones: 0.22.0, v0.23.0 Oct 25, 2018

danesfeder force-pushed the example-activity branch from 53e0339 to 0576df1 Compare November 1, 2018 16:10

Guardiola31337 approved these changes Nov 2, 2018

View reviewed changes

danesfeder and others added 15 commits November 2, 2018 15:05

MapboxNavigator Offline Routing Integration (#1300)

65421f6

* Offline Routing Integration * add offline route activity, fix offline integration / apis and fix voice instruction milestones being fired off twice

bump mapbox-navigation-native version to 3.0.0 (#1325)

db56228

Add example test Activity for Navigation Test Application

b8e70e1

add long press event for selecting a destination (#1328)

cafe118

Add ability to display / select alternative routes (#1333)

b1d06d5

* Add example test Activity for Navigation Test Application * Add ability to display / select alternative routes

use tracking animation multiplier 2x (#1343)

5b080cf

UI tweaks and improvements (#1346)

51d8e1f

Add callout manager to show callout on a route (#1339)

d84721b

MapboxNavigator Offline Routing Integration (#1300)

5642629

* Offline Routing Integration * add offline route activity, fix offline integration / apis and fix voice instruction milestones being fired off twice

bump mapbox-navigation-native version to 3.0.0 (#1325)

aafe8ef

Add example test Activity for Navigation Test Application

e7401b2

add long press event for selecting a destination (#1328)

0f7d9b6

Add ability to display / select alternative routes (#1333)

d1f39e7

* Add example test Activity for Navigation Test Application * Add ability to display / select alternative routes

UI tweaks and improvements (#1346)

c255004

Add callout manager to show callout on a route (#1339)

40fcb77

danesfeder force-pushed the example-activity branch from 0576df1 to 40fcb77 Compare November 2, 2018 19:06

danesfeder merged commit 4b2c20c into master Nov 2, 2018

This was referenced Nov 5, 2018

Check for valid geocoding response in ExamplePresenter #1499

Merged

Release v0.23.0 #1504

Closed

danesfeder deleted the example-activity branch November 7, 2018 21:50

Add example test Activity for Navigation Test Application #1317

Add example test Activity for Navigation Test Application #1317

Uh oh!

Conversation

danesfeder commented Sep 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tobrun left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobrun commented Sep 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Guardiola31337 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Guardiola31337 commented Oct 17, 2018

Uh oh!

danesfeder commented Oct 17, 2018

Uh oh!

danesfeder commented Oct 17, 2018

Uh oh!

Guardiola31337 left a comment

Choose a reason for hiding this comment

Uh oh!

danesfeder commented Nov 2, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

danesfeder commented Sep 26, 2018 •

edited

Loading

tobrun commented Sep 27, 2018 •

edited

Loading