sockeqwe / mosby

A Model-View-Presenter / Model-View-Intent library for modern Android apps
http://hannesdorfmann.com/mosby/
Apache License 2.0
5.49k stars 841 forks source link

Mosby 3 MVP - presenter is always recreated when go back to the fragment in the back stack #265

Closed contrudar closed 6 years ago

contrudar commented 7 years ago

Hello, I discribed the problem on stackoverflow first, but didn't get the response so far. So, ask my question here.

When we do popBackStack and return to the previous fragment in the back stack, the bundle in the method onViewCreated(View view, Bundle bundle) is always null due to the fact that onSaveInstanceState(Bundle outState) wasn't called before. So, when we go back, bundle is null and a presenter (and view state) is created again. How in this case we can reuse presenter and view state (and not recreate it)?

You can see a dummy example below. There is a fragment with 2 buttons. One button opens a new Fragment and another button goes to the previous one. When you go back, presenter and view states are recreated. That's not I need, but I described above why it's happenning according to the code in the library. Is there a way to ensure that we reuse presenter and view state when we go back?


public class FirstFragment extends MvpViewStateFragment<FirstFragmentView, FirstFragmentPresenter, FirstFragmentViewState> {

public static final String TAG = "TAG";

private Button moveToAnotherFragmentButton;
private Button moveBackButton;

@Nullable
@Override
public View onCreateView(final LayoutInflater inflater, @Nullable final ViewGroup container, @Nullable final Bundle savedInstanceState) {
    final View rootFragmentView = inflater.inflate(R.layout.first_fragment, container, false);
    moveToAnotherFragmentButton = (Button) rootFragmentView.findViewById(R.id.first_fragment_go_to_another_fragment_button);
    moveBackButton = (Button) rootFragmentView.findViewById(R.id.first_fragment_back_button);
    return rootFragmentView;
}

@Override
public void onViewCreated(@NonNull final View view, @Nullable final Bundle savedInstanceState) {
    super.onViewCreated(view, savedInstanceState);

    moveToAnotherFragmentButton.setOnClickListener(ignored -> addToStack(FirstFragment.class));
    moveBackButton.setOnClickListener(ignored -> getFragmentManager().popBackStack());
}

@Override
@NonNull
public FirstFragmentPresenter createPresenter() {
    Log.e(TAG, "createPresenter");
    return new FirstFragmentPresenter();
}

@NonNull
@Override
public FirstFragmentViewState createViewState() {
    Log.e(TAG, "createViewState");
    return new FirstFragmentViewState();
}

@Override
public void onNewViewStateInstance() {
    Log.e(TAG, "onNewViewStateInstance");
}

private void addToStack(@NonNull final Class<? extends Fragment> fragmentClass) {
    final FragmentManager fragmentManager = getFragmentManager();
    if (fragmentManager.isDestroyed()) {
        throw new UnexpectedException("FragmentManager is destroyed");
    }

    final Fragment fragment = Fragment.instantiate(getContext(), fragmentClass.getName());

    final FragmentTransaction fragmentTransaction = fragmentManager.beginTransaction();
    fragmentTransaction.setTransition(FragmentTransaction.TRANSIT_FRAGMENT_OPEN);

    fragmentTransaction.replace(R.id.activity_main_content_container, fragment, null)
            .addToBackStack(null)
            .commit();
}
}```
sockeqwe commented 7 years ago

Thanks for reporting. Seems like a bug. I will take a look at it in first week of August.

Alex Urzhumtcev notifications@github.com schrieb am Mi., 19. Juli 2017, 10:26:

Hello, I discribed the problem on stackoverflow https://stackoverflow.com/questions/45165126/mosby-3-mvp-presenter-is-always-recreated-when-go-back-to-the-fragment-in-the first, but didn't get the response so far. So, answer my question here.

When we do popBackStack and return to the previous fragment in the back stack, the bundle in the method onViewCreated(View view, Bundle bundle) is always null due to the fact that onSaveInstanceState(Bundle outState) wasn't called before. So, when we go back, bundle is null and a presenter (and view state) is created again. How in this case we can reuse presenter and view state (and not recreate it)?

You can see a dummy example below. There is a fragment with 2 buttons. One button opens a new Fragment and another button goes to the previous one. When you go back, presenter and view states are recreated. That's not I need, but I described above why it's happenning according to the code in the library. Is there a way to ensure that we reuse presenter and view state when we go back?

`public class FirstFragment extends MvpViewStateFragment<FirstFragmentView, FirstFragmentPresenter, FirstFragmentViewState> {

public static final String TAG = "TAG";

private Button moveToAnotherFragmentButton; private Button moveBackButton;

@nullable https://github.com/nullable @override https://github.com/override public View onCreateView(final LayoutInflater inflater, @nullable https://github.com/nullable final ViewGroup container, @nullable https://github.com/nullable final Bundle savedInstanceState) { final View rootFragmentView = inflater.inflate(R.layout.first_fragment, container, false); moveToAnotherFragmentButton = (Button) rootFragmentView.findViewById(R.id.first_fragment_go_to_another_fragment_button); moveBackButton = (Button) rootFragmentView.findViewById(R.id.first_fragment_back_button); return rootFragmentView; }

@override https://github.com/override public void onViewCreated(@nonnull https://github.com/nonnull final View view, @nullable https://github.com/nullable final Bundle savedInstanceState) { super.onViewCreated(view, savedInstanceState);

moveToAnotherFragmentButton.setOnClickListener(ignored -> addToStack(FirstFragment.class)); moveBackButton.setOnClickListener(ignored -> getFragmentManager().popBackStack());

}

