mosby icon indicating copy to clipboard operation
mosby copied to clipboard

Navigation in MVi

Open littleGnAl opened this issue 7 years ago • 11 comments

Consider such a demand:Click the button to post data to the server and then navigate to the other Activity when request success. I modified the ProductDetailsPresenter and ProductDetailsActivity to provide some test for me.

I add a testBtnIntent() method for the ProductDetailsView to handle a click intent.

public class ProductDetailsActivity extends MviActivity<ProductDetailsView, ProductDetailsPresenter>
    implements ProductDetailsView {

    @Override
    public Observable<Boolean> testBtnIntent() {
      return RxView.clicks(btnTest).share().map(it -> true);;
    }

    @Override public void render(ProductDetailsViewState state) {
    Timber.d("render " + state);

    if (state instanceof ProductDetailsViewState.LoadingState) {
      renderLoading();
    } else if (state instanceof ProductDetailsViewState.DataState) {
      renderData((ProductDetailsViewState.DataState) state);
    } else if (state instanceof ProductDetailsViewState.ErrorState) {
      renderError();
    } else if (state instanceof ProductDetailsViewState.TestViewState) {
      TestClickActivity.start(this);
    } else {
      throw new IllegalStateException("Unknown state " + state);
    }
  }
}

And then mergeWith the loadDetails method

 @Override protected void bindIntents() {
    ...
    Observable<ProductDetailsViewState> loadDetails =
        intent(ProductDetailsView::loadDetailsIntent)
            .doOnNext(productId -> Timber.d("intent: load details for product id = %s", productId))
            .flatMap(interactor::getDetails)
            .observeOn(AndroidSchedulers.mainThread());

    Observable<ProductDetailsViewState> clickTest =
        intent(ProductDetailsView::testBtnIntent)
        .map((aBoolean) ->  new ProductDetailsViewState.TestViewState());

    subscribeViewState(loadDetails.mergeWith(clickTest), ProductDetailsView::render);
  }

Because the viewStateBehaviorSubject will reemit the latest viewstate , so it will always reemit the clickTest when I go back the previous page and the TestClickActivity will open again and again, The expected should be to go back to the previous page without reemit the click event. Sorry for my bad English. hope I can explanation of this. And can you give me some suggestion?

littleGnAl avatar Jul 04 '17 17:07 littleGnAl

We have some similar discussion about SnackBar here #255

However, I think navigation can be seen as "side effect" since it is not affecting (nor changing) the state for your current View. For Example the state of ProductDetailsActivity is not changed by navigation per se, right?

Navigation is "just" a side effect of a state change in ProductDetailsViewState. Side effects can be modeled in RxJava with doOnNext() operator.

I think something like this should work: Assuming you have some class Navigator:

class Navigator {
    private final Activity activity;

    @Inject
    public Navigator(Activity activity){
        this.activity = activity;
    }

    public void navigateToTestActivity(){
          TestClickActivity.start(activity);
    }
}

Then you could inject the Navigator into ProductDetailsPresenter

class ProductDetailsPresenter extends MviBasePresenter<...> {

   private final Navigator navigator;
   
   @Inject
   public ProductDetailsPresenter(Navigator navigator){
         this.navigator = navigator;
    }


    @Override protected void bindIntents() {
    ...
    Observable<ProductDetailsViewState> loadDetails =
        intent(ProductDetailsView::loadDetailsIntent)
            .doOnNext(productId -> Timber.d("intent: load details for product id = %s", productId))
            .flatMap(interactor::getDetails)
            .observeOn(AndroidSchedulers.mainThread());

    Observable<ProductDetailsViewState> clickTest =
        intent(ProductDetailsView::testBtnIntent)
        .map((aBoolean) ->  new ProductDetailsViewState.TestViewState())
        .doOnNext( aBoolean -> navigator.navigateToTestActivity() ); // Navigation as side effect

    subscribeViewState(loadDetails.mergeWith(clickTest), ProductDetailsView::render);
  } 
}

Of course you have to keep in mind that Navigator might leak the Activity on config change (since presenter is kept during config change). I just wanted to illustrate a possible solution.

Does this makes sense?

sockeqwe avatar Jul 04 '17 21:07 sockeqwe

@sockeqwe Thanks. It's a good solution, and maybe I can just pass the View to the Presenter just like MVP does, but I think whether have a more elegant way to do this?(Maybe like your discussion in #255)

littleGnAl avatar Jul 05 '17 07:07 littleGnAl

Hi! I chose to include navigation through the MVI pattern to be able to log every user action within the app. I had the same problem above and couldn't just pass references to a Navigator or start activities from the Presenter because I keep the presentation layer in a separate project module. My solution was to clear the navigation flags with another MVI intent triggered by onPause. I know it's a bit dirty, I'm new to MVI, any feedback is welcomed! :D PS: Great library!

