[android] Add track recording widget to the map screen #9929

kirylkaveryn · 2024-12-24T08:07:57Z

kavikhalique commented

2024-12-24 08:07:57 +00:00

(Migrated from github.com)

This implementation is mirror of #9847 for android devices
Partially Fixes #9173

It implements track recording widget on Map screen, Route plan screen and Navigation screen.
Redesigns the landscape layout (similar to iOS) to avoid overlaps.

If the user taps the button the "Save" dialog will be shown.
The button has a small blinking effect to attract the user slightly.
The compass view will be moved under this button when it is active.
The button is always visible when TR is active and cannot be hidden.

Map Screen Route Planning Navigating

Map Screen Landscape Route Planning Landscape Navigating Landscape

This implementation is mirror of #9847 for android devices Partially Fixes #9173 It implements track recording widget on Map screen, Route plan screen and Navigation screen. Redesigns the landscape layout (similar to iOS) to avoid overlaps. - If the user taps the button the "Save" dialog will be shown. - The button has a small blinking effect to attract the user slightly. - The compass view will be moved under this button when it is active. - The button is always visible when TR is active and cannot be hidden. <img src="https://github.com/user-attachments/assets/1ce25cf3-af9e-47ce-871f-354eb48b5013" alt="Map Screen" height="400"> <img src="https://github.com/user-attachments/assets/000d9fbe-42c6-4c1d-ae21-b51e1c949a54" alt="Route Planning" height="400"> <img src="https://github.com/user-attachments/assets/eb0e0319-f05c-4623-bad6-85c304b4a9f2" alt="Navigating" height="400"> <img src="https://github.com/user-attachments/assets/1f1b9c63-a0d4-4f9a-871e-895e43ebe0d5" alt="Map Screen Landscape" height="150"> <img src="https://github.com/user-attachments/assets/e8aa788a-d99f-4f51-9e7e-a61cf59c5119" alt="Route Planning Landscape" height="150"> <img src="https://github.com/user-attachments/assets/f3bef216-5000-4a18-802c-837ee93a1143" alt="Navigating Landscape" height="150"> <img src="https://github.com/user-attachments/assets/d5f32c59-d542-49e1-822a-b1563800960b" height="500"> <img src="https://github.com/user-attachments/assets/bb4f5cd5-fe62-4dea-acca-d6a130b7b055" height="150">

🚀 1

biodranik (Migrated from github.com) reviewed 2024-12-24 08:07:57 +00:00

kirylkaveryn reviewed 2024-12-24 08:07:57 +00:00

pastk reviewed 2025-02-23 09:08:58 +00:00

pastk left a comment

Sorry for the delay with the review!

android/app/src/main/java/app/organicmaps/MwmActivity.java

pastk commented

2025-02-23 08:42:50 +00:00

you forgot your debug output :)

android/app/src/main/java/app/organicmaps/MwmActivity.java

					
				@ -594,4 +594,4 @@

				      mNavBarHeight = isFullscreen() ? 0 : windowInsets.getInsets(WindowInsetsCompat.Type.systemBars()).bottom;

				      // For the first loading, set compass top margin to status bar size

				      // The top inset will be then be updated by the routing controller

				      if (mCurrentWindowInsets == null)

pastk commented

2025-02-23 08:46:16 +00:00

are you sure the else branch is not needed anymore?
i.e. when mCurrentWindowInsets != null then no need to update compass offset?

are you sure the `else` branch is not needed anymore? i.e. when `mCurrentWindowInsets != null` then no need to update compass offset?

android/app/src/main/java/app/organicmaps/MwmActivity.java

pastk commented

2025-02-23 08:48:48 +00:00

put TrackRecorder.nativeIsTrackRecordingEnabled() into a final/const so that you don't have query it multiple times

put `TrackRecorder.nativeIsTrackRecordingEnabled()` into a final/const so that you don't have query it multiple times

pastk commented

2025-02-23 08:51:13 +00:00

its a bit strange to first increase offsetX and then decrease it back :)
maybe the logic could be re-arranged

its a bit strange to first increase `offsetX` and then decrease it back :) maybe the logic could be re-arranged

pastk commented

2025-02-23 08:52:10 +00:00

final?

pastk commented

2025-02-23 08:52:24 +00:00

final?

android/app/src/main/java/app/organicmaps/MwmActivity.java

pastk commented

2025-02-23 08:52:38 +00:00

final?

android/app/src/main/java/app/organicmaps/MwmActivity.java

pastk commented

2025-02-23 08:53:25 +00:00

final?

android/app/src/main/java/app/organicmaps/maplayer/MapButtonsController.java

pastk commented

2025-02-23 08:57:56 +00:00

button would be a more clear variable name

`button` would be a more clear variable name

android/app/src/main/java/app/organicmaps/maplayer/MapButtonsController.java

pastk commented

2025-02-23 08:59:29 +00:00

make it into one if?

make it into one `if`?

android/app/src/main/java/app/organicmaps/util/UiUtils.java

					
				@ -211,0 +217,4 @@

				    windowManager.getDefaultDisplay().getRealMetrics(metrics);

				    return metrics.heightPixels;

				  }

pastk commented

2025-02-23 08:40:10 +00:00

please add a comment what does it return - pixels or dp?
or maybe rename the method so that its clear from the name

please add a comment what does it return - pixels or dp? or maybe rename the method so that its clear from the name

android/app/src/main/res/layout/map_status_track_recording.xml

pastk commented

2025-02-23 08:14:29 +00:00

this string is about Layers

is it needed at all?

this string is about Layers is it needed at all?

pastk commented

2025-02-23 08:16:33 +00:00

why do you use LayersButton and not generic FloatingActionButton?

if LayersButton functionality is needed then it'd be better to rename LayersButton to something more generic

why do you use LayersButton and not generic FloatingActionButton? if LayersButton functionality is needed then it'd be better to rename LayersButton to something more generic

kavikhalique (Migrated from github.com) reviewed 2025-02-28 08:27:23 +00:00

android/app/src/main/java/app/organicmaps/MwmActivity.java

					
				@ -594,4 +594,4 @@

				      mNavBarHeight = isFullscreen() ? 0 : windowInsets.getInsets(WindowInsetsCompat.Type.systemBars()).bottom;

				      // For the first loading, set compass top margin to status bar size

				      // The top inset will be then be updated by the routing controller

				      if (mCurrentWindowInsets == null)

kavikhalique (Migrated from github.com) commented

2025-02-28 08:27:23 +00:00

I have moved it out of else and we are updating it every time, if part is for special case (see line 600 below) organicmaps/organicmaps#9929/commits/af8e6c1ae1bac60ee63cf429c625df3d64dd5e16#:~:text=updateBottomWidgetsOffset

I have moved it out of else and we are updating it every time, if part is for special case (see line 600 below) https://git.omaps.dev/organicmaps/organicmaps/pulls/9929/commits/af8e6c1ae1bac60ee63cf429c625df3d64dd5e16#:~:text=updateBottomWidgetsOffset

kavikhalique (Migrated from github.com) reviewed 2025-02-28 08:35:51 +00:00

android/app/src/main/java/app/organicmaps/MwmActivity.java

kavikhalique (Migrated from github.com) commented

2025-02-28 08:35:51 +00:00

For simplicity of code and to avoid usage of else statement I did that (i.e same logic less lines). Should I replace it with else statements?

kavikhalique (Migrated from github.com) reviewed 2025-02-28 09:19:47 +00:00

android/app/src/main/java/app/organicmaps/MwmActivity.java

kavikhalique (Migrated from github.com) commented

2025-02-28 09:19:47 +00:00

for offsetY final can't be used because its value is updated later

kavikhalique (Migrated from github.com) reviewed 2025-02-28 09:20:40 +00:00

android/app/src/main/java/app/organicmaps/maplayer/MapButtonsController.java

kavikhalique (Migrated from github.com) commented

2025-02-28 09:20:39 +00:00

sorry I forgot to change it : )

pastk reviewed 2025-02-28 09:20:49 +00:00

android/app/src/main/java/app/organicmaps/MwmActivity.java

pastk commented

2025-02-28 09:20:49 +00:00

How about

        final boolean isSmallScreen = UiUtils.getDisplayTotalHeight(this) < UiUtils.dimen(this, R.dimen.dp_400);
        if (!isSmallScreen || TrackRecorder.nativeIsTrackRecordingEnabled())
          offsetX += UiUtils.dimen(this, R.dimen.map_button_size);

How about ```java final boolean isSmallScreen = UiUtils.getDisplayTotalHeight(this) < UiUtils.dimen(this, R.dimen.dp_400); if (!isSmallScreen || TrackRecorder.nativeIsTrackRecordingEnabled()) offsetX += UiUtils.dimen(this, R.dimen.map_button_size); ```

kavikhalique (Migrated from github.com) reviewed 2025-02-28 09:32:14 +00:00

kavikhalique (Migrated from github.com) reviewed 2025-02-28 09:34:33 +00:00

android/app/src/main/res/layout/map_status_track_recording.xml

kavikhalique (Migrated from github.com) commented

2025-02-28 09:34:33 +00:00

This page contains Most of the codes for the other buttons present on the screen. I just copied and manipulated it. Since other buttons are LayerButtons so I used it for similarity. Should I change it?

kavikhalique (Migrated from github.com) reviewed 2025-02-28 09:37:43 +00:00

android/app/src/main/res/layout/map_status_track_recording.xml

kavikhalique (Migrated from github.com) commented

2025-02-28 09:37:42 +00:00

no its redundant since I copied the layer button that's why its there, I will remove it : )

kavikhalique (Migrated from github.com) reviewed 2025-02-28 09:55:49 +00:00

android/app/src/main/java/app/organicmaps/MwmActivity.java

kavikhalique (Migrated from github.com) commented

2025-02-28 09:55:49 +00:00

thanks, this looks perfect

pastk reviewed 2025-02-28 11:05:26 +00:00

android/app/src/main/res/layout/map_status_track_recording.xml

pastk commented

2025-02-28 11:05:26 +00:00

yes please use a generic FAB

the LayersButton has functionality specific to the layers button (left top corner) and is not needed for track recording

yes please use a generic FAB the LayersButton has functionality specific to the layers button (left top corner) and is not needed for track recording

pastk reviewed 2025-02-28 14:02:03 +00:00

android/app/src/main/java/app/organicmaps/MwmActivity.java

					
				@ -590,14 +590,14 @@ public class MwmActivity extends BaseMwmFragmentActivity

				    ViewCompat.setOnApplyWindowInsetsListener(mPointChooser, (view, windowInsets) -> {

pastk commented

2025-02-28 13:56:27 +00:00

final

pastk reviewed 2025-03-01 09:13:24 +00:00

android/app/src/main/res/layout/map_status_track_recording.xml

pastk commented

2025-03-01 09:13:24 +00:00

what about this one?

kavikhalique (Migrated from github.com) reviewed 2025-03-01 09:28:19 +00:00

android/app/src/main/res/layout/map_status_track_recording.xml

kavikhalique (Migrated from github.com) commented

2025-03-01 09:28:19 +00:00

Thanks working on this

rtsisyk approved these changes 2025-03-01 13:42:16 +00:00

pastk approved these changes 2025-03-01 13:49:02 +00:00

pastk left a comment

Thanks!

This repo is archived. You cannot comment on pull requests.

No reviewers

biodranik

kirylkaveryn

rtsisyk

pastk

No labels

Android Automotive (AAOS)

API

AppGallery

AppStore

Battery and Performance

External Map Datasets

F-Droid

Fonts

Frequently User Reported

No milestone

No project

No assignees

4 participants

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: organicmaps/organicmaps-tmp#9929

				`@ -590,14 +590,14 @@ public class MwmActivity extends BaseMwmFragmentActivity`
				`ViewCompat.setOnApplyWindowInsetsListener(mPointChooser, (view, windowInsets) -> {`

Rows
Columns