Skip to content

Disable landscape view on phones#6608

Merged
luacmartins merged 3 commits into
mainfrom
cmartins-disableLandscape
Dec 7, 2021
Merged

Disable landscape view on phones#6608
luacmartins merged 3 commits into
mainfrom
cmartins-disableLandscape

Conversation

@luacmartins

@luacmartins luacmartins commented Dec 7, 2021

Copy link
Copy Markdown
Contributor

Details

We decided to disable landscape view on phones, but keep it on tablets.

Fixed Issues

$ #6568

Tests

  1. Launch the app on a phone and verify that the app orientation doesn't change when you rotate the phone.
  2. Launch the app on a tablet and verify that the app orientation changes when you rotate the tablet.

QA Steps

Steps above.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Mobile Web

Desktop

iOS

ios.mov
ipad.mov

Android

android.mov
android-tablet.mov

@luacmartins luacmartins self-assigned this Dec 7, 2021
@luacmartins luacmartins marked this pull request as ready for review December 7, 2021 01:47
@luacmartins luacmartins requested a review from a team as a code owner December 7, 2021 01:47
@MelvinBot MelvinBot requested review from puneetlath and removed request for a team December 7, 2021 01:47
@puneetlath

puneetlath commented Dec 7, 2021

Copy link
Copy Markdown
Contributor

@luacmartins everything looks good. Just one thing -- I was reading this Stack Overflow post and it sounds like there are also some extra large Android tablets out there that wouldn't get captured via res/values-sw600dp but would get captured by adding res/values-xlarge . Do you think we should add that as well?

@luacmartins

Copy link
Copy Markdown
Contributor Author

I decided to leave that out because it only affected devices running on pre-Android 3.2 (released 2011). However, I guess it's simple enough to support those as well, so I'm including it here. Thanks for the input!

Just a note for my future self: We actually only need res/values-large (equivalent to values-sw600dp) since old android versions (< 3.2) use device size, i.e. small, medium, large, etc and not screen width, i.e. 600px, 720px, etc. Any devices larger than large or 600px would be caught in the values-large and values-sw600dp config.

@puneetlath

Copy link
Copy Markdown
Contributor

Looks good to me! Can merge once tests finish.

@luacmartins

Copy link
Copy Markdown
Contributor Author

Self-merging with approval!

@luacmartins luacmartins merged commit 3b5c680 into main Dec 7, 2021
@luacmartins luacmartins deleted the cmartins-disableLandscape branch December 7, 2021 18:05
@OSBotify

OSBotify commented Dec 7, 2021

Copy link
Copy Markdown
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify

OSBotify commented Dec 8, 2021

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by @luacmartins in version: 1.1.18-4 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@mvtglobally

Copy link
Copy Markdown

@luacmartins @puneetlath @roryabraham
We were able to pass this PR on iPad, iPhone and Android phone. Do we need to verify Android Tablet as well?

@roryabraham

Copy link
Copy Markdown
Contributor

@mvtglobally while that would be ideal, let's not block deploy on it. We can check it off but let's try to remember to test this at some point?

@luacmartins

Copy link
Copy Markdown
Contributor Author

I agree with @roryabraham! I asked for some testing help in Slack.

@mvtglobally

Copy link
Copy Markdown

@luacmartins @roryabraham , We were able to get Android tablet user verify this PR today and its a Pass. Checking off

@luacmartins

Copy link
Copy Markdown
Contributor Author

That's great @mvtglobally! Thank you so much!

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by @Julesssss in version: 1.1.21-1 🚀

platform result
🤖 android 🤖 failure ❌
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

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.

5 participants