-
Notifications
You must be signed in to change notification settings - Fork 39
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
Landscape dialogs do not use all available height, resulting in small layouts. #29
Comments
It is possible to calculate the height required for the dialog. Using 6 calendar rows of 1:1 aspect ratio views, and accounting for the vertical spacing and button heights, the height will exceed the available height of the dialog if the guidelines are followed (which I would like to use otherwise). If fixed sizes are used, as per the guidelines, the height of the dialog in landscape mode may exceed the available height, depending on the screen size of the smartphone. Do you have any solution for this issue? Unfortunately, the guidelines do not provide any information or specifications for layouts in landscape mode on phones, as it is often limited by space. Please correct me if I am wrong. |
Well to my mind the paddings for the dialog are too large, don't you agree? Taking a look at the screenshot, around 50% of the height is used. What we could do is set the size according to margins (as is done now), but make them smaller and also constrain the max size of the dialog so that it does not appear huge on tablets. I would use 90% of the available height of the display but set maxHeight to somewhere around 500dp. |
Which paddings do you mean? The border of the dialog to the content? That's 24 dp and follows directly the guidelines: I reduced the vertical spacing between buttons to content from 24dp to 16dp. |
No, the padding from the screen edge to the dialog card border. |
Can you tell me the solution that you have in mind? Stretching the height of the dialog for all use-case dialogs won't work. Making it optional for some, will look weird and might behave weird as well. |
I think what we can do is set a minimum size of either the dialog or the widgets used in the layout of that dialog so that they are simply usable. We've got a minimum landscape device screen height: I think we can assume the smallest phone screen will be at least 480dp in width as per the guidelines. You can safely design a layout that's as big as 480dp in its biggest dimension. In my opinion, the only thing left will be designing such a layout that is also accommodating the minimum touch area requirement of 48x48dp of the accessibility guidelines, that will be up to your designer part of the brain I guess :) |
While I can now overcome the height problem, somewhat. I noticed that the platform width for the dialog does not automatically scale depending on the child's width. If I force a larger width of the child view, it will just be cut away due to the fixed dialog width. Seems super odd. I'm at the point, where I would just enforce a switch from the monthly view to the weekly view when it's in the landscape mode... |
That doesn't solve the problem for other dialogs though. I suspected that the dialog may be constrained by the platform in some way (the Popup you use under the hood). It may be worth checking that out, and if that's true, we are stuck with using those constraints. Maybe making the layouts more compact could solve the problem then? |
Reverting to 1.0.4 fixes the problem: I think the bug is from this change |
After updating to 1.1.0, we noticed that on landscape, dialogs are sometimes unnecessarily small (depends on target device dpi)
Is this intended? The button sizes and paddings do not follow the Android accessibility guidelines in most landscape layouts.
See attached screenshots for examples. Calendar dates' click areas are less than 18dp, whereas the minimum is 48x48dp. I think that the issue stems from paddings being too large for layouts - you can see that the landscape dialog occupies a very small portion of the screen.
Screenshots
The text was updated successfully, but these errors were encountered: