-
-
Notifications
You must be signed in to change notification settings - Fork 319
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
Retry fixing #1869 (bad display on square screen) #2062
Conversation
Although the screen size is reportedly 716x720, it seems that the viewport size is just 665x716.
(BTW, I was able to successfully run this from Android Studio on my phone. However, neither that nor the APK I built there is unable to connect to lichess.org, so I cannot log in to my account and test this more heavily. If that's a dumb mistake and I can fix that easily, happy to test this more thoroughly.) |
Disclaimer: testing in production is not generally recommended. That being said, the default appconfig.dev.json points to lichess.dev. You can create your own e.g. |
Oh, I should have figured that out :( Honestly, even though I can see the docs on running your own Lichess instance, I just thought it might be too much trouble for casual contributions. Also, I was thinking of just building me an APK until this is merged and released, so I can play :) IF a separate PR to extend the docs explaining how to point to lichess.org is welcome, I can do that. (With a big disclaimer on top). But I can see that some might prefer to leave that undocumented, so up to you. In the meantime, if the PR needs any further work to be reviewed, happy to do that too. |
Yeah, unless you're hosting your own lichess instance on a server easily accessible from your phone, it's not worth it. I've found it useful for testing via the webapp or emulator on your local machine or emulator, but not so much for a real device.
I'll have to defer to the maintainer of this repo (veloce) and/or ornicar on that. I'll also defer to veloce on this PR itself - I'm not sure of all the implications of widening the "square" aspect ratio, and he is the final authority on this repo anyway. 🙂 |
@alexpdp7 does it mean that my last release didn't change anything for your screen? If I used a min and max in the media query, it was to handle the screen rotation: since it's not perfectly square we need to deal with that. |
Yeah @veloce ; the media query didn't apply to my screen. I think the root of the issue is that while the screen size is the one I told you (716x720), the status bar and bottom gesture thing reduce the viewport to less than that, so the media query does not apply, as it was very "strict". Your rule applied to aspect ratios from 0.9944 to 1.0056 (716/720 to 720/716). 665/716 is 0.92 actually, so it lies outside that. I suspected the range was due to portrait/landscape. For this particular smartphone, it never makes sense to use landscape (it's got a keyboard south of the screen, so if you do landscape, you have a sideways keyboard to the right... and as the screen is [nearly] square, you don't gain much). I can do some further testing and tell you the exact viewport on landscape (it's probably not 716x665, the status bar and bottom gesture thing will likely eat a different amount of space). |
The screenshot you sent appears as landscape: in the app landscape mode, the board is on the left and controls on the right. In portrait the board would be on top with controls below. To me your changes are fine, I'll merge as is. Thanks! |
Sorry @veloce , I didn't give you the proper data to fix #1869. Although the screen size is reportedly 716x720, it seems that the viewport size is just 665x716 (as reported by Chrome when doing remote debugging)
After applying this change, this is how the screen looks:
I have no idea about media queries, so I'm not exactly sure why you pinned the aspect-ratio between 716/720 and 720/716, but I have preserved that. Happy to make more tests if needed, now that I figured out how to build/debug.