1

I am working on an android application with many dialogs, all of which extend a custom class called "DialogFragmentBase" which extends the library's "DialogFragment". All activities use the show() method overridden in DialogFragmentBase.

I want to prevent showing the dialogs if the parent activity is backgrounded (as in receiving a phone call) since that results in the illegalStatEexception, but at the same time I don't want to guard every show() call in the application with: 

if(getLifecycle().getCurrentState().isAtLeast(Lifecycle.State.RESUMED))

So I wanted to do something like this in the DialogFragmentBase:

@Override
public void show(FragmentManager manager, String tag){

    List<Fragment> fragments = manager.getFragments();
    if(fragments != null && fragments.size() > 0){
        FragmentActivity activity = fragments.get(fragments.size()-1).getActivity();
        if(activity != null && activity.getLifecycle().getCurrentState().isAtLeast(Lifecycle.State.RESUMED)){
            super.show(manager, tag);
        }
    }
}

So my question is: Is accessing the previous fragment like that considered a bad practice? It does work...but I remember reading somewhere that fragments shouldn't intercommunicate. If it is indeed a bad practice, what would be a better solution that can be implemented in the DialogFragmentBase (to avoid adding guards everywhere).

Note: I tried the onSaveInstanceState solution described here, but since the DialogFragment hasn't been shown yet, the onSaveInstanceState isn't called for it at that point. Also getActivity() returns null since onAttach hasn't been called yet at that point.

Thanks!

Menna Mostafa
  • 25
  • 4

2 Answers2

0

Perhaps you could create a new method in your dialog base class:

public void showIfResumed(FragmentActivity activity, String tag) {
    if (/* your check here, e.g. activity.getLifecycle()...  */) {
        show(activity.getSupportFragmentManager(), tag);
    }
}

And then, in your activities, rather than calling show(getSupportFragmentManager(), TAG), you could call showIfResumed(this, TAG). You'll still have to change every call to show() to a call to showIfResumed(), but this would reduce the code duplication considerably.

Ben P.
  • 52,661
  • 6
  • 95
  • 123
  • thanks that was my next option, but is the solution of accessing the fragments list a bad practice? because it allows me to not change anything out of the DialogFragmentBase. While with your solution I'd have to change a lot and I am hoping for a solution that can be easily enforced instead of depending on devs to call the showIfResumed which is not intuitive. – Menna Mostafa Aug 09 '18 at 19:24
  • @MennaMostafa I would say yes, what you posted in your question is a bad practice. Why is your fragment interrogating the fragment manager about the other fragments is is managing? What happens if there isn't a "previous" fragment? What happens if the "previous" fragment isn't attached to an activity? Etc. You could potentially change your existing `show()` method in the base class to throw an `UnsupportedOperationException` so that anyone who used it would immediately see that they should not. – Ben P. Aug 09 '18 at 19:28
0

The support library FragmentManager has an isStateSaved() method. Depending on exactly what your requirements are, you could leverage this to check if it is "safe" to show your dialogs:

@Override
public void show(FragmentManager manager, String tag) {
    if (!manager.isStateSaved()) {
        super.show(manager, tag);
    }
}

Note that the above implementation is relatively similar to using commitAllowingStateLoss(), in that you'll silently fail to show your dialog in certain cases. But perhaps that is fine for your requirements.

Ben P.
  • 52,661
  • 6
  • 95
  • 123
  • isStateSaved() returns false even if the parent activity is already in the background. I'm starting to think that maybe `commitAllowingStateLoss()` isn't such a bad approach (at least in my case, since the dialog I want to show the user is useless if the app is in the background anyways). – Menna Mostafa Aug 09 '18 at 20:02
  • Generally, if the parent activity is in the background but the state hasn't been saved yet, that's a fine time to show dialogs. When the user comes back to the app, the dialog will be visible. – Ben P. Aug 09 '18 at 20:05
  • Really? that is not what I am seeing, I am getting `illegalStateException` Could it be because I am using `show()` not `commitNow()`, so by the time that the show is scheduled the state has already been saved? – Menna Mostafa Aug 09 '18 at 20:11
  • Assuming the exception you're getting is the `can not be performed after onSaveInstanceState()` one, then yeah. What's important is whether or not the fragment manager has saved its state; if it hasn't then any transaction is safe. But you're right, I can believe that there are scenarios where it's possible to add a transaction to the queue and then have the fragment manager save its state before that transaction is processed. – Ben P. Aug 09 '18 at 20:20
  • So I managed to get it working by doing this: if(!manager.isStateSaved() && !savedInstanceStateDone) { transaction.commitNow(); } Does this look okay? – Menna Mostafa Aug 10 '18 at 15:26
  • Seems reasonable to me – Ben P. Aug 10 '18 at 16:30