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

Fix height to big for iPhone when using many apps #10276

Closed
wants to merge 2 commits into from

Conversation

kevin147147
Copy link
Contributor

No description provided.

@kevin147147 kevin147147 changed the title Fix height to big for iPhone Fix height to big for iPhone when using many apps Jul 17, 2018
@kevin147147
Copy link
Contributor Author

@MorrisJobke

@MorrisJobke
Copy link
Member

cc @nextcloud/designers

@MorrisJobke
Copy link
Member

@kevin147147 Could you explain the fix and also explain the issue? best with screenshots of before and after?

@MorrisJobke MorrisJobke added design Design, UI, UX, etc. 3. to review Waiting for reviews labels Jul 17, 2018
@kevin147147
Copy link
Contributor Author

Before

e8dd6d23-f036-4683-8ee9-8316e767f23d

After
4bad7297-2b03-4abb-b2dd-584b568d2f18

@kevin147147
Copy link
Contributor Author

Look at the bottom of the screenshots. The Dropdown-Menu is now fully visible. Before you could only barely touch the last entry.

It has to do with using the viewport height:
https://nicolas-hoizey.com/2015/02/viewport-height-is-taller-than-the-visible-part-of-the-document-in-some-mobile-browsers.html

@@ -324,7 +324,7 @@ nav[role='navigation'] {
/* Apps management */

#apps {
max-height: calc(100vh - 100px);
max-height: calc(100vh - 200px);
Copy link
Member

Choose a reason for hiding this comment

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

Could you change this to inherit then it looks perfect ;) We just tested it over here and then it looks how it is supposed to.

@MorrisJobke
Copy link
Member

While doing this, amend the commit and add a sign-off message to the commit ;) See https://github.com/nextcloud/server/blob/master/CONTRIBUTING.md#sign-your-work for more details

@kevin147147
Copy link
Contributor Author

max-height: inherit doesn't do it for me. It then has the same height as before and the menu gets under the safari bar again like before. How did you test it? With a real iOS device? Sadly the responsive design modes in chrome, etc. will not show this bug.

@jancborchardt
Copy link
Member

On small screens this would cut off too much? If it only is an issue in Safari mobile, can it be solved differently? (Like max-height: 80% or something else.)

@skjnldsv
Copy link
Member

skjnldsv commented Jul 18, 2018

You can also set it to 80vh on mobile yes :)
Just put max-height: unset to .header-left #navigation also then

@kevin147147
Copy link
Contributor Author

This is how it looks even on horizontal. What smaller devices are you thinking about?

62e531d9-c62c-43d7-991e-0cd288882c4c

@jancborchardt
Copy link
Member

The issue is that there might always be smaller devices out there. iPhones are among the largest, and doing the calculation like "100% minus a set amount of pixels" just seems like not a good way to calculate (yes it was there before, but let's not make it worse :)

Could you try with the 80vh as @skjnldsv mentioned? Thank you 😊

@kevin147147
Copy link
Contributor Author

kevin147147 commented Jul 18, 2018

Well 80vh just ever so slightly does it in vertical. In horizontal it looks nearly like above just a litte bigger :)

f65e1833-f0f0-4cbd-8cb2-1b2c7c3008d6

@MorrisJobke
Copy link
Member

Let's wait here for #9982 as well, because it fixes the issue on iOS that the control bar of the browser is always there.

@ChristophWurst
Copy link
Member

Let's wait here for #9982 as well, because it fixes the issue on iOS that the control bar of the browser is always there.

That PR was merged. @kevin147147 could you please re-verify that the patch is still necessary? There have been lots of changes and fixes for Nextcloud 14 with the apps menu and there's the possibility that your issue has been fixed by that. Thanks!

@kevin147147
Copy link
Contributor Author

That works

@ChristophWurst
Copy link
Member

Perfect. Thank you for your feedback!

@MorrisJobke MorrisJobke added this to the Nextcloud 14 milestone Sep 24, 2018
kevin147147 added a commit to kevin147147/server that referenced this pull request Jul 25, 2020
Same story as nextcloud#10276
I'm testing on iPhone without home button. These devices now have an even higher bottom bar.
backportbot-nextcloud bot pushed a commit that referenced this pull request Jul 30, 2020
Same story as #10276
I'm testing on iPhone without home button. These devices now have an even higher bottom bar.
MorrisJobke pushed a commit that referenced this pull request Jul 31, 2020
Same story as #10276
I'm testing on iPhone without home button. These devices now have an even higher bottom bar.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews design Design, UI, UX, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants