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

DialogController like DialogFragment #248

Closed
wants to merge 9 commits into from

Conversation

dmdevgo
Copy link

@dmdevgo dmdevgo commented Mar 16, 2017

Implemented DialogController as a wrapper over standard android dialog.

@dmdevgo dmdevgo mentioned this pull request Mar 16, 2017
@hannesstruss
Copy link
Contributor

I haven't looked at the source closely, but could this be just a separate library?

if (mDismissed) {
return;
}
getRouter().popCurrentController();
Copy link
Collaborator

Choose a reason for hiding this comment

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

What if there are multiple dialogs. It would make sense to let the user supply a tag.

import com.bluelinelabs.conductor.changehandler.FadeChangeHandler;

/**
* @author Dmitriy Gorbunov
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove header and add documentation

import com.bluelinelabs.conductor.demo.util.BundleBuilder;

/**
* @author Dmitriy Gorbunov
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove author


private static final String SAVED_DIALOG_STATE_TAG = "android:savedDialogState";

private Dialog mDialog;
Copy link
Collaborator

Choose a reason for hiding this comment

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

No Hungarian style

public void onShow(DialogInterface dialog) {
TextView messageTextView = ((TextView) alertDialog.findViewById(android.R.id.message));
if (messageTextView != null) {
//Make the textview clickable
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why?

Copy link
Author

Choose a reason for hiding this comment

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

Typo, make links clickable in textview

@dmdevgo dmdevgo changed the title AlertDialogs DialogController like DialogFragment Mar 16, 2017
public void showDialog(@NonNull Router router, @Nullable String tag) {
dismissed = false;
router.pushController(RouterTransaction.with(this)
.pushChangeHandler(new FadeChangeHandler(false))

Choose a reason for hiding this comment

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

Wouldn't it be better to let the implementer supply a ChangeHandler instead of forcing one? I guess the showDialog method could be overloaded to allow this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think so too. The dialog needs to call removesFromViewOnPush on the push change handler in that case and throw an exception if it's true.
I also ask myself if it's possible to skip the whole showDialog stuff and use the controller lifecycle callbacks to handle the dialog.

Copy link
Author

Choose a reason for hiding this comment

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

The DialogController return a stub view because a method onCreateView requires non-nullable result. This view not visible for user. At the same time we should not remove the view of controller on top of which the dialog is displayed. Therefore, there are no views that need to be animated. I think apply SimpleSwapChangeHandler(false) instead FadeChangeHandler(false).

private Dialog dialog;
private boolean dismissed;

public DialogController(@Nullable Bundle args) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a no-args constructor too

@esafirm
Copy link

esafirm commented May 8, 2017

Any update on this guys?

@ghost
Copy link

ghost commented Jun 18, 2018

Why use stub view?
This is not obvious that we will get just a stub when using Controller.getView()

There is must be a way to pass Dialog's view as Controller's view.
This is very common when we want to implement custom view for the Dialog.

Now we forced to use getDialog().findViewById() instead of just getView()

@PaulWoitaschek
Copy link
Collaborator

Thank you for your efford! Closing this for the same reason as #467

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.

5 participants