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

Replace Material Dialogs with androidx Dialog #1246

Open
veyndan opened this issue Jul 1, 2019 · 3 comments
Open

Replace Material Dialogs with androidx Dialog #1246

veyndan opened this issue Jul 1, 2019 · 3 comments

Comments

@veyndan
Copy link
Collaborator

veyndan commented Jul 1, 2019

I understand that this has been a (touchy) subject that has come up a couple of times before (#896, #924), but the state of affairs hasn't be reevaluated since those discussions five years ago.

As mentioned by @ZacSweers here, one of the main reasons for using Material Dialogs is that they use sensible widths for dialogs on tablet. To see the validity of this statement as it stands today, I've taken an assortment of screenshots taken on a phone (Pixel 2, Android Pie) and a table (Nexus 10 emulator, Android Pie) comparing Material Dialog and the androidx dialog.

Phone

Current (Material Dialog) Proposed (Androidx Dialog)
phone_horizontal new_phone_horizontal
phone_vertical new_phone_vertical

Tablet

Current (Material Dialog) Proposed (Androidx Dialog)
tablet_horizontal tablet_horizontal
tablet_vertical tablet_vertical

The margins are virtually indistinguishable between the Material Dialog and androidx dialog.

I must admit that the second argument made in the same comment regarding Material Dialog's supposed superior customizability has not been investigated, though if not true has not been obviously utilized by the app.

Therefore, my proposition is to replace the use of Material Dialogs with the dialog provided by androidx. Such a switch allows the removal of an seemingly unnecessary dependency.

@vivekshivarajkumar
Copy link

@veyndan You can fork the Repo and upgrade the build and compile sdk to 28 and Migrate to Androidx! I don't think it is a core problem.

@veyndan
Copy link
Collaborator Author

veyndan commented Jul 15, 2019

Please read the contributing guidelines before making snarky comments. It's true that this isn't a core problem, but there is a reason why I didn't label it "priority".

@Meisolsson
Copy link
Contributor

@veyndan I think this is will be good. If you have this ready feel free to make a PR.

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

No branches or pull requests

3 participants