Skip to content

Conversation

@Andreas-Kainz
Copy link
Contributor

Signed-off-by: Andreas-Kainz [email protected]

Summary

richdocumentframe use 50px less than screen height, which cause ain issue on mobile.

Before

before

After

after

@Andreas-Kainz
Copy link
Contributor Author

@juliushaertl, @eszkadev, @pedropintosilva I only know that I still have the sizing issue on mobile (all browsers) at https://staging.eu.collaboraonline.com server. Maybe the sizing issue is already fixed.

Please let me know.

@juliusknorr
Copy link
Member

The 50px offset was needed for conditions where the height could not be properly determined through css on larger iOS devices, though I haven't considered that we actually hide the Nextcloud toolbar on smaller ones.

It is determined by the viewport width

@media only screen and (max-width: 768px) {
so it would make sense to only use those 50px if the header is visible then.

@juliusknorr juliusknorr added 2. developing Work in progress bug Something isn't working labels Oct 5, 2021
@Andreas-Kainz
Copy link
Contributor Author

If I test try.nextcloud.com or the colabora staging and demo nextcloud account, everywhere I have this issue on my phone. If I remove the max-height: 752px everything work fine. I'm no expert, but I reviewed different machines and everywhere it work within the css rule.

Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

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

Resizing to 100%-50px would still be required for larger devices (this rule is actually only necessary for Safari on iOS which doesn't work well with the calc(100vh - 50) that is applied form css.

In order to get this in I'd suggest to do a check like this to not apply the 50px offset for the nextcloud header if mobile.css styles apply.

@Andreas-Kainz
Copy link
Contributor Author

The screenshots were done from my fairphone 3 (1,080x2,160 pixels) with android and chrome or firefox as browser. So there is the issue already, not only on ios with Safari.

I'm sorry that my technical expertise is not good enough to review the suggested change. Is there somewhere a pre-build where I can test the change on the phone? Or at least after the PR was merged to have a look if the issue is fixed?

@juliusknorr
Copy link
Member

The screenshots were done from my fairphone 3 (1,080x2,160 pixels) with android and chrome or firefox as browser. So there is the issue already, not only on ios with Safari.

Moobile phones might report a different smaller screen size based on the devicePixelRatio https://developer.mozilla.org/en-US/docs/Web/API/Window/devicePixelRatio

Your fix is totally valid, just that it reverts that the resize 50px offset for iOS.

I'm sorry that my technical expertise is not good enough to review the suggested change. Is there somewhere a pre-build where I can test the change on the phone? Or at least after the PR was merged to have a look if the issue is fixed?

There is no preview environment, but I've committed the change directly to your branch. I need to do a real testing on iOS again, but then we could get this in from my side 👍

@Andreas-Kainz
Copy link
Contributor Author

As the fix is valid from your side, push it (when CI is ready)

@Andreas-Kainz
Copy link
Contributor Author

can the MR get to master?

@juliusknorr juliusknorr merged commit f4f21a8 into nextcloud:master Oct 25, 2021
@juliusknorr
Copy link
Member

/backport to stable4

@juliusknorr
Copy link
Member

/backport to stable3.8

@backportbot-nextcloud
Copy link

The backport to stable4 failed. Please do this backport manually.

@juliusknorr
Copy link
Member

/backport to stable22

@backportbot-nextcloud
Copy link

The backport to stable22 failed. Please do this backport manually.

@juliusknorr
Copy link
Member

/backport to stable4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developing Work in progress bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bit of file list visible after resizing window

2 participants