-
Notifications
You must be signed in to change notification settings - Fork 109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mobile UI enhancements #1028
Mobile UI enhancements #1028
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this submission.
Required change:
- There is a
src/gui/map/map_editor.cpp.orig
file which needs to be removed from the commits (i.e. please edit the commits).
Extra comments:
- I have the feeling that the term "GPS" should be avoided in identifiers (and in UI). It is the most popular GNSS but it is not the only one. In Android settings you will hardly find this term.
- Icon and button position are okay for me at the moment. However in the long term I would prefer to move to a more common "position" icon - the current one looks more like "direction".
src/gui/map/map_editor.cpp
Outdated
bottom_action_bar->addAction(gps_temporary_clear_act, 1, col++); | ||
auto zoom_out_button = bottom_action_bar->getButtonForAction(zoom_out_act); | ||
auto mobile_zoom_out_menu = new QMenu(zoom_out_button); | ||
auto base_scale_action = mobile_zoom_out_menu->addAction(tr("Base scale")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Base scale" is a new term in Mapper. How about "1x" (or "100%" if we should switch to percentage)?
src/gui/map/map_editor.cpp
Outdated
connect(base_scale_action, &QAction::triggered, [this]() { | ||
main_view->setZoom(1); | ||
}); | ||
auto mapping_scale_action = mobile_zoom_out_menu->addAction(tr("Mapping scale")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Mapping scale" is a new term here. How about "2x" (or "200%" if we should switch to percentage)?
I do acknowledge that 200% is the common printing scaling when preparing paper for field work. Does it make sense to introduce this term for devices which are more flexible in scaling than paper?
Thanks, Kai, for reviewing the patches and catching the run-away .orig file. I'll push updated patches soon. Ad "GPS". While searching I could not find a common understandable term for position fix. The most precise would IMO be "satellite navigation position fix" but who would get the meaning. Next one is "(GNSS) position fix" but it sounds much like expert term. Following the Android trace and trying to figure out what terminology Android developers use I pushed the Google button and the first result took me to Android tutorial about Making Your App Location-Aware. Sadly the first sentence refers to "GPS". Next one also talks about "GPS" in the "Device only" mode. IIRC, the same wording is in my Android 7 Location services setting dialog. Therefore I'm for keeping "GPS" on the UI side until we have a commonly understood term. On the code side, there is already over 330 occurrences of "gps" string in origin/master and 10 files containing with "gps" in its name. While I'm in favor of adopting a more generic name, switching the naming convention only with this commit would be inconsistent. I'm willing to do the renames when there is consensus about the terms to use. Ad Icon. I like the idea. The current one is a combination of the Pan tool icon (the four blue arrows) and Mapper's position fix symbol (red arrow in the middle). While the current icon is in line with Mapper's outlook, the location graphical idiom might be more understandable for newcomers. Ad zoom levels. "2x zoom" and "1x zoom" is aligned with the zoom level info toast display. I'd like to keep the word "zoom" in the menu entry to make it larger and therefore easier to hit with stylus. Plain "2x" and "1x" might be too tiny for field work. I'll change the strings. Regarding 2:1 magnification, it turned out to be useful in my practice as a overmapping prevention. While my device has about 290 PPI and can render 1:15 000 map with amazing details at first glance, 1x is incomparable with printed map and is not legible enough. 2x zoom level turned out to be useful for getting clearer view of the surveyed area. In my current work 2x represents a good compromise between "legible" and "showing the context". I'd be happy to hear other user's voices. If I'm not mistaken @mlerjen had an opinion about fieldwork map scale. |
related
|
Implemented as a menu of Zoom Out button.
I already used a more common location icon for the "Copy coordinates" pie menu action: https://github.com/OpenOrienteering/mapper/blob/master/images/copy-coords.png |
Ad "GPS" - just word "Position" might be suitable enough. |
This proposed change is certainly very useful. But how to deal with regarding v0.8.0 (which still needs updates to Mapper app manual page anyway)? I would like to see/do a few modifications. Major:
Minor:
Anyway, position/sensor related actions remain scattered over three corners of the screen (top left, bottom left, bottom right). |
I never use actions "Record temporary GPS trace", "Record temporary GPS position", and "Clear temporary GPS markers". This might be because I use a lot the scribbling into bitmap tool. I wonder if anybody is a heavy user of this? |
This relates to #952 where "Show whole map - this button shall be rather in the down left corner, where zooming occurs" was also mentioned. Could you move also the Pan map button a little bit? I would suggest to put it, up and right - to the right side of Zoom in, zoom out buttons. This way one of the button clashes will disappear (close map x pan map, see also #952). I do not mind having other sensors in upper left corner - these buttons are seldomly used, usually at the start end of work. |
"Reversed drop" was the word I was looking for :-) I can't say much about "Record temporary GPS trace", "Record temporary GPS position", and "Clear temporary GPS markers". As I already said at the of my last post, the position features need consolidation, but its out of scope for v0.8.0 / #1028. Important is to
Given that I have this branch checked out the moment I can add a commit with my adjustments for further review. |
Actually, Android now uses the term "location" in its API. But there is no text anyway in the Android button: |
Sorry the question, but where are versions to test? |
There are no downloads which include this change. I'm waiting for feedback from lpechacek. As soon as I can merge the change, I will update the documentation page and start building the v0.8.0 release (candidate). |
I would argue that making incremantal changes is better than a sudden overhall of UI. Moving buttons within a region is a very small change. It can be tested only in unstable versions. This way we get feedback. |
If this is a minor change, it should be fine to release it (soon) with the next stable build which is v0.8.0. If it is considered a major change, it won't be in v0.8.0. There is litte reason to provide another preview before v0.8.0. |
Sorry for delayed response - was out sick and kept away from computers in general. @dg0yt: Good point about icon grouping. I didn't notice it as I made some more changes in my devel branch which shadowed the discrepancy. I also like the term "my location" and the proposal of moving over to "reversed drop" graphical idiom. With that I'm all fine with commit 19454b6. Regarding the mobile UI as whole, I agree with @ollesmaps that it needs a serious overhaul. While I have made some more changes which I'm happily using in the field, I'm not offering them for merge yet. IMO a big-bang change in direction of mlerjen's proposal in #534 is a better course of action than incremental rework. |
@lpechacek Glad to hear you are back. I will do the merge and also update the documentation page. (I plan to remove the numbers from the screenshot, now that there are icons.) It may still take a few more days until I can make a release which includes Android and macOS. |
Two practical enhancements recently tested in the field. I'm willing to update manual pages if they get merged.
For now, mapping (fieldwork) zoom level is fixed at 1:2. While this is an established practice for forest maps, it may not make sense for sprint maps.