pockethub / PocketHub

PocketHub Android App
Apache License 2.0
9.39k stars 3.47k forks source link

Replace Material Dialogs with androidx Dialog #1246

Open veyndan opened 4 years ago

veyndan commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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