[android] refactor map button and remove layers selection from main menu #2905
Labels
No labels
Accessibility
Accessibility
Address
Address
Android
Android
Android Auto
Android Auto
Android Automotive (AAOS)
Android Automotive (AAOS)
API
API
AppGallery
AppGallery
AppStore
AppStore
Battery and Performance
Battery and Performance
Blocker
Blocker
Bookmarks and Tracks
Bookmarks and Tracks
Borders
Borders
Bug
Bug
Build
Build
CarPlay
CarPlay
Classificator
Classificator
Community
Community
Core
Core
CrashReports
CrashReports
Cycling
Cycling
Desktop
Desktop
DevEx
DevEx
DevOps
DevOps
dev_sandbox
dev_sandbox
Directions
Directions
Documentation
Documentation
Downloader
Downloader
Drape
Drape
Driving
Driving
Duplicate
Duplicate
Editor
Editor
Elevation
Elevation
Enhancement
Enhancement
Epic
Epic
External Map Datasets
External Map Datasets
F-Droid
F-Droid
Fonts
Fonts
Frequently User Reported
Frequently User Reported
Fund
Fund
Generator
Generator
Good first issue
Good first issue
Google Play
Google Play
GPS
GPS
GSoC
GSoC
iCloud
iCloud
Icons
Icons
iOS
iOS
Legal
Legal
Linux Desktop
Linux Desktop
Linux packaging
Linux packaging
Linux Phone
Linux Phone
Mac OS
Mac OS
Map Data
Map Data
Metro
Metro
Navigation
Navigation
Need Feedback
Need Feedback
Night Mode
Night Mode
NLnet 2024-06-281
NLnet 2024-06-281
No Feature Parity
No Feature Parity
Opening Hours
Opening Hours
Outdoors
Outdoors
POI Info
POI Info
Privacy
Privacy
Public Transport
Public Transport
Raw Idea
Raw Idea
Refactoring
Refactoring
Regional
Regional
Regression
Regression
Releases
Releases
RoboTest
RoboTest
Route Planning
Route Planning
Routing
Routing
Ruler
Ruler
Search
Search
Security
Security
Styles
Styles
Tests
Tests
Track Recording
Track Recording
Translations
Translations
TTS
TTS
UI
UI
UX
UX
Walk Navigation
Walk Navigation
Watches
Watches
Web
Web
Wikipedia
Wikipedia
Windows
Windows
Won't fix
Won't fix
World Map
World Map
No milestone
No project
No assignees
4 participants
No due date set.
Dependencies
No dependencies set.
Reference: organicmaps/organicmaps-tmp#2905
Loading…
Add table
Reference in a new issue
No description provided.
Delete branch "layers-menu"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Idea was discussed in organicmaps/organicmaps#1745.
Layers are now only present in the main menu. The layers button raised too many issues and I personally never use it so removing it makes the app cleaner.Layers button is now between zoom and position buttons and layers selection has been removed from the main menu to avoid duplicating UI elements.
To simplify handling of on-map buttons, a new class
MapButtonsControllerhas been created and all on-map buttons have been added to themap_navigation_buttonsxml layout (even the bookmark and search buttons only shown when planning/navigating). This class is responsible of controlling the visibility of all map buttons and will translate them when the place page moves. The class provides a callback to use in theMwmActivityto detect which button has been pressed.All button resources (button background and icons) have been updated to use xml/svg format. This makes buttons appear sharper and makes maintaining easier.
Here is a video demo in portrait mode:
https://user-images.githubusercontent.com/80701113/177939604-1eda3ecd-b273-4d15-9f68-2839433a37f7.mp4
And here is the demo in landscape mode:
https://user-images.githubusercontent.com/80701113/177939641-894d7d0a-77b1-4ccf-8218-4b5bd81f2150.mp4
Closes #2614, closes #1655, closes #508
Makes perfect sense to de-duplicate the UI.
However I'm a bit worried that after adding a few alternative map styles (outdoors, every day, cycling, etc.) it will be too crowded in the main menu pop-up (atm it takes ~60% of screen estate when open on my device) and we'll have to go back to a separate layers/styles button.
Also personally I use the layers button always - never used the main menu pop-up to switch layers. And there will be many users who are accustomed to it too and they may "lose" the feature after the update - and I don't know how we can address it easily.
If we keep it then we have to change the way map buttons are handled. Right now it is quite complicated and introduces many bugs.
Let's think about the future:
What I can try to do then is to re-add the layers button but making sure it fixes the current bugs. And then I will remove the layers selection in the main menu (maybe replace it by the OM logo?).
It is a possible approach but needs a bit of UI/UX designing first.
Yeap its a very good point about a visible current layer/style indicator.
And from my POV it makes a perfect sense to combine it with a switcher.
So from my POV keeping the separate button and removing layers from the menu (TBH I see no point to substitute it with an OM logo or anything else - the menu will be just more compact and its great).
But I agree there are some unresolved usability issues with that layers switch button.
Do you have an idea already how to fix them?
I was thinking of handling all map buttons at the same time, maybe by resizing the parent layout instead of working with each button separately.
Should I close this issue and create another one or can I simply update this one?
You can work on this one, I think your initial intention was to clean up the old/buggy code, and that's good )
I would use some icons to indicate the selected style (Apple maps do it great), and probably move it on the lower right, near the position button, for easier switching.
And please no animations! The interface should be blazing fast everywhere :)
I updated the main post with information from my latest commits.
I tried to refactor as much as possible but there is still room for improvement.
The button is now in the lower right but I did not change the icon yet. I will have to experiment with this.
Buttons are instantly shown/hidden without animations!
Thanks for the videos! It looks great, and can be improved even more ;-)
@biodranik I implemented 1. and 2. I ended up removing the class
NavigationButtonsAnimationControllerand moved the logic insideMapsButtonsControllerto make the architecture easier to understand.Here is a video demonstrating point 1:
https://user-images.githubusercontent.com/80701113/178043981-3517032a-39ed-428f-8c87-47a1f1f3fdf9.mp4
And here for point 2:
https://user-images.githubusercontent.com/80701113/178043985-fbaf50a0-b254-4f50-a6f7-ab0e1b860d17.mp4
For point 3, here is how it looks on the Pixel C. Like in landscape, it does not move the buttons because the code checks for the place page width to see if it will obstruct the buttons.
https://user-images.githubusercontent.com/80701113/178045032-50fdfce6-8e60-4ced-bad6-ee361007b42f.mp4
If everything is good for you, I can rework the PR git history to make it cleaner or you can squash when merging, your call.
This should be carefully tested in all modes before the merge :) @rtsisyk can you please help?
LGTM with minor comments.
When it can be null?
nit: final
Won't it be covered anymore?
(v)?
@ -110,20 +104,13 @@ public class NavigationController implements Application.ActivityLifecycleCallbaUiUtils.extendViewWithStatusBar(mStreetFrame);UiUtils.extendViewMarginWithStatusBar(turnFrame);@pastk @rtsisyk @vng it should be squashed on merge after a proper testing
It'd be nice to have a left-handed mode that flips the buttons to the left :) I really like it so far though!
Yes, mirroring the interface for left-handed, and un-mirroring it for RTL text layouts was already requested by several people. Not sure if it's easy to do though (mirror interface only while leaving the text direction as is).
I agree I would be nice but it should probably done in an other PR as it would change not only the map buttons, but also the FAB buttons in the search, bookmark and download screens.
It is a leftover from the original code but you are right it should not be possible (and if it is there better be a crash than a silent error)
It is still necessary to adjust the compass position to show the navigation toolbar
OnClickListenerexpects a function accepting aViewand not returning anything. If I do not put the view argument the compiler complains. So here thevargument is simply ignored. I did not find a better way, seems like a limitation from Java.This refactoring is not possible as the
bookmarkButtonvariable is used twice (bookmarkButton.setImageDrawableandbookmarkButton.getContext()).Why don't we show the layers button in routing and navigation modes?
Its quite inconvenient to go back to the main map mode just to e.g. turn on subway layer to locate that subway entrance you're going to. Especially inconvenient if one had a customized route with a lot of intermediate stops - it'll be reset.
Some bugs I found after a quick testing on a Nexus S emulator (480x800 res):
+zoom button doesn't fit in the landscape modeAlso when I open a PP first buttons to disappear because of lack of space are zoom buttons and I'm left with the "center" and "layers" ones. I think zoom buttons are used more often and thus are more important to stay on the screen.
Also in portrait mode there is space actually to preserve all buttons, but they won't move higher that the top
+button.Also TBH personally I liked the previous top left corner location of the layers button more.
RE: Case when buttons don't fit the height in landscape. Should their size be automatically reduced to fit the height? Or do we need a different layout and button positions?
Does the layers button show which layer is active at the moment? It would be great if it will be clear for users.
It was the behavior before the PR and even if I show it, it has no effect when routing.
If layers are a feature often used, then it should not be in the top left. Bottom right is more accessible for one-handed use.
Using a different layout would take too much screen space. IMO reducing button size would be better
It does not but I don't know if using the layers graphics is a good idea as they may not render well on a small button. We may need a new very simple icons.
True. But we don't put buttons for seldom used functions onto the map screen, so by this definition all of them are used often;
the point is that "often" is relative :)
For sure the zoom and center buttons are used more often. Also the right side of the screen is crowded with buttons already, while the left side will be empty if we move the layers button...
At the very least if not keeping the layers button in top left then let's put it above the zoom buttons (ideally - top right corner and then the compass button should appear right under it or, alternatively, the compass button should push the layers one below it when visible). It'll make more logical for the layers button to disappera before zoom buttons when there is not enough space.
What do you think?
I believe all buttons will fit if we just reducing spacing between them.
I believe there are icons for that in the iOS version already. Is that right @biodranik ?
Ugh.. its even more crowded when the compass button is visible...
One more glitch:
Yeap the subway layer doesn't switch unless a routing mode (car/bicycle...) is changed. The contour lines switching works well though. Let's leave it for another issue.
Looks like we need to sit and carefully think over the whole UI and UX to avoid introducing new issues.
Let's brainstorm what would be the best UI, including:
Oh ok, then its gonna be tricky to put the layers button into the top right corner...
They could be combined I think.
E.g. almost any style (cycling, vehicle, etc.) + contour lines layer. Same for subway layer.
Also possibly a "clean/minimal" mode + any style.
For small screens a big problem is the bottom bar. It takes way too much space so if we want to show map buttons as well, half the screen becomes unusable for the map.
The bottom bar has only 4 actions:
Help can be accessed from the settings so this one can go. Search and bookmarks already have map buttons ready, but those are only shown when navigating/routing. Settings can only be accessed from the menu.
So in the end, if we want to remove the bottom bar, we would only need to add one more button to access the settings. We would then have to rethink where each button should be placed. When routing is enabled, layers and settings button could then disappear to leave more screen space.
This would help solving issues for small screens in regular map view mode. For routing/planning, we could move some less used buttons in the planning header or the navigation bottom sheet (we would need to decide which buttons are less useful for navigation). Reducing the size of buttons would also help on small density screen.
Here are examples from the StreetComplete app:
In this app, bottom sheets when selecting elements in the map simply hide the position and zoom buttons. But this is not an issue as those are never used when a map element is selected.
And here is a comparison side by side with OM using split screen to simulate a small screen size:
Thanks, that's a great start. A few more important points:
Let's put the help button into the main menu pop-up then?
It'll be easily accessible then (compared to the traditional bottom location inside of settings).
And also won't always occupy screen space as its something not used often anyway.
And the red dot will be visible on the menu button like it is now. A user will open it and see if its map updates or some news.
To make working with map buttons easier, I converted all of them from
ImageButtonwith a custom background to a regularFloatingActionButton. This looks way better and I was able to make buttons smaller and feel more consistent. The previous buttons had their margin in their background so I was not able to reduce it as much as I wanted. Here is what it looks like on a Pixel 2 (small screen):If we remove the bottom bar, there would be even more space.
Looks good! Can you please make buttons a bit more transparent, like on iOS?
Btw, iOS works now with SVG, so it would be great to also have source icons in it.
Sure, I will try once I managed to remove the bottom menu (it is a bit harder than I thought).
I am not sure I understand. Do you want me to use the iOS icons in Android, or the other way around? I created the icons with the Android Studio assets generator.
This is how it looks on the Pixel 2 without the bottom bar. I was not able to lower the map buttons as there are still some cases where a bottom bar is shown (search, place page toolbar and navigation menu). There are still some issues with landscape mode but this is slowly improving.
https://user-images.githubusercontent.com/80701113/178246638-d365d7e4-badc-4899-bd70-abd30ef6c748.mp4
Regarding button placement, we could place the menu button in the top left, position, search and zoom buttons to the right (from bottom to top in this order), and then at the left layers and bookmark. We would have 4 buttons max on each side, and while navigating the layers and settings buttons disappear.
There is a compass button also, so for your suggestion it makes 5 buttons on the right side (a bit crowded):
compasspositionsearchzoom +zoom -And 3 buttons on the left:
menulayersbookmarksHere are some thoughts:
menu,searchandbookmarksin the bottom bar made sense;menu,searchandbookmarkswhen a PP is open - we don't have to make these 3 buttons always visible in toolbar-less layout tooHere is a raw buttons layout idea based on these points:
Right top corner:
compassRight side (very convenient location, often-used functions):
zoom +zoom -positionRight bottom edge (convenient location, often-used functions):
searchbookmarks(to the left of thesearch)Left bottom corner (less convenient location, less often-used functions):
menuTop left corner (less convenient location, less often-used functions):
layersThe bottom buttons will be hidden when a PP is open (just like it is now).
So overall for the regular map view mode there is very little UX change.
In the routing and navigation mode the
searchandbookmarksare moved to the left side,menuis gone (like it is now). Thelayersbutton will be gone for now. When its functionality is fixed for routing/navigation then it could be moved to the right along the top edge just enough so to give space to the next turn sign.When zoom buttons are disabled:
Right top corner:
compassRight side:
searchbookmarkspositionLeft bottom corner:
menuTop left corner:
layersSo
searchandbookmarkswill be available always regardless of PP state. Also they won't be moved in routing/navigation mode.What do you think?
Also need to think about a landscape layout and layouts for left-handed people and RTL languages...
You are right it makes sense to have a large area at the center of the screen free of buttons.
I can try to make a zone where buttons won't move and put those in there.
This could complicate the code quite a bit. Right now layouts are defined in xml, but this would mean defining layout in java code or dynamically loading the right layout from java.
Now that I've simplified how buttons are handled, I can try to make such a layout.
Something like this would be ok?
Looks great!!!
I managed to make the layout change based on navigation state. The map buttons are now stored in a fragment, and this fragment has its layout replaced when starting/exiting navigation mode. So now we have a system to completely customize the map button layout in one place!
https://user-images.githubusercontent.com/80701113/178336312-745e5a40-f9e1-4955-b1bd-caf76f8fd43e.mp4
Thanks for the experiments! A few observations:
Re:3
It should be possible to change the buttons size depending on the screen dpi and size, right?
Good news! I have been able to create a separate layout for each mode (regular, planning and navigation) and for each orientation (portrait and landscape). So we now have 6 xml layout files to describe all possible button positions.
Each layout is separated into 4 zones: top, bottom, left and right. Top and bottom are fixed, buttons in those zones will not move with the place page. Buttons in left and right will move according to the place page and disappear when reaching the top. Buttons in the left and right have a separate parent . This means you can fine tune the top and bottom margins to make them appear and disappear on different heights.
For example, bookmark and search buttons need to disappear before other buttons in navigation mode because of the next turn indication.
Here are some quick videos showing all the layouts. I did not put much thought into the layout themselves, but we now have the system in place so we can start fine tuning everything.
https://user-images.githubusercontent.com/80701113/178556004-691c6a09-b194-4a9f-8998-2bbe8dd678c1.mp4
https://user-images.githubusercontent.com/80701113/178556008-5eeaa00c-61e0-4d75-b983-44b2abcae602.mp4
Yep I just discovered you can create files storing dimensions depending on the phone's DPI.
wow, your changes look really good. I love it!
this PR also fixes #2280, I guess :)
Thanks! haven't tried split screen but this should make it easier to work with such cases.
@j13m126 This PR fixes some of the issues in #2280 but there are still some overlapping with the rest of the UI in navigation mode.
Looks great, thanks! Looking forward to using it :)
I like the idea. Can we add this new mode under a feature flag in Settings? It will take some time for users to accept this significant change of UI.
No! We definitely don't want to keep old and new code together, the goal is to make the UI better for users and make UI code support easier for us.
There is a way to test these changes. It is called a beta release. We'll make a separate build of the beta so everyone can provide feedback before merging changes into the master.
Users will have some resistant against this change, even if it is considered "better".
My wishes:
3
Seeing how I nearly rewrote everything related to buttons I'm afraid this would not be possible without much additional work.
In the end there is not much UX changes. We still have the same buttons at the bottom, but in floating buttons instead of a menu bar. The rest of the buttons are also in their original place, but better handle small screens / landscape. I agree some will be resistant to change, but there are also some who will welcome it. There will always be people who dislike changes but the app must continue to evolve one step at a time.
I did that at first but I was told to put it back here to match the previous UX. I was planning on creating a setting to hide it.
We want to avoid having too much buttons near the center of the screen. Concentrating buttons in the bottom right makes it easier for users to reach (and maybe in an other PR a left handed mode will be added. It also makes it easier to adapt to small screens.
@arnaudvergnet, what do you think about merging all preliminary refactorings first and then continue discussion about new UI? If something can be merged - it should be merged to reduce the size of PR.
Refactoring has been made according to the new UI needs, so it will be difficult to separate without introducing new bugs. The diff might look scary with nearly 150 files changes, but most of it is old resources and icon images removed to be replaced by SVGs or Google's official components (eg: using FloatingActionButton instead of a custom button implementation).
If you want I can try to reproduce the old UI with the new architecture, and then create a new PR to discuss about the new UI but I think it will be counter productive. Modifying the UI afterwards would certainly mean new refactoring anyways.
IMO the Beta release idea is good. I can continue to rebase this PR on top of master so we can gather as much feedback as possible on the new UI for a few days/weeks.
Please don't waste time on the old code/interface. Let's polish it with beta.
Rebased on latest master
Made the navigation layout change based on screen height and not portrait/landscape mode. This makes handling small screens easier. If the screen has less than 360dp height, the bookmarks buttons goes next to the search button.
Testing on Emulator. Am I right that we had buttons appear/disappear animations on tap before? Now I see raw show/hide visibility.
I never saw any animation since I started using OM. Maybe this is a thing in the iOS version.
And as @biodranik said:
I made all map buttons layout react to screen height instead of portrait/landscape, I felt it was more robust to different screen dimensions.
I also noticed an issue related to organicmaps/organicmaps#3051. With this PR, the app not only becomes unresponsive on splitscreen change, but also crashes. Seems like the fragment looses reference to the main activity. I will try to investigate more.
Sad, because I strongly believe that animations make UI look and feel better ..
I agree but making good animations that don't reduce the app's speed and usability are not easy to implement. This could be the work of another PR.
No problem here. To clarify, when buttons are immediately hided, it looks like a bug, but not like a defined behaviour.
I fixed the crash problem when changing splitscreen mode, but organicmaps/organicmaps#3051 still persists. Seems the issue comes from the activity lifecycle and is not directly related to this PR so I will not work on this here.
@biodranik @rtsisyk Merge or test with some public beta branch?
@arnaudvergnet Could you please attach some actual screenshots to debrief in our telegram channels with the users. Not only navigation, but regular map view.
Pixel 5
Portrait
Landscape
Nexus One
Portrait
Landscape
Let's make a beta from (a copy of) this branch. Buttons should be tuned before public beta, it would be great if @rtsisyk provides some initial feedback.
https://github.com/organicmaps/organicmaps/runs/7777793931?check_suite_focus=true
@biodranik should I rebase this branch on latest master or on a specific tag/revision for the beta?
Made poll in one of our groups. Not honest to make any conclusions based on screenshots, but anyway :)
If I understand correctly, you want me to create only 2 commits for this PR to be merged instead of squashing all into one commit? Just wanna make sure before I destroy the history.
I find opaque buttons better as well. I made them slightly transparent to mimic the previous UX.
This is an issue only in regular map mode right? I tried to squash all buttons at the bottom to make the center of the screen less cluttered but I can try to make them a bit higher on larger screen sizes.
No, destroying history! :) Just to change order or make some minor changes if needed to have clear commits set with refactoring and commits set with removing toolbar and changing layout.
Great patch, fantastic work! Thanks! I am OK.
Plus one
@vng @rtsisyk does this new commit history look good to you?
Can we move "remove layers selection from main menu" to the up, just before "remove main menu bar".
In this case we can assume that commits until "FloatingActionButton" doesn't change current behaviour/layout.
Let's merge the same layout and button positions as it is now first. Then we can play with buttons separately, beta test them, etc.
Otherwise, there is a huge risk of getting 1-star reviews.
You can create another PR for that while keeping your proposed layout here.
What do you want me to do then? Should I split this PR before editing the buttons/layout, or should I try to replicate the old layout with the new system?
Splitting the PR may not be that easy as I did most of the testing with the new system and the initial refactoring may introduce new bugs.
IMO the simplest way to achieve what you want would be to bring back the bottom menu bar and position the new buttons at about the same place as before. I can then create a new PR removing the menu bar and experimenting with new button placement.
I vote for a small tuning before merge:
All other changes are ok, throw toolbar away ..
Or the variant "IMO the simplest ...". That looks like a compromise now if it's easy to revert in this PR, keeping all other fixes.
@vng applied your small tuning
@arnaudvergnet @vng is it hard to add a different icon state when the current location is unknown and is not actively searched? So users can understand it and press the location button to refresh it? Maybe a crossed location arrow would be understandable.
Is it also possible to make internal icons in buttons a bit larger than they are now?
Let's make a final decision here, other experiments with icons in follow up PRs.
I don't known the location code well enough to answer.
This can be easily tweaked.
Completely agree, this PR was not meant to be this big at first. If we do not decide when to stop the changeset will continue to grow. I'd say we keep it like this and simply check with the beta if no new bugs or issues with screen sizes appear. If it is good we merge and continue the work in follow up PRs. In the meantime we can collect user ideas on the best buttons placement.
Just keep in mind I won't be very active next week, so no rush in merging as I won't be able to quickly fix issues.
@arnaudvergnet looks like buttons size and positions should be dynamic:
Did not know this was a feature! Supporting this use case should be the subject of another PR tho because it will not be trivial to handle. Could you please create an issue for this if you think this feature is worth supporting?
It's not about the feature, it's about the size of buttons that should fit small screens. Or does it break only for that specific "feature"?
@arnaudvergnet if you left button positions exactly as they were before your changes (but with proper fixes for different screen sizes), we could merge it and release it. My plan/idea was:
We already got a lot of feedback for these changes, and from our experience users often are too conservative for many valid reasons. It would be a pity to get a lot of 1-star reviews...
This PR already handles small screens, but not that small. Reducing the buttons size may not be a good idea as they could become unusable. I see supporting such small screen size as a feature as no phone has a physical screen this small. I have never used this picture in picture feature (or whatever it is called), but I guess we would need a specific UX for it.
Buttons are where they were before my changes. Not exactly as the layout reacts differently to screen sizes but not far enough to create a new UX.
If it ain't broken, don't fix it as we say :). The may layout changes are needed to support small screen for which the app is hardly usable as of now, so I don't think users would complain about those changes. If there is no need to change buttons in other cases then we won't move them.
To make sure we go in the right direction, it would be nice to create an issue with all the feedback on what should change and what should not. We could then work on refining the map buttons after this initial refactoring has been merged. I feel right now we do not clearly share the same understanding of what should be worked on and details get lost in the many comments.
Agree, it goes complicated. I tested it today and found, that:
I'm going back to my proposal: to leave all button positions exactly as they were positioned before on the toolbar and in other places, leave exactly the same transparency, exactly the same symbol/icon sizes (well, maybe we may try to make toolbar's buttons square or to look a bit different than other buttons in some other way?), apply your positioning fixes for different resolutions, and merge the PR.
Then we can continue with experiments and optimizations.
So, now it's the same as to keep toolbar as before. Round buttons like toolbar but not toolbar is the worst variant, imho.
@vng No, it's not the same. Without the toolbar it looks way better, more map is visible.
@arnaudvergnet Also two more comments to your redesigned style that should be addressed later:
4. Compass button should look more consistent (at least its position should be aligned with other buttons).
5. The ruler should be placed not above the bottom left buttons, but in the center.
I get conflicting information on this. Sometimes I'm told to make them opaque, sometimes transparent. They look similar yes but it was the case before. I can try to make them look as close to the old version as possible to not confuse the users for this PR. The looking good part is subjective, IMO they look way better than before but then everyone has a different opinion of design. Another reason why we need to clearly discuss what to do before doing more work after this PR.
I can place the old bottom buttons to their old position on the toolbar, but as I told you no button will be exactly as it was before. Same for the button and icon sizes. The new system is so different from the previous one that getting the exact same size is impossible but I can try to replicate it as much as I can.
If I am not mistaken, the compass button is handled in C++. I can change its position from Java but I don't know about its size.
If you want to place the bottom buttons in the same position as the toolbar buttons, there will be no place left in the center. And even if there was, this is not a good idea as the ruler could get covered if the screen is not wide enough (look as screenshots from the Nexus One organicmaps/organicmaps#2905 (comment)). Above seems fine to me, and compared to the previous version it handles fullscreen mode (moves the ruler at the edge of the screen).
@vng the point of the refactoring was to remove the toolbar. Reverting it back is a lot of unnecessary work considering that we want to remove it anyway.
There is no positional and visual difference between floating buttons and the same buttons in the same positions on the toolbar. So the user's UX will be exactly the same.
@biodranik applied your suggestions:
If everything is good I'll clean up the git history.
Thanks, it looks great! Sorry for the inconsistent feedback, we were actively discussing it many times ) UI is a very hot topic.
@arnaudvergnet you can cleanup commit history.
@rtsisyk it would be great to get a feedback from the real use case :)
Done
@arnaudvergnet , I see strange white artifacts on all buttons:
That must be because the background is semi transparent and the buttons have an elevation casting a shadow. I don't think we can have both without creating this issue. Would you prefer to have opaque background or to remove the elevation?
Personally I would just make the background opaque but I understand if you want to be as close as possible from the old layout.
Pick your fighter:
I'd go for opaque.
And just minimal shadow, if even necessary.
I'm sure it is possible to make the shape opaque and the background semitransparent by editing the vector icon.
It's hard to choose now, it would be great to try different APKs and play with them first.
Before, the buttons were hand made with a custom transparent background including a shadow. But it required creating resources for each button state which made the code harder to maintain and gave a less "native" look to the buttons. From what I could find, this is the only way to have proper shadows on a transparent background.
Here I only use a standard FAB with an icon and a custom background color. Several apps such as Google Maps or StreetComplete use standard FAB with an opaque background so I don't think users will miss the transparent background that much.
Here are the APKs if you want to try the different options: https://github.com/arnaudvergnet/organicmaps-apk-test. As explained above I cannot do case 6 without the artifacts. I would need to manually create the FAB background shape (which I want to avoid).
I don't like versions with shadows. 3 and 5 is my choice.
Base shadows are quite strong, but it can be tweaked by changing the elevation.
@arnaudvergnet thank you very much for your time and work! I tested all apks, here are my notes:
So far I'm ready to merge the best-looking variant: 5.no-shadow-opaque-shape-transparent-background.apk
Everything else can be done/improved/tested later.
Can't seem to reproduce this on my end. For me the ripple effect is played as expected on tap.
I don't know why it was done this way but there is a custom fading animation for the search activity. This can be removed in an other PR.
Same as 5, it can be done in another PR.
I can try to experiment with this. Do you want this to be done in another PR?
I took the background color used for the old bottom menu toolbar, which is white with 80% alpha. From what I could see the map buttons where white with 60% alpha in master. Do you want to go back to 60?
@biodranik, let's merge 3 or 5. All other improvements can be done in separate PRs.
I noticed an issue with shadows when pressing a button (this is using APK 3, not sure if it happens with the others)
also, is the help button really needed? it could easily be a menu option in the hamburger menu instead
I updated the PR to match the behavior in 5
Nice catch, this is now fixed.
We debated on it earlier in this PR and it was decided to keep it here to match the current UX.
@arnaudvergnet Let's leave it as it was: 60% for left/right buttons and 80% for bottom (toolbar) buttons. It's the closest behavior to the master. We can tune it up later, if necessary.
Done, this is how it looks now:
Cool!
The PR history looks good to me
The code is fine. The PR history looks good. The new UI is super cool. Let's merge this PR finally and move forward. Further tuning and refactoring can be performed separately.
I was skeptical initially, but with buttons on the same positions, we don't need the old toolbar anymore. This patch is probably the most significant change in UI of the project in the last couple of years, if not more. Really great work, @arnaudvergnet.
@ -0,0 +200,4 @@if (searchLayout != null){final int animRes;if (mIsExpanded)nit: It looks unusual for me, but I don't understand nothing in Android layouts :)
nit: It looks confusing when we get and check searchLayout var, but use mSearchLayout inside.
I'd make like this:
@ -0,0 +200,4 @@if (searchLayout != null){final int animRes;if (mIsExpanded)The search wheel has 2 possible shapes: round or straight line. Before this PR, the round shape was used in portrait mode, and the straight line in landscape. But now this PR uses the available height to decide which shape to show (threshold is 400dp). So instead of checking the orientation, this function must now check the available height to understand which shape is loaded.
I agree this is not the prettiest code but I could not find a better way to handle this.
I agree it could be clearer, I will make a new PR for this
is there a plan to update the place page UI? at the moment it still relies on the old taskbar, so the change is quite jarring:
https://user-images.githubusercontent.com/26939824/188507065-833b5cdc-8ad4-42e9-92ca-c436969df25a.mp4
something like this maybe?:
I had planned to look at it once I'm done with map buttons. I don't have design ideas so feel free to open an issue for this!
opened organicmaps/organicmaps#3355 👍
@arnaudvergnet is the current alpha for the toolbar 80 and 60 for zoom/my position/layers buttons? It doesn't visually match the production version in stores. At least zoom buttons. Can you increase their transparency to 80?
Also, if the toolbar's buttons transparency is less than 80, then it should be 80.
Also, zoom buttons should not be transparent inside, they should behave the same as bottom (ex)toolbar buttons. Is it hard to implement?
Bottom buttons have 80 alpha, position and zoom are 60 as you requested. I can make them all 80 if you find it better. The zoom buttons are exactly like the others I don't understand what you mean. Or I can make them all 90 if you find it too transparent.