Skip to content
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

Reenable rotation to and from landscape mode on Android #470

Merged
merged 11 commits into from
Jan 27, 2022

Conversation

maxammann
Copy link
Member

fixes #419

@michael-markl
Copy link
Member

I just installed the app-release.apk from https://app.circleci.com/pipelines/github/digitalfabrik/ehrenamtskarte/912/workflows/8920355f-b301-4e08-bc96-1f93d2031941/jobs/6414/artifacts, but I found the issue sitll persists. I can rotate the app again, but there is still an area (at the right side in my case) after rotating to landscape which is unresponsive. :/

@maxammann
Copy link
Member Author

hm I suspect that I only fixed it for debug mode :/ I'll test it again.

@maxammann
Copy link
Member Author

@Schedulaar feel free to test again :)

@michael-markl
Copy link
Member

@maxammann I am sorry that I have to disappoint you, but it's still not working on my device :/

@maxammann
Copy link
Member Author

hm that it weid worked in release mode on my device

@maxammann
Copy link
Member Author

Oke, weird the ci build is not working 🤔

@maxammann
Copy link
Member Author

I definitely have a release build which is working locally. But the CI build is not working

@maxammann
Copy link
Member Author

@Schedulaar it seems only not to work if the intro slides are shown before that. Can you try to restart the app (latest build) and see whether the issue is still there?

@michael-markl
Copy link
Member

Yep, it does indeed work after an app restart. However, I have noticed that sometimes (like 1 out of 50 times) there's a black screen where the map should be...

@maxammann maxammann force-pushed the 419-fix-map-orientation branch from 8edd4f9 to 558dd47 Compare January 25, 2022 17:24
@maxammann
Copy link
Member Author

Hm that is weird, maybe the black screen is even unrelated.

Last try is pushed. The change which I made is worth discussing. Maybe that is actually the better behavior for the app in general.
First start with the intro route in focus. After the intro slides are done, replace them with the main route. Before this PR the intro slide was "above" the main route.

Copy link
Member

@michael-markl michael-markl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems (almost) perfect! Having the intro slides not above the main route is way better in my opinion. That way, if the location permissions are given in the intro slides, the app will start initialized properly after finishing the intro slides.

frontend/lib/entry_widget.dart Outdated Show resolved Hide resolved
frontend/lib/map/map/screen_parent_resizer.dart Outdated Show resolved Hide resolved
frontend/lib/map/map/screen_parent_resizer.dart Outdated Show resolved Hide resolved
@maxammann
Copy link
Member Author

@Schedulaar I think my beauty if finished now. Maybe check one last time.

The black screen was actually introduced in my commits. I fixed this now.

@steffenkleinle
Copy link
Member

@maxammann I'd review this tomorrow if you want :)

Copy link
Member

@michael-markl michael-markl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again closer to perfect, but not yet exactly there: When I rotate to landscape (given I start the app in portrait mode), the info icon is again not shown (because the map's actual size is the huge square).

Accordingly, if the app is started in landscape mode and then rotated to portrait mode, the same thing happens.

@maxammann
Copy link
Member Author

@Schedulaar thanks for noticing! I get now the correct constrains within the map should be placed.

Copy link
Member

@michael-markl michael-markl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buttery smooth now 😍

frontend/lib/map/map/screen_parent_resizer.dart Outdated Show resolved Hide resolved
@maxammann
Copy link
Member Author

@klinzo feel free to test tomorrow :) then I'll merge this!

Copy link
Member

@steffenkleinle steffenkleinle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on android, did not experience any unresponsiveness.

frontend/android/app/src/main/AndroidManifest.xml Outdated Show resolved Hide resolved
@maxammann maxammann enabled auto-merge January 27, 2022 14:37
@maxammann maxammann merged commit 5f0564d into main Jan 27, 2022
@maxammann maxammann deleted the 419-fix-map-orientation branch January 27, 2022 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reenable rotation to and from landscape mode on Android
3 participants