criticalmaps / criticalmaps-android

🤖 Critical Maps Android App
http://criticalmaps.net
Apache License 2.0
136 stars 27 forks source link

Make map rotateable #219

Closed stephanlindauer closed 6 years ago

stephanlindauer commented 6 years ago

i got an email with that feature request from a user. i think we should enable that feature. please review

cbalster commented 6 years ago

Any specific reason why this points to master? Do you want to do this as a hot fix? Besides that the change works fine on top of develop, although I think we should also add a compass with reorientation to North onclick.

stephanlindauer commented 6 years ago

@cbalster sorry i got to used to where i point my PRs at work. :wink: fixed it. the reorientation button is a good idea. i started implementing it and now i am not sure where to get a nice icon from that looks similar to the ones we already have. any recommendations?

stephanlindauer commented 6 years ago

maybe this https://material.io/tools/icons/?icon=vertical_align_top&style=baseline ?

stephanlindauer commented 6 years ago

or this rotated 45 degrees: https://material.io/tools/icons/?icon=all_out&style=baseline

cbalster commented 6 years ago

Or maybe something like https://material.io/tools/icons/?icon=navigation&style=baseline that rotates with the map and re-aligns to north onClick - similar to the google maps app? Maybe there is some implementation like this in the osmdroid samples?

stephanlindauer commented 6 years ago

@cbalster ok i got this working. it was a bit of a struggle after after my android skills are quite rusty. would be nice if you could have another look.

cbalster commented 6 years ago

Nice! I like it. It would be nice to have it rotate in sync with the map while dragging, but I guess osmdroid makes this hard? On thing I noticed is when rotating past the 0°/360° border (e.g. 350 to 10), the icon behaves a little odd like does a full spin + the rotation. Can you look into that? Besides that can you remove the device id removal stuff since we already have this in another commit in develop and maybe rebase your commit onto develop so we can avoid the huge merge commit?

stephanlindauer commented 6 years ago

but I guess osmdroid makes this hard?

nope actually i removed the animation nonsense and now its more in-sync :wink: also i made the map rotate with the button animation when resetting the orientation. looks a bit smoother now.

can you remove the device id removal stuff

yea sorry i rebased to master again :grimacing:

the icon behaves a little odd like does a full spin

probably just a missing modulo somewhere. will fix tomorrow.

stephanlindauer commented 6 years ago

the icon behaves a little odd like does a full spin

fixed as well. please take another look.

cbalster commented 6 years ago

Seems to need a setRotationNorth.setRotation(mapView.getMapOrientation()) in onActivityCreated() to sync orientations on start, but otherwise good to go as far as I am concerned. :+1: Also orientation state should be saved and restored but that could also be done in another pr.