maxkeppeler / sheets-compose-dialogs

✨ A Multiplatform Library to enhance UIs, supporting a wide range of common use-cases with Dialogs, Popups, and Bottom Sheets. ✨
https://maxkeppeler.github.io/sheets-compose-dialogs/
Apache License 2.0
814 stars 38 forks source link

Landscape dialogs do not use all available height, resulting in small layouts. #29

Open Nek-12 opened 1 year ago

Nek-12 commented 1 year ago

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 ![Screenshot_20230220-143221](https://user-images.githubusercontent.com/46318640/220887353-6267162c-f657-4981-a142-9e4bcae67ce5.png) ![Screenshot_20230220-143213](https://user-images.githubusercontent.com/46318640/220887362-247ca27c-f432-4f47-9ed2-a1dacf317609.png)
maxkeppeler commented 1 year ago

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.

Nek-12 commented 1 year ago

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.

maxkeppeler commented 1 year ago

Which paddings do you mean? The border of the dialog to the content? That's 24 dp and follows directly the guidelines: https://m3.material.io/components/dialogs/specs

I reduced the vertical spacing between buttons to content from 24dp to 16dp.

Nek-12 commented 1 year ago

No, the padding from the screen edge to the dialog card border.

maxkeppeler commented 1 year ago

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.

Nek-12 commented 1 year ago

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 :)

maxkeppeler commented 1 year ago

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...

Nek-12 commented 1 year ago

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?

risalfajar commented 4 months ago

Just want to add, this is what happened in mine CleanShot 2024-06-03 at 23 25 23

I think this is a bigger problem, what can I do?

risalfajar commented 4 months ago

Reverting to 1.0.4 fixes the problem: CleanShot 2024-06-04 at 00 51 37

I think the bug is from this change