[android] Migrate nav menu to bottom sheet behavior

[arnaudvergnet]

This is an early draft to convert the navigation menu to a bottom sheet. It continues the work done in https://github.com/organicmaps/organicmaps/pull/2248 for issue #1745.

For the moment I only managed to migrate from the NavMenu class to using BottomSheetBehavior. Code still need to be cleaned up and style needs to be updated to match work in https://github.com/organicmaps/organicmaps/pull/2248.

Changes to UX

• There is no background dim when the bottom sheet is expanded so users cannot click on the background to close it (back to collapsed state).
• Users can swipe up and down to expand/collapse the menu.

Changes to UI

• None so far

Here is a demo of the work so far.

https://user-images.githubusercontent.com/80701113/158698785-ead71b12-f5c7-492b-b066-8d953c29e386.mp4

[biodranik]

Do you plan to support dim? It is hard to open and close the menu while driving.

[arnaudvergnet]

I will see what I can do.

In my previous PR I used the BottomSheetDialogFragment class as base for the bottom sheet. This is a modal component so when it is shown it will have a background dim. But this dim is displayed when the sheet is visible, either in expanded or collapsed state (collapsed = visible at peek height).

Here I use the helper BottomSheetBehavior to create a standard bottom sheet. This is a permanent component because we want the bottom sheet to be always visible while navigating, so it will not show a dim.

I will experiment with two options

• Use BottomSheetDialogFragment like for other menus, somewhat make the dim disappear in collapsed state and disable hiding.
• Keep using BottomSheetBehavior but adding a dim manually when going into expanded state.

[biodranik]

The dim itself is not as important as the ability to partially hide back the navigation bar by touching anywhere.

[biodranik]

Or maybe I'm overestimating the importance of it… We may try it as is, without dim, and see how it goes. In the worst case, the driver will see a bit less map on the screen.

[arnaudvergnet]

Or maybe I'm overestimating the importance of it… We may try it as is, without dim, and see how it goes. In the worst case, the driver will see a bit less map on the screen.

As the menu can be swiped down, this also makes it easier to hide it.

[arnaudvergnet]

Worked a bit more on this and I ended up removing the toggle button (the arrow) and adding a handle. If you like this design I'll add it to the bottom sheets added in the previous PR.

I also improved the landscape mode. Now the bottom sheet only takes a small portion of the left side making it easier to see the map.

Portrait	Portait expanded

Landscape	Landscape expanded

What is your opinion on this?

[biodranik]

That looks good. Regarding dimming, I tried to use it in the car and found the ability to touch a dimmed area and hide the bottom sheet really useful.

[arnaudvergnet]

I added the background dim and this is the result

https://user-images.githubusercontent.com/80701113/159585824-6cef11a8-c55e-4c7d-bf20-57cd7446b749.mp4

[biodranik]

Cool! Does a single tap to the bottom bar expand it too?

[arnaudvergnet]

Cool! Does a single tap to the bottom bar expand it too?

No, clicking on the bar toggle the date of arrival from relative to absolute. Swiping up is not that difficult I think. If we want a single click to open the menu, we will need to find another way to toggle between relative and absolutes arrival times.

[biodranik]

Now the menu icon is removed, and there is more space to keep both times on the screen.

[arnaudvergnet]

Now the menu icon is removed, and there is more space to keep both times on the screen.

I will try to see how to rework the layout a bit to prevent issues in low DPI screens

[arnaudvergnet]

Managed to implement this behavior. This is now what the menu looks like. Both remaining and estimated times are shown

[biodranik]

Thanks for the screenshot!

1. It would be hard to see a small text for many people. And it looks like there is plenty of space.
2. Why blue color?
3. It makes sense to save this branch and compare how it will look with "speed 3 min 11:31pm 1.4km" (note the longer US time format).

[arnaudvergnet]

1. It would be hard to see a small text for many people. And it looks like there is plenty of space.

There is plenty of space because the phone I use has a large screen. It may note be the case everywhere. How would you rearrange the different number?

2. Why blue color?

I used the app's accent color but I can use a different one. I felt making the remaining time colors would bring attention to it because it is often what we want to know when navigating.

3. It makes sense to save this branch and compare how it will look with "speed 3 min 11:31pm 1.4km" (note the longer US time format).

I'm sorry but I don't understand what you want me to do here. In the screenshot I already use the long US format.

[biodranik]

I meant displaying everything in one line. Display eta and then arrival time together, not under each other.

[arnaudvergnet]

Does this look better?

portrait	landscape

[biodranik]

No. I would make all texts larger and in one line, including the speed and distance.

[arnaudvergnet]

Making everything on one line would break on small density screens. Here is what it would look like on a Nexus One:

As you can see most of the text is truncated.

[arnaudvergnet]

Here are examples from apple maps (left) and google maps (right):

[biodranik]

If we remove the current speed (which should go separately to work with speed limits), then everything will fit. Ok, let's leave it for the future.

Coming back to your initial proposal:

1. It makes sense to make fonts even larger to see them better.
2. The horizontal line above the time doesn't look good, especially if you increase the font. Are there any other ideas how to make it look better?

[arnaudvergnet]

1. It makes sense to make fonts even larger to see them better.

I made the menu a bit higher with bigger fonts for dimensions/time estimate.

2. The horizontal line above the time doesn't look good, especially if you increase the font. Are there any other ideas how to make it look better?

Are you talking about the bottom sheet handle? If so I made it thinner and using a lighter color so that it stands out less.

Here is the result (open the first draft and this one in tabs side by side to clearly see the changes)

portrait	landscape

[biodranik]

It's better, but bold numbers are still too small to see them with non-ideal eyesight when the device is on the car's panel.

[arnaudvergnet]

Here is another test with even bigger fonts everywhere. Again open tabs side by side to see the changes.

portrait	landscape

[biodranik]

Looks great! What do you think?

[arnaudvergnet]

Yes I'm quite satisfied with the result, I'll push the changes.

Maybe if you have time you can try to see if it looks good on a car dashboard (I don't have one). I'll keep this PR as draft while I try to find/fix bugs and refactor a bit. I'll notify you when it is ready.

[arnaudvergnet]

The old navigation menu and the main menu (the bar at the bottom with 4 icons) were using the same base class BaseMenu so I needed to refactor a bit. But I got carried away and completely reworked the main menu so the diff grew in size.

[arnaudvergnet]

@biodranik it looks ready for me. Tell me if you find any issue with the PR.

[pastk]

I love the change! Looks nice and sleek! Tested on 5.0 emulator and 9.0 hardware device.

If system font size is increased then the text doesn't fit vertically:

Is it possible to make the navbar autosize vertically?

Another minor issue (though not sure its related to this PR). Too much space above "the handle":

[arnaudvergnet]

If system font size is increased then the text doesn't fit vertically

Did not think about that use case, I will see what I can do because it might also overflow horizontally.

Another minor issue (though not sure its related to this PR). Too much space above "the handle":

This PR does not change the place page, this will come later :)

[arnaudvergnet]

Managed to handle the system font scaling by computing the bottom sheet peek height dynamically. Height is computed on layout change so it will work even if you change the font scaling while navigating.

https://user-images.githubusercontent.com/80701113/160464736-67142ca0-9450-4a4a-a88a-56d853702dcb.mp4

[arnaudvergnet]

Sorry for the delay I did not have time to work on the PR recently. I'll make the changes this week.

Regarding code style, there is a real need for a linter to check and apply styling in the Android app. Nobody wants to spend time adjusting spaces and line returns. I can work on a separate PR to at least introduce an EditorConfig developers can import in Android studio or any other IDE to automatically format files.

But in the long term, the whole app should be reformatted using this new config (this should be carefully planned out as it will probably change every file), and there should be a GitHub action to check styling for new contributions. This way nobody will spend more time on this and the code will stay consistent. An example tool for this is checkstyle.

[pastk]

Please check android/code_style_scheme.xml. Not sure if all the project adheres to it though.. And we should mention it in the docs, of course!

[biodranik]

1. There is already a config in android/code_style_scheme.xml and in tools/android/
2. Let's not change the existing code style ATM to avoid unnecessary history changes (that's why I've mentioned unnecessary spaces).

A whole reformatting can be done later when github will support this feature properly: https://akrabat.com/ignoring-revisions-with-git-blame/

[arnaudvergnet]

I fixed everything, I'll make a PR to update the doc and explain how to import the code style.

[arnaudvergnet]

The old bottomsheet implementation was already removed in the previous PR: https://github.com/organicmaps/organicmaps/pull/2248/files#diff-197b190e4a3512994d2cebed8aff5479ff88e136b8cc7a4b148ec9c3945bd65aL86

The previous nav menu was using a custom implementation, it was not using any dependency.

The other third party bottomsheet implementation is com.trafi:anchor-bottom-sheet-behavior, which is only used by the place page.

So for this PR the refactoring is done unless you find something else to change.