Skip to content

Fix android modal width #152

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

litinskii
Copy link

For iOS platform we have fixed width
image
in this case when we have dynamic content like input and user type something modal container not increase witdh, so we need to do the same for other platform.

Before
https://user-images.githubusercontent.com/10028441/209297979-cc201a21-63e5-4237-ba0d-4a9f7899f094.mov

After
https://user-images.githubusercontent.com/10028441/209298011-543a4ed1-f20e-49b2-805a-e84fa27a20da.mov

Overview

Test Plan

@litinskii
Copy link
Author

@mmazzarolo ^

Copy link
Owner

@mmazzarolo mmazzarolo left a comment

Choose a reason for hiding this comment

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

Hola! Thanks for reporting and creating a PR. Wondering it we want a fixed-witdth value or something that scales a bit with the device size? Do you know what the iOS/Android guidelines say about this?

@litinskii
Copy link
Author

The problem is that now in this lib, you have different behavior for ios/android styles for container, if you check code you will see that for ios you have width style, for android you have minWidth.

image

image

I believe behavior should be the same, need to use minWidth or width, but the same in ios/android.

@litinskii
Copy link
Author

btw if you check dialog in material https://m2.material.io/components/dialogs#usage dialog not maked wider if child content increasing, which is impossible if you use minWidth and overflow hidden, and there are no point to use overflow hidden if you not use fixed width

@litinskii
Copy link
Author

@mmazzarolo do you have take a look ?

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

Successfully merging this pull request may close these issues.

None yet

2 participants