Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Increased support for camera movement via keyboard #15736

Conversation

langsmith
Copy link
Contributor

@langsmith langsmith commented Oct 1, 2019

This pr resolves #10131 by bringing more keyboard to the Maps SDK for Android. It is a follow up to #14789.

Regarding @LukasPaczos' comment at #14789 (review): All the code is now in the MapKeyListener's onKeyDown/onKeyLongPress methods.

Regarding @tobrun's comment at #14789 (comment): MapGestureDetector no longer has any key listening code. All the code is contained in MapKeyListener, which I think is appropriate.

So far, this pr adds support for:

The recommended way to try out this pr's work is to open the Maps SDK's test app on an emulator. Make sure the emulator supports keyboard input. Chances are good that the emulated device does. Open any test app example that loads a map.

  • directional navigation to adjust the focus

    • Tap the Tab key on your keyboard to see the app's focus cycle between the MapView, the CompassView (if it's visible), and the attribution i. Pressing the Enter key when the focus is on a particular view ID, is the equivalent of tapping on the view with your finger.
  • adjusting map pitch via shift key and +/- buttons

    • Hold down shift on your keyboard and then tap the up or down arrow. You can also hold either arrow. The up/down arrows should adjust the map camera's pitch. The up arrow will tilt the map plane "down", which matches the behavior in Mapbox Studio.
  • adjusting map bearing via shift key and directional left/right arrows

    • Hold down shift key and then tap the left or right arrows. You can also hold either arrow. shift + left will move the camera bearing so that the map camera appears to move in a clockwise direction around the focal point. shift + left will move the camera bearing so that the map camera appears to move in a counter-clockwise direction around the focal point.
  • adjusting map zoom via +/- buttons

    • - will move the camera farther away from the map by decreasing the zoom value. + will move the map camera closer to the map by increasing the zoom value.

The map movement in the GIFs below is all based on keyboard keys.

ezgif com-resize
ezgif com-resize (1)

Debug toggling and switching to Style.MAPBOX_STREETS via keyboard

ezgif com-resize (2)

@langsmith langsmith added Android Mapbox Maps SDK for Android ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold needs review labels Oct 1, 2019
@langsmith langsmith requested a review from a team October 1, 2019 01:45
@langsmith langsmith self-assigned this Oct 1, 2019
@langsmith
Copy link
Contributor Author

2 remaining hurdles here:

  1. Getting the bearing logic correct so that changes in bearing are smooth.

As you might notice in the 2nd GIF above, the bearing jumps after holding down shift and tapping the other direction. For example, holding down shift + right for a couple of seconds and then pressing shift + left. I'm not sure what's the best way to handle the math of the bearing so that it just rotates in the opposite direction without any jumping. The logic goes in at https://github.com/mapbox/mapbox-gl-native/pull/15736/files#diff-5a9c1c39bb734de93bf9bc0de7706757R76-R82 and I've tried a bunch of different things, mainly with transform.getBearing().

Thoughts?

  1. If tests need to be written for tilt/bearing/zoom changes via keyboard, I still haven't found a way to mock the holding down of a key with Espresso.

@tobrun
Copy link
Member

tobrun commented Oct 1, 2019

adjusting map zoom via alt key and directional up/down arrows

I think the most natural UX will come from using control + and control - or is the Android OS using those already for other purposes?

toggle debug mode

nice feature but should only be possible for debug builds. We don't want end users enabling this.

switch the map style to Style.MAPBOX_STREETS.

I feel we shouldn't allow this functionality for end users, this could potentially break the interaction that a developer tries to achieve and break their application (they aren't managing style changes, they only have 1 style)

@langsmith
Copy link
Contributor Author

I think the most natural UX will come from using control + and control - or is the Android OS using those already for other purposes?

Ok, my latest commit has switched zoom and pitch adjustments to + and -. I've updated OP above.

For some reason, isCtrlPressed() isn't registering when control is being held down. Doesn't matter whether it's my external keyboard or laptop keyboard.
When control is held down and a letter is pressed, neither key comes through onKeyDown(). You're welcome to try it on your setup. event.isShiftPressed() works for all map camera movement and I'm fine with sticking with shift.

nice feature but should only be possible for debug builds. We don't want end users enabling this.

I have the check for BuildConfig.DEBUG before the style is switched.
. I believe this would be enough to prevent end-users from displaying debug mode. Correct?

I feel we shouldn't allow this functionality for end-users, this could potentially break the interaction that a developer tries to achieve and break their application (they aren't managing style changes, they only have 1 style)

I saw mention of load style in #10131 (comment)

Screen Shot 2019-10-01 at 2 12 06 PM

and thought that's what you meant. I have the check for BuildConfig.DEBUG before the style is switched, so that developers' production-level apps don't break. Thought this would be enough. I'm fine with removing the style switch all together though.

@tobrun
Copy link
Member

tobrun commented Oct 2, 2019

Ok, my latest commit has switched zoom and pitch adjustments to + and -. I've updated OP above.

the arrow keys implementation was great for pitch, the request for zooming like this just came from other programs where the zoom action is associated with it.

For some reason, isCtrlPressed() isn't registering when control is being held down. Doesn't matter whether it's my external keyboard or laptop keyboard.

This means the platform is already consuming the event and we can't use it

Apologies for the confusion in above 2 sections, can we move forward with your initial setup of arrow keys?

@tobrun
Copy link
Member

tobrun commented Oct 2, 2019

I saw mention of load style in #10131 (comment)

That was more specific for inclusion of the test app not for all end-users (even with debug build).

@langsmith langsmith changed the title Increased support for camera movement + debug actions via keyboard Increased support for camera movement via keyboard Oct 2, 2019
@langsmith
Copy link
Contributor Author

I like your suggestion of + & - for zoom.

In my latest commit, I've removed all DebugActionsKeyListener interface stuff from the actual MapView & MapKeyListener. Adding these debug bells & whistles to the test app can be handled in a separate pr.

Keyboard controls are now:

Zoom: + & -

Pitch: shift + &

Bearing: shift + &

@langsmith langsmith force-pushed the langsmith-increased-support-for-camera-movement-via-keyboard branch 2 times, most recently from 6d6c367 to b75e5ae Compare October 4, 2019 23:54
@langsmith
Copy link
Contributor Author

@tobrun , I need to finish tests, but are you 👍 with the setup as described in the comment above?

@langsmith langsmith force-pushed the langsmith-increased-support-for-camera-movement-via-keyboard branch from b75e5ae to 867edcb Compare October 7, 2019 22:33
@tobrun
Copy link
Member

tobrun commented Dec 3, 2019

We are removing platform code from gl-native in #15970. If you want to see this PR merged, it will require re-implementation in https://github.com/mapbox/mapbox-gl-native-android.

@tobrun tobrun closed this Dec 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold needs review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Revisit keyboard/mouse events
2 participants