Always show next turn option #7196

2024-01-17T20:01:06Z

jm355 commented

2024-01-17 20:01:06 +00:00

(Migrated from github.com)

Implements #4171 for android, and potentially #849

I tried to set it up for iOS, but I don't have a mac so I don't know if I did it right. I can say for sure that alwaysShowNextTurnCell needs to be uncommented.

I also haven't been able to test it, app:compileFdroidDebugAndroidTestJavaWithJavac fails when I try to Build APKs in android studio. It built when I hit the debug button, but the emulator didn't work, and the apk it made for that wouldn't install on my phone

edit: I had to export JAVA_HOME=/opt/android-studio/jbr and then ./gradlew assembleFdroidBeta -Parm64 worked.

Implements #4171 for android, and potentially #849 I tried to set it up for iOS, but I don't have a mac so I don't know if I did it right. I can say for sure that alwaysShowNextTurnCell needs to be uncommented. I also haven't been able to test it, app:compileFdroidDebugAndroidTestJavaWithJavac fails when I try to Build APKs in android studio. It built when I hit the debug button, but the emulator didn't work, and the apk it made for that wouldn't install on my phone edit: I had to export `JAVA_HOME=/opt/android-studio/jbr` and then `./gradlew assembleFdroidBeta -Parm64` worked.

🎉 2

jm355 (Migrated from github.com) reviewed 2024-01-17 20:20:16 +00:00

biodranik (Migrated from github.com) reviewed 2024-01-18 20:25:33 +00:00

map/routing_manager.hpp

biodranik (Migrated from github.com) commented

2024-01-18 20:25:32 +00:00

Is there an easier way to check that the map is in landscape mode using the Framework? E.g. compare the dimensions of the map screen?

CC @vng

Is there an easier way to check that the map is in landscape mode using the Framework? E.g. compare the dimensions of the map screen? CC @vng

jm355 (Migrated from github.com) reviewed 2024-01-18 20:57:12 +00:00

map/routing_manager.hpp

jm355 (Migrated from github.com) commented

2024-01-18 20:57:12 +00:00

There are some spots in the code that compared dimensions, and some that used the system. Either way, I'm pretty sure that data will have to be passed through the routing manager to the routingsession, so the logic should just need to be changed at the top.

It's worth noting, the orientation needs to be checked every time the orientation changes, so to me it made sense to pass that data down so we can update it in the onOrientationChange functions as well as when the app starts

There are some spots in the code that compared dimensions, and some that used the system. Either way, I'm pretty sure that data will have to be passed through the routing manager to the routingsession, so the logic should just need to be changed at the top. It's worth noting, the orientation needs to be checked every time the orientation changes, so to me it made sense to pass that data down so we can update it in the onOrientationChange functions as well as when the app starts

jm355 (Migrated from github.com) reviewed 2024-01-18 22:59:18 +00:00

biodranik (Migrated from github.com) reviewed 2024-01-20 01:06:40 +00:00

map/routing_manager.hpp

biodranik (Migrated from github.com) commented

2024-01-20 01:06:40 +00:00

Please check if void Framework::OnViewportChanged is called on landscape/portrait rotation, and if yes, then landscape flag can be passed from there.

Please check if `void Framework::OnViewportChanged` is called on landscape/portrait rotation, and if yes, then landscape flag can be passed from there.

jm355 (Migrated from github.com) reviewed 2024-01-20 01:49:40 +00:00

map/routing_manager.hpp

jm355 (Migrated from github.com) commented

2024-01-20 01:49:40 +00:00

Ooh nice, yeah putting it there (and removing it from the other spots) updates it correctly. I'll commit the change shortly

biodranik (Migrated from github.com) reviewed 2024-01-20 06:08:30 +00:00

biodranik (Migrated from github.com) left a comment

Are you sure that the logic is not broken? Did you test your changes?

map/routing_manager.hpp

biodranik (Migrated from github.com) commented

2024-01-20 06:01:02 +00:00

Is this function needed? m_routingManager.RoutingSession().SetIsLandscape() should be enough, right?

Is this function needed? `m_routingManager.RoutingSession().SetIsLandscape()` should be enough, right?

routing/routing_session.cpp

biodranik (Migrated from github.com) commented

2024-01-20 06:03:22 +00:00

  if (distanceToTurnMeters < kShowLanesMinDistInMeters || m_route->GetCurrentTimeToNearestTurnSec() < 60.0)

```suggestion if (distanceToTurnMeters < kShowLanesMinDistInMeters || m_route->GetCurrentTimeToNearestTurnSec() < 60.0) ```

biodranik (Migrated from github.com) commented

2024-01-20 06:05:50 +00:00

Please follow the convention.

  else if (m_isLandscape)

Please follow the convention. ```suggestion else if (m_isLandscape) ```