@override https://github.com/override @nonnull https://github.com/nonnull public FirstFragmentPresenter createPresenter() { Log.e(TAG, "createPresenter"); return new FirstFragmentPresenter(); }

@nonnull https://github.com/nonnull @override https://github.com/override public FirstFragmentViewState createViewState() { Log.e(TAG, "createViewState"); return new FirstFragmentViewState(); }

@override https://github.com/override public void onNewViewStateInstance() { Log.e(TAG, "onNewViewStateInstance"); }

private void addToStack(@nonnull https://github.com/nonnull final Class<? extends Fragment> fragmentClass) { final FragmentManager fragmentManager = getFragmentManager(); if (fragmentManager.isDestroyed()) { throw new UnexpectedException("FragmentManager is destroyed"); }

final Fragment fragment = Fragment.instantiate(getContext(), fragmentClass.getName());

final FragmentTransaction fragmentTransaction = fragmentManager.beginTransaction(); fragmentTransaction.setTransition(FragmentTransaction.TRANSIT_FRAGMENT_OPEN);

fragmentTransaction.replace(R.id.activity_main_content_container, fragment, null) .addToBackStack(null) .commit();

} }`

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sockeqwe/mosby/issues/265, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjnrqWkiiJ1hwzpaxkglNXcPdQ1lCPxks5sPb3MgaJpZM4OcZK- .

sockeqwe commented 7 years ago

I read your reported issue again and actually it is not a bug, it has never been implemented and we never claimed that MVP + Backstack work.

The philosophical reason behind this is: If there is no View, do we need the Presenter to keep alive? If Presenter will update view while not attached, then this "update" event will be lost? Therefore we thought it is easier to not support this feature out of the box but rather "recreate" everything so that fresh data is loaded as if the presenter / Fragment would be started for the first time.

However, Mosby Model-View-Intent (MVI) module supports fragment on the backstack because it is very clear that the unidirectional dataflow can still be alive even if no view is attached. If you use MVP and RxJava we encourage you to take a look at Mosby MVI.

So supporting MVP and backstack is not a matter of technically possible to implement or not. We know how to implement it. We just didn't do it on purpose so far.

Nevertheless, we will add back stack support for MVP in Mosby 3.1 (scheduled somewhen around August 20) along with a new MVP Presenter that keeps the update events in an internal "queue" while view not attached and dispatch them once the view get's reattached.

What do you think about this?

contrudar commented 7 years ago

@sockeqwe I understood the explanation and I'll take a look at MVI. Also what you said about Mosby 3.1. looks like what the library "Moxy" does. But still curious how to solve my problem. I use only one activity and when I go from one fragment to another and go back I don't wanna load data again and to make a user wait for data again. Just interesting how this standard case should be implemented with current version of Mosby?

Thomas-Vos commented 7 years ago

@sockeqwe Thanks for an explanation about the back stack. If Mosby 3.1 adds adds support for a "queue" it would be great! How would you recommend me implementing my code, until the new 3.1 approach, so it will be easy to migrate?

sockeqwe commented 7 years ago

Since this is mostly yet another Presenter implementation I try to find some time for it this week. Its not like moxy which needs an entirely framework and tools like annotation processing around it.

Thomas Vos notifications@github.com schrieb am Mo., 24. Juli 2017, 16:50:

@sockeqwe https://github.com/sockeqwe Thanks for an explanation about the back stack. If Mosby 3.1 adds adds support for a "queue" it would be great! How would you recommend me implementing my code, until the new 3.1 approach, so it will be easy to migrate?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/sockeqwe/mosby/issues/265#issuecomment-317447597, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjnrlvE91DYTA0IRmgyAtRX8CK9BKQUks5sRK9KgaJpZM4OcZK- .

Thomas-Vos commented 7 years ago

@sockeqwe Hi Hannes, have you had a chance to work on the new presenter implementation? Could you share your idea about on how you are going to implement it? If you have a snapshot ready, I'll be glad to help with testing. Thanks, Thomas

sockeqwe commented 7 years ago

Hi, sorry, not yet. Most likely I have time at end August to work on that...

On Do., 17. Aug. 2017, 18:52 Thomas Vos notifications@github.com wrote:

@sockeqwe https://github.com/sockeqwe Hi Hannes, have you had a chance to work on the new presenter implementation? Could you share your idea about on how you are going to implement it? If you have a snapshot ready, I'll be glad to help with testing. Thanks, Thomas

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/sockeqwe/mosby/issues/265#issuecomment-323131223, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjnrqs000n_etDBqxN9niUo1fr3MCEwks5sZG_HgaJpZM4OcZK- .

sockeqwe commented 6 years ago

See #274. Feel free to reopen this issue if 3.1.0 (snapshot) doesn't solve this porblem.