search

avast / android-styled-dialogs

Backport of Material dialogs with easy-to-use API based on DialogFragment
Apache License 2.0
2.15k stars 450 forks source link

Current listener structure is clunky #5

Closed yarian closed 11 years ago

yarian commented 11 years ago

Hey,

I'd like to user your library but I find the targetFragment set up for defining positive and negative listeners to be annoying.

It would be significantly better to be able to set a listener on your dialog directly. Forcing you to have the fragment implement some listener interface fails because now all the state you have available when you launch the dialog has to be translated somehow to the onPositiveButtonClicked method, which would probably mean adding a ton of member variables where you had none before.

Does this make sense?

davidvavra commented 11 years ago

This design is there on purpose - because in case your dialog is opened and you rotate the screen, underlying Activity is destroyed and the listener as well. It could work for retained fragments, but we cannot rely on that. That's why we are forcing in the API to implement those interfaces.

yarian commented 11 years ago

I see. Your reasons are fair. We handle that at the fragment/activity level but I guess you can't depend on all your users doing that.

yarian commented 11 years ago

Hey David,

If I wanted to do a radical re-work of this library to meet my very specific needs (and put it up publicly on github, under the same Apache license). Do you have any problem with that?

If so, would it be okay to move the Apache license to a NOTICE.txt file at the project level instead of including it per-file?

davidvavra commented 11 years ago

This is how open-source works - create a fork, change it how you like, you don't even have to ask :) If you think some parts could benefit others as well, send a pull request. GitHub makes it super-easy.

jenzz commented 10 years ago

I have created a hybrid of both variants that lets you specify either an inline click listener or a callback to the hosting Activity / Fragment.

It's based on retaining the DialogFragment's instance state so obviously there's the danger of leaking a Context. But hey, if you don't know what you're doing, don't do it :)

Here's the fork: https://github.com/jenzz/android-styled-dialogs