routing/routing_session.cpp

					
				@ -432,1 +429,4 @@

				  info.m_displayedStreetName = info.m_targetName;

				  info.m_lanes.clear();

				  if (distanceToTurnMeters < kShowLanesMinDistInMeters || m_route->GetCurrentTimeToNearestTurnSec() < 60.0)

				  {

biodranik (Migrated from github.com) commented

2024-01-20 06:05:31 +00:00

No need to modify variable twice.

routing/routing_session.cpp

biodranik (Migrated from github.com) commented

2024-01-20 06:07:45 +00:00

```suggestion ```

biodranik (Migrated from github.com) reviewed 2024-01-20 06:09:07 +00:00

routing/routing_session.cpp

biodranik (Migrated from github.com) commented

2024-01-20 06:09:07 +00:00

What happens if m_isLandscape is false?

Jean-BaptisteC (Migrated from github.com) reviewed 2024-03-18 21:10:53 +00:00

map/routing_manager.hpp

Jean-BaptisteC (Migrated from github.com) commented

2024-03-18 21:10:53 +00:00

Remove?

biodranik (Migrated from github.com) reviewed 2024-05-20 18:18:12 +00:00

android/app/src/main/res/layout-land/layout_nav_top.xml

					
				@ -0,0 +39,4 @@

				    android:layout_height="wrap_content"

				    app:layout_constraintTop_toBottomOf="@id/street_frame"

				    app:layout_constraintStart_toStartOf="parent"

				    android:layout_marginTop="-40dp"

biodranik (Migrated from github.com) commented

2024-05-20 18:12:03 +00:00

Should it be hard-coded or made relative in some other way? How was this value determined? Will it work with a very large font set in system settings?

jm355 (Migrated from github.com) reviewed 2024-05-20 18:30:53 +00:00

android/app/src/main/res/layout-land/layout_nav_top.xml

					
				@ -0,0 +39,4 @@

				    android:layout_height="wrap_content"

				    app:layout_constraintTop_toBottomOf="@id/street_frame"

				    app:layout_constraintStart_toStartOf="parent"

				    android:layout_marginTop="-40dp"

jm355 (Migrated from github.com) commented

2024-05-20 18:30:53 +00:00

This file was just copied from 656686cbb4/android/app/src/main/res/layout/layout_nav_top.xml, there are only three lines that are different, and they're in the first FrameLayout section:

~ $ diff -u layout_nav_top.xml layout_land_nav_top.xml
--- layout_nav_top.xml	2024-05-20 11:26:45.962049801 -0700
+++ layout_land_nav_top.xml	2024-05-20 11:27:18.522029199 -0700
@@ -18,16 +18,16 @@
     android:background="?cardBackground">
     <RelativeLayout
       android:layout_width="match_parent"
-      android:layout_height="@dimen/nav_street_height"
+      android:layout_height="wrap_content"
       tools:ignore="UselessParent">
       <TextView
         android:id="@+id/street"
         style="@style/MwmWidget.TextView.NavStreet"
         android:layout_width="match_parent"
-        android:layout_height="match_parent"
+        android:layout_height="wrap_content"
         android:layout_marginStart="@dimen/nav_street_left"
         android:maxLines="2"
-        android:layout_gravity="center_vertical"
+        android:layout_gravity="center_horizontal"
         android:gravity="center"
         tools:text="Sample street name.\nLong looooooooong!!!!"/>
     </RelativeLayout>

I don't remember the exact reason I changed them, I think it was based on some of the requests to make landscape look different to portrait mode. Personally I think changes to landscape might fit better in a different PR (and would be better off being done by someone who understands the android layout xml better), but that was what was requested so I figured out how to make it look different

Edit: Better diff format

This file was just copied from https://github.com/organicmaps/organicmaps/blob/656686cbb4f768d0c28313e7921aa555992a68eb/android/app/src/main/res/layout/layout_nav_top.xml, there are only three lines that are different, and they're in the first FrameLayout section: ``` ~ $ diff -u layout_nav_top.xml layout_land_nav_top.xml --- layout_nav_top.xml 2024-05-20 11:26:45.962049801 -0700 +++ layout_land_nav_top.xml 2024-05-20 11:27:18.522029199 -0700 @@ -18,16 +18,16 @@ android:background="?cardBackground"> <RelativeLayout android:layout_width="match_parent" - android:layout_height="@dimen/nav_street_height" + android:layout_height="wrap_content" tools:ignore="UselessParent"> <TextView android:id="@+id/street" style="@style/MwmWidget.TextView.NavStreet" android:layout_width="match_parent" - android:layout_height="match_parent" + android:layout_height="wrap_content" android:layout_marginStart="@dimen/nav_street_left" android:maxLines="2" - android:layout_gravity="center_vertical" + android:layout_gravity="center_horizontal" android:gravity="center" tools:text="Sample street name.\nLong looooooooong!!!!"/> </RelativeLayout> ``` I don't remember the exact reason I changed them, I think it was based on some of the requests to make landscape look different to portrait mode. Personally I think changes to landscape might fit better in a different PR (and would be better off being done by someone who understands the android layout xml better), but that was what was requested so I figured out how to make it look different Edit: Better diff format