Presenter.kt
...
override fun bindIntents() {
...
val clearViewStateIntent = intent { view -> view.clearNavigationIntent()}
                .map { LandingViewState() }
                .subscribeOn(Schedulers.io())

        val allIntents = Observable.merge(
                createViewIntent,
                loginIntent,
                signUpIntent,
                clearViewStateIntent
        ).observeOn(AndroidSchedulers.mainThread())

        subscribeViewState(allIntents, LandingContract.View::render)
}
Activity.kt
...
override fun onPause() {
        pauseSubject.onNext(true)
        super.onPause()
    }
override fun render(viewState: LandingContract.State) {
        when (true) {
            viewState.navigateToLoginScreen() -> SignInActivity.start(this)
            viewState.navigateToManageScreen() -> ManageActivity.start(this)
            viewState.navigateToSignUpScreen() -> SignUpActivity.start(this)
        }
    }

    override fun clearNavigationIntent(): Observable<LandingContract.Intent.ClearNavigationIntent> {
        return pauseSubject.map { LandingContract.Intent.ClearNavigationIntent() }
    }

andreiverdes avatar Aug 29 '17 22:08 andreiverdes

@sockeqwe How do you suggest handling navigation in the case where the next screen needs data that is in the view state?

abyrnes avatar Feb 16 '18 23:02 abyrnes

I'm having the same problem. It seems the problem is caused by using view state to accomplish navigation as a side effect, as @sockeqwe mentioned. I am not a big fan of passing the Activity into the Presenter though, so I will try out the approach @andreiverdes outlined.

Would it be feasible to allow certain View States to be consumed instead of replacing the previous one?

skykelsey avatar Apr 11 '18 00:04 skykelsey

Would it be feasible to allow certain View States to be consumed instead of replacing the previous one?

@skykelsey For this I've been having a dummy state that gets emitted by navigation intents (I call it NoOp) and is just filtered out to avoid any unnecessary renderings:

val intent1 = intent(...)
val intent2 = intent(...)

val viewStateObservable = Observable.merge(intent1, intent1).filter { it !is NoOp } 
subscribeViewState(viewStateObservable, View::render)

This, however, doesn't solve the main problem at hand, for which I still haven't come up with an elegant solution, unfortunately. It doesn't seem like Mosby is well equipped to handle navigation in the presentation layer. So in the meantime I've implemented a base presenter that inherits from MviPresenter and hacks away at its implementation details to retrieve the previous view state, if it exists, and directly notify the view that a navigation event has occurred. This, combined with filtering out NoOp view states, has yet to cause me any issues.

abyrnes avatar Apr 11 '18 04:04 abyrnes

Jfyi: I have figured out a simple pattern of how to do navigation properly (not only MVI related, but works well with MVI).

I can share it next week as I'm going to present it on a Meetup next Monday.

Stay tuned.

Andy Byrnes [email protected] schrieb am Mi., 11. Apr. 2018, 06:22:

Would it be feasible to allow certain View States to be consumed instead of replacing the previous one?

@skykelsey https://github.com/skykelsey For this I've been having a dummy state that gets emitted by navigation intents (I call it NoOp) and is just filtered out to avoid any unnecessary renderings:

val intent1 = intent(...)val intent2 = intent(...) val viewStateObservable = Observable.merge(intent1, intent1).filter { it !is NoOp } subscribeViewState(viewStateObservable, View::render)

This, however, doesn't solve the main problem at hand, for which I still haven't come up with an elegant solution, unfortunately. It doesn't seem like Mosby is well equipped to handle navigation in the presentation layer. So in the meantime I've implemented a base presenter that inherits from MviPresenter and hacks away at its implementation details to retrieve the previous view state, if it exists, and directly notify the view that a navigation event has occurred. This, combined with filtering out NoOp view states, has yet to cause me any issues.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sockeqwe/mosby/issues/261#issuecomment-380322785, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjnrgjDcIoSd1TIj7nYK38vlE5ctmrdks5tnYUegaJpZM4ONlOH .

sockeqwe avatar Apr 11 '18 06:04 sockeqwe

I have figured out a simple pattern of how to do navigation properly

Any news on this? I couldn't find anything about it on your blog.

tschuchortdev avatar Apr 27 '18 10:04 tschuchortdev

My bad, I haven't had time to write a blog post or clean up my example repository ... Maybe next week ... sorry ...

sockeqwe avatar Apr 27 '18 13:04 sockeqwe

Thanks @sockeqwe I'm interested as well when you find the time.

skykelsey avatar May 01 '18 23:05 skykelsey

Here you go: http://hannesdorfmann.com/android/mosby3-mvi-8

Hope someone find it helpful.

sockeqwe avatar May 06 '18 10:05 sockeqwe