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

Responsive drawer div needs to be slightly deeper #7019

Closed
goobertron opened this issue Aug 22, 2016 · 10 comments · Fixed by #7509
Closed

Responsive drawer div needs to be slightly deeper #7019

goobertron opened this issue Aug 22, 2016 · 10 comments · Fixed by #7509
Labels

Comments

@goobertron
Copy link

#7013 removed the notifications and conversations icons from the header bar and replaced them with items in the drawer menu.

The drawer is now cut off at the bottom, with a scroll bar. I tested this on two browsers: on Firefox 47.0.1, the scroll bar appears only when the user menu has also been opened; on Opera 36.0, the scroll bar appears whether the user menu is closed or open.

header-mobile-open2

One for @svbergerem perhaps?

@svbergerem
Copy link
Member

I tried it with Firefox 45.3 and I only see the scrollbar when the dropdown is higher than 480px (which is exactly what I expected from the code) but that doesn't happen on my pod because I didn't add the “My aspects” link like @Flaburgan did on diaspora-fr.

@AugierLe42e changed the max-height in #6325 from 340px to 480px so now you'll need at least 50px (for the header) + 480px (for the dropdown) = 530px height on your device to display the dropdown without any problems. If the window/your device is not that high the rest of the header will be cut off which is basically something Bootstrap does. Will this be a problem for some devices? Should we go back to 340px? @AugierLe42e what was the reason here to increase the height?

@ghost
Copy link

ghost commented Aug 22, 2016

AFAIR, it was to let the menu be entirely displayed so that there's no need to scroll to reach the last items.

@goobertron
Copy link
Author

This was viewed on my laptop with a narrow browser window, so testing the responsive 'desktop' version rather than the discrete 'mobile' version. The window is higher than 530px - 668px in this screenshot (I cropped out the bottom bit before as I had taken the screenshot to use to illustrate the drawer in the tutorials).

drawer_open

I'm not sure what the best solution is - set max-height on the drawer a bit higher, or give a podmin setting in diaspora.yml to add n number of extra items to the drawer which will add an appropriate amount to the max-height if podmin adds extra items?

@ghost
Copy link

ghost commented Aug 22, 2016

I'm not sure what the best solution is - set max-height on the drawer a bit higher

The risk is to permanently make some menu items unavailable on small devices. If the menu's height is higher than the device's screen, the user won't be able to scroll down to the last items.

@svbergerem
Copy link
Member

set max-height on the drawer a bit higher

You will see a scroll bar depending on the max-height. The dropdown won't be higher than the max-height and if it needs more space then you'll see the scrollbar. Increasing the max-height would mean that parts of the dropdown won't be visible on smaller screens at all.

Let's say your window has a height of 500px. The header has a height of 50px and we currently allow the dropdown to have a height of 480px. This means that header and scrollbar together may be higher than your window and 30px of the dropdown will be missing on your device and you won't be able to see them since there is no scrollbar.

@goobertron
Copy link
Author

goobertron commented Aug 22, 2016

How reliable is max-device-height in media queries? If it works OK, we could set different max-height values for the drawer for shallower and deeper screens. I've only set max-device-width in media queries on sites I've built, so I can't say whether this is a viable solution.

@svbergerem
Copy link
Member

I tend to go back to the Bootstrap default and have a scrollbar there if the dropdown is too high.

@Flaburgan
Copy link
Member

If the notification and conversation icons are kept, our problem is mostly solved. Why have they been removed?

@svbergerem
Copy link
Member

@Flaburgan Because it didn't work on small screens. (First screenshot in #7013)

@SuperTux88
Copy link
Member

Could we display an asterisk instead of the pod name on small screens? (like on the current mobile interface)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants