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

View actions (snackbar, activity navigation, ...) in ViewModel #63

Open
fabioCollini opened this issue Jun 20, 2017 · 82 comments
Open

View actions (snackbar, activity navigation, ...) in ViewModel #63

fabioCollini opened this issue Jun 20, 2017 · 82 comments

Comments

@fabioCollini
Copy link

@fabioCollini fabioCollini commented Jun 20, 2017

Sometimes the ViewModel needs to invoke an action using an Activity (for example to show a snack bar or to navigate to a new activity). Using MVP it's easy to implement it because the presenter has a reference to the Activity. The ViewModel doesn't contain a reference to the Activity, what's the best way to implement this feature?
In the GithubBrowserSample you created a method getErrorMessageIfNotHandled to solve this problem, are there other solutions?
In a demo project I am working on I have created a UiActionsLiveData class that allows the ViewModel to invoke an action on the view. The connection between the ViewModel and the view is managed using a LiveData so the ViewModel doesn't contain a reference to the View even if it can invoke actions on it. The usage is simple, the ViewModel can add an action to the UiActionsLiveData:

uiActions.execute { navigationController.showError(it, t.message) }

Thanks to the LiveData implementation the action will be executed on the Activity when it's resumed.
I like this solution but it's a bit complicated, an official solution would be great.

@JoseAlcerreca
Copy link
Collaborator

@JoseAlcerreca JoseAlcerreca commented Jun 21, 2017

@burntcookie90
Copy link

@burntcookie90 burntcookie90 commented Jun 21, 2017

I think using SingleLiveEvent as a basis for other one off (notification style) events works great. I've used this pattern in a similar (internal) framework with an older project. Deploying it as part of the library makes sense, allows us all to not have to reinvent the wheel 😄 .

@fabioCollini
Copy link
Author

@fabioCollini fabioCollini commented Jun 21, 2017

In my implementation I am saving a list of events but in the SingleLiveEvent class you are managing just a single value. For example if the ViewModel produces more than a value when the Activity is paused the observer will get only the last value produced. Is this an expected behaviour? I would expect to see all the value on the observer (then the View can decide to ignore some values).

@JoseAlcerreca
Copy link
Collaborator

@JoseAlcerreca JoseAlcerreca commented Jun 22, 2017

Interesting, for navigation it makes no sense to hold a list and it's officially not recommended for Snackbar but you can join multiple messages in one. We'll take it into account thanks!

@sockeqwe
Copy link

@sockeqwe sockeqwe commented Jun 22, 2017

A big NO from my side for SingleLiveEvent!
Unidirectional Dataflow and immutability ftw! (also a little state reducer over here and there doesn't hurt)

SearchViewModel which offers a LiveData<LoadMoreState> getLoadMoreStatus() method for the view to subscribe / listen to.
So once the view has displayed the error message (i.e. with a snackbar) the View calls:

viewModel.setLoadMoreErrorShown();

This will then force the viewModel to emit a new (immutable) LoadMoreState.

class SearchViewModel extends ViewModel {
   private MutableLiveDate<LoadMoreState> loadMoreState;
   ...
   
   // Subscribed by the View
   public LiveData<LoadModeState> getLoadMoreStatus() {
       return loadMoreState;
    }

   public void setLoadMoreErrorShow(){
         loadMoreState.setValue(new LoadMoreState(false, null)); // null means no error message to display , false means don't display loading more progress bar
    }
}

behaves like SingleLiveEvent (i.e. after screen orientation change, snackbar wont show again, because LoadMoreState.errorMessage == null) but is immutable!

@fabioCollini
Copy link
Author

@fabioCollini fabioCollini commented Jun 23, 2017

I agree that unidirectional dataflow and immutability are great but in this example I have a doubt about your solution: you update a LiveData while you are updating the view because of a modification of the same LiveData. At the beginning of dispatchingValue method there is a check to avoid observer nested invocations, will it work correctly? Are there other cases when this double update of the view can be a problem? For example can you manage more than one field in this way? Or what about multiple updates of the same field when the activity is paused, will the state contain a list instead of a single value?
However the two solutions are quite similar, in my opinion a SingleLiveEvent is not a violation of immutability and unidirectional dataflow. In your example the view invokes a method on the ViewModel to update the LiveData, using a SingleLiveEvent the LiveData is updated in a similar way automatically.

@sockeqwe
Copy link

@sockeqwe sockeqwe commented Jun 23, 2017

At the beginning of dispatchingValue method there is a check to avoid observer nested invocations

Well, I wasn't aware of this implementation detail. I was just saying that to me invoking something like setLoadMoreErrorShown() seems more correct, transparent and easier to test compared to SingleLiveData. I'm wondering if one could implement it's own LiveData class without this check (or at least allow it if the value to dispatch has changed)

For example can you manage more than one field in this way?

I cant see why not.

Or what about multiple updates of the same field when the activity is paused, will the state contain a list instead of a single value?

No, just the latest value, not a list. In contrast to MVP, in MVVM ViewModels typically have this fields to observe a certain value. I cant find a use case where I need to.keep track of previous undispatched values. Do you?

in my opinion a SingleLiveEvent is not a violation of immutability and unidirectional dataflow.

I disagree, it clearly is a side effect (not a pure function) and obviously not immutable and not a unidirectional dataflow (state is not changed and maintained from one component).
However, if SingleLiveData works for you and then sure go ahead an use it. At the end of the day no user of your app cares about immutability and unidirectional data flow. I was just saying that from my point of View SingleLiveData violates some core principles (like immutability) and that this problem of a snackbar could be solved more idiomatically i.e. viewModel.setLoadMoreErrorShown, but maybe SingleLiveData is a pragmatic solution... practical vs. idealistic ...

@ZakTaccardi
Copy link

@ZakTaccardi ZakTaccardi commented Jun 25, 2017

I didn't read this in too much detail, so sorry if I'm off base.

But the snack bar is a pretty simple solution. Its timed dismissal is just another form of input from your UI

@JoseAlcerreca
Copy link
Collaborator

@JoseAlcerreca JoseAlcerreca commented Jun 26, 2017

@sockeqwe since your blog post has attracted so much attention, can you confirm the following? It seems that your solution:

  1. requires more logic in the activity (onDismissed listener and a null check)
  2. setLoadMoreErrorShow should be called resetLoadMoreState, or to use the original,setLoadMoreShown, not sure why you added "Error" there.
  3. it behaves differently than SingleLiveData, since you're resetting after dismissal while we have a "fire and forget" approach. I understand you want to store state but there could be a case where the app is opened days after the last use and the user sees a misleading error message from 2 days ago. It might not be a good fit for a Snackbar. It is a better fit for a navigation event, for example.

As already rectified on your blog post, SingleLiveEvent doesn't use null internally. So statements like

state is not changed and maintained from one component

are not true in this case (if I understand you correctly). You thought the activity was setting the event's data to null. Only the ViewModel, IIRC, sets the value. We considered using composition instead of inheritance to protect who sets the value but we would have to proxy things like observeForever().

@sockeqwe
Copy link

@sockeqwe sockeqwe commented Jun 27, 2017

@JoseAlcerreca thanks for your feedback. It's hard to describe this in detail with text only. I have forked the Github client sample and will implement the repo search with what I have in mind. I hope that a concrete code example makes my point more understandable and a better basis for the future discussion, don't you think so? Will work on it on Thursday...

@svasilinets
Copy link
Collaborator

@svasilinets svasilinets commented Jun 27, 2017

At the beginning of dispatchingValue method there is a check to avoid observer nested invocations, will it work correctly?

Yes, it will. This check prevents reentrance in onChanged method of livedata's observers. If an observer in itsonChanged method sets a new value to LiveData it listens to, this new value will be dispatched right after execution of that onChanged method.

@sockeqwe
Copy link

@sockeqwe sockeqwe commented Jun 30, 2017

I have worked a little bit on refactoring the SearchViewModel of GithubBrowserSample and would like to hear your feedback.
My forked repo can be found here: https://github.com/sockeqwe/android-architecture-components

So I have introduced a class SearchState. This class basically represents state and can be "rendered" directly by the view. So instead of LiveData<Resource<List<Repo>>> results and LiveData<LoadMoreState> my refactored SearchViewModel only provides a single LiveData<SearchState> state for the view (SearchFragment) to observe since SearchState contains all state information in one single object.

Please note that you could split this class in something like kotlin's sealed class or other solutions. The code itself is far from perfect in my example. It is just a proof of concept.

Whenever the user triggers an action like typing search, scroll to the end to load next page, this action sinks down to the business logic. The business logic knows the current state and applies this action on it to compute the new state (which is a new immutable state object). For "SingleLiveEvents" like displaying a error message in a SnackBar I would like to suggest to apply the same concept. If SnackBar has shown the message the action "clear load more error message" sinks down to the business logic as it would be any other action like a click on a button.

you update a LiveData while you are updating the view because of a modification of the same LiveData .At the beginning of dispatchingValue method there is a check to avoid observer nested invocations, will it work correctly?

@fabioCollini yes, it does. It works as you can see in my linked example repository.

Regarding @JoseAlcerreca questions:

  1. requires more logic in the activity (onDismissed listener and a null check)

Not sure if I would call a listener "more logic". Then each click listener is also more logic? I guess it depends on how you define logic in context of view (like activity etc.). (btw. the number of lines of code is decreased by about 20 lines, not sure how meaningful this information is though).

  1. setLoadMoreErrorShow should be called resetLoadMoreState, or to use the original,setLoadMoreShown, not sure why you added "Error" there.

Yes, naming is not great.

  1. it behaves differently than SingleLiveData, since you're resetting after dismissal while we have a "fire and forget" approach. I understand you want to store state but there could be a case where the app is opened days after the last use and the user sees a misleading error message from 2 days ago. It might not be a good fit for a Snackbar. It is a better fit for a navigation event, for example.

Yes, this are two different approaches: "fire and forget" vs. "single state driven by the business logic".
Sure, this is more a "religious" question and as you have already noticed I see more value in the later approach (which obviously doesn't mean that this approach is the better one) when it comes to predictable state management, debugging and testing. For example, debugging: We can easily log every step we have made from one state to the other state (and which user interaction was involved to trigger the state transition).

One problem with "fire and forget" is that it may leak View implementation details to the underlying layer (like ViewModel, "business logic", or whatever layer you have in your app).
For example: Right now the app is showing a SnackBar if an error has occurred. What If one day we would like to display this error with a traditional TextView? Then we not only have to change code in the view layer, but also in the underlying layers where we have produced the SingleLiveEvent because then it shouldn't be a SingleLiveEvent anymore but for TextView we would use simply MutableLiveData, right?

@deviant-studio
Copy link

@deviant-studio deviant-studio commented Jul 4, 2017

i've implemented similar approach for ui commands
https://github.com/deviant-studio/energy-meter-scanner/blob/master/app/src/main/java/ds/meterscanner/mvvm/Extensions.kt#L30

and Snackbar example here
https://github.com/deviant-studio/energy-meter-scanner/blob/master/app/src/main/java/ds/meterscanner/mvvm/Command.kt#L8

it would be great to have such functionality out of the box in the upcoming lib release

@JoseAlcerreca
Copy link
Collaborator

@JoseAlcerreca JoseAlcerreca commented Jul 5, 2017

@deviant-studio your Command has two problems:

  • You can't use null as a value
  • If the value is set while an observer is not active, it will be lost. For example, if done from activity.onCreate.
@deviant-studio
Copy link

@deviant-studio deviant-studio commented Jul 5, 2017

@JoseAlcerreca true. that is why i'm going to use your SingleLiveEvent implementation ;)

@asantibanez
Copy link

@asantibanez asantibanez commented Jul 19, 2017

Hi! I just started/learning Android Architecture Components and see that how to handle View Actions is a missing piece on the proposed components. By no means I am criticizing the work done by Google. I love what they have brought to the table.

With that being said, I kind of like @deviant-studio approach to the problem. Making all Commands one shot only. Imagine if this approach can be used together with a Event Queue System that the ViewModel would manage. The Events would be like constant ACTIONS that the Activity/Fragment could subscribe to and would be dispatched in order.

A seudo implementation of this would be:
In ViewModel:

dispatch(SHOW_PROGRESS)

//do long work
loadPosts()

dispatch(HIDE_PROGRESS)
dispatch(SHOW_RESULTS)

In Activity:

queueSubscribe {
    switch(event) {
        SHOW_PROGRESS ->  progressBar.setVisibility(View.VISIBLE)
        HIDE_PROGRESS -> progressBar.setVisibility(View.GONE)
        SHOW_RESULTS -> {
            adapter.setPosts(viewModel.posts)
        }
    }
}

Once the event is consumed, it should be dropped from the queue and all should be processed sequentially. In this way, the Activity/Fragment would loose track of all ViewActions that need to be handled by the view.

This is my 2 cents on this. Got the inspiration after working with Vuex (https://vuex.vuejs.org/en/intro.html) on another project (somewhat React-ish) to handle all data flow on one direction.

If this is not good or valid at all, no problem! Just sharing some ideas.

@gaara87
Copy link

@gaara87 gaara87 commented Jul 21, 2017

I love the idea of SingleLiveEvent but i'm thinking, theres 10~ish callbacks, is it better to have n unique SingleLiveEvents or would it be better to have a singleliveevent passing in different event types?

@etiennelenhart
Copy link

@etiennelenhart etiennelenhart commented Jul 27, 2017

@JoseAlcerreca We tried your SingleLiveEvent implementation and are having problems when using it together with observeForever. removerObserver isn't working correctly since the original Observer is wrapped inside the SingleLiveEvent. Maybe it needs some kind of internal map of wrapped observers?

I created a naive implementation (in Kotlin) which works for us: https://gist.github.com/etiennewelsch/142ab9e080b0144927fd2486cd34feb9

@jshvarts
Copy link
Contributor

@jshvarts jshvarts commented Aug 3, 2017

@JoseAlcerreca I am successfully using your SingleLiveEvent and vote for including it in the library when released.

@JoseAlcerreca
Copy link
Collaborator

@JoseAlcerreca JoseAlcerreca commented Aug 3, 2017

@etiennewelsch observeForever is not meant to be used in production (I think!). Even in tests I ended up using observe. If you share more details on how you use it I might be able to help.

@JoseAlcerreca
Copy link
Collaborator

@JoseAlcerreca JoseAlcerreca commented Aug 3, 2017

@gaara87 my suggestion is that you try both approaches, as it doesn't take much time, and decide. In principle 10 callbacks sounds easier to manage than 1.

Edit: But I would like to know your use case, as 10 SingleLiveEvents sounds like a lot!

@etiennelenhart
Copy link

@etiennelenhart etiennelenhart commented Aug 3, 2017

@JoseAlcerreca I was not aware that we shouldn't use observerForever. Good to know, if that's really the case. :)
We're currently doing manual Dependency Injection via a custom Application class and are using the SingleLiveEvents to notify AndroidViewModels of events in our business logic. So there is no LifecycleOwner to pass to observe.

@StefMa
Copy link

@StefMa StefMa commented Aug 4, 2017

This thing is indeed a interesting discussion.
I need a "OneTimeObserver" as well in my project.
I ended up in something like that:

    myLiveData.observeForever(object : Observer<String> {
            override fun onChanged(t: String?) {
                mainModelLiveData.value = MainModel(true, t)
                myLiveData.removeObserver(this)
            }
        })

Now I thinking about to use the SingleEvent class. The problem: I use this in a ViewModel. Which means I don't have a LiveCycle here. Which means. The SingleEvent class can't be used.

Why I listen to LiveData in a ViewModel you think?
I have a similar architecture like described here:
I have a UserRepo which returns a LiveData. But beside of "tunneling" it directly in to the Activtiy/Fragment (within a ViewModel code like that):

fun getUser(): User {
   repo.getUser()
}

I have a similar approve like @sockeqwe.
I have one LiveData object which is accessable from the Activtiy/Fragment which sends one Model outside. Like data class Model(var loading: Boolean, var toolbarTitle: String).
But my Repo can be load multiple user. Which means I have to listen in my ViewModel to the changes of the LiveData from the Repo. If everything loaded (with the "OneTimeObserver") I can send change the Model object and it will be send to the Activtiy/Fragment...

Yeah a lot of text i know 🙃

But: Are there other solutions?
Even if you say observeForever shouldn't be used... Why not? 🤔

@JoseAlcerreca
Copy link
Collaborator

@JoseAlcerreca JoseAlcerreca commented Aug 4, 2017

@StefMa of course you can use the LiveData in the ViewModel, just observe it from the activity. You can create a getLiveData method in the VM or an observe(owner, observer). As with any LiveData, the VM exposes it.

@StefMa
Copy link

@StefMa StefMa commented Aug 4, 2017

That's is the point. I don't want to observe it in the activity. As written above. I have multiple LiveData from different "services" in on ViewModel but want only send one "global Modal" to the Activity...

Sent from my Google Nexus 5X using FastHub

@JoseAlcerreca
Copy link
Collaborator

@JoseAlcerreca JoseAlcerreca commented Aug 4, 2017

Gotcha. If you're talking with a data source that doesn't need to be scoped to a view, you might want to use observeForever. However, LiveData was designed for the presentation layer, not to replace RX.

@etiennelenhart
Copy link

@etiennelenhart etiennelenhart commented Aug 4, 2017

@JoseAlcerreca @StefMa We came to the same conclusion today and just wrote our own Observable implementation which works equally well without the lifecycle overhead. SingleLiveEvent is still really useful for ViewModel to Activity events though.

@etiennelenhart
Copy link

@etiennelenhart etiennelenhart commented Aug 10, 2017

@JoseAlcerreca We came across a possible bug in the SingleLiveEvent implementation. When multiple observers are registered (e.g. two fragments), only the first is notified since pending is reset immediately. Any thoughts?

@JoseAlcerreca
Copy link
Collaborator

@JoseAlcerreca JoseAlcerreca commented Aug 10, 2017

Yes, that's working as intended (there's even a warning when you register the second). It's because we can't know how many observers are active (we can only know there are >0) as the LiveData doesn't expose the information.

We created a version that counted the number of active observers but it was too complex for a sample. We'll try to fix it if it's included in the library.

The workaround is to create a SingleLiveEvent per observer.

@etiennelenhart
Copy link

@etiennelenhart etiennelenhart commented Aug 11, 2017

@JoseAlcerreca Ah, sorry I totally forgot that you put in that warning. It seems to be quite difficult to get ViewModel to Activity communication right. I'm pretty confident, that you'll come up with something though. The Components already simplified our projects a lot.

@nicolasjafelle
Copy link

@nicolasjafelle nicolasjafelle commented Jul 21, 2018

After all... is there any official/best solution to consume only one event? Is the Event class by @JoseAlcerreca plus the EventObserver by @JoseAlcerreca the best solution? For example:

In ViewModel:
var eventLiveData: MutableLiveData<Event<ErrorResponse>> = MutableLiveData()

In Activity or Fragment:

viewModel.eventLiveData.observe(this, EventObserver {
     it?.let {
          shortToast(it.message)
     }
})
@guelo
Copy link

@guelo guelo commented Jul 22, 2018

That seems to be the recommended solution but it has one big problem, if the ViewModel sends multiple events while the View is rotating then only the last event will be handled and the other events will be dropped. If instead you need the View to never miss an event and to handle them serially then I would recommend using rx's ReplaySubject instead of MutableLiveData along with Jose's Event wrapper.

In my case I had several types of View commands so I added a sealed class wrapper to end up with two wrappers.

class ViewModel {
  val viewCommandsPublisher = ReplaySubject.create<SingleReadEvent<ViewCommand>>()

  sealed class ViewCommand {
    data class Navigate1(val some: Data)
    data class FetchSomethingFromFramework(val some: OtherData)
    data class Snackbar(val message: String)
  }

  fun doStuff() {
    viewCommandsPublisher.onNext(SingleReadEvent(ViewCommand.Navigate1(data)))
  }
}

class View {
  fun observe() {
    viewModel.viewCommandsPublisher
      .subscribe { 
        val event = it.getContentIfNotHandled()
        if (event != null) {
          when (event) {
            is Navigate1 -> navigate(event)
            is FetchSomethingFromFramework -> fetch(event)
            is Snackbar -> snackbar(event)
         }}}
}
@Zhuinden
Copy link

@Zhuinden Zhuinden commented Jul 22, 2018

Okay, now just for the sake of completion, I'm adding that this is how I handled this scenario 3 years ago and it worked just fine without all these hacks:

public enum SingletonBus {
    INSTANCE;

    private static String TAG = SingletonBus.class.getSimpleName();

    private final Vector<Object> eventQueueBuffer = new Vector<>();

    private boolean paused;

    public <T> void post(final T event) {
            if (paused) {
                eventQueueBuffer.add(event);
            } else {
                EventBus.getDefault().post(event);
            }
    }

    public <T> void register(T subscriber) {
        EventBus.getDefault().register(subscriber);
    }

    public <T> void unregister(T subscriber) {
        EventBus.getDefault().unregister(subscriber);
    }

    public boolean isPaused() {
        return paused;
    }

    public void setPaused(boolean paused) {
        this.paused = paused;
        if (!paused) {
            Iterator<Object> eventIterator = eventQueueBuffer.iterator();
            while (eventIterator.hasNext()) {
                Object event = eventIterator.next();
                post(event);
                eventIterator.remove();
            }
        }
    }
}

public class BaseActivity extends AppCompatActivity {
     @Override
     public void onPostResume() {
          super.onPostResume();
          SingletonBus.INSTANCE.setPaused(false);
     }

     @Override
     public void onPause() {
          SingletonBus.INSTANCE.setPaused(true);
          super.onPause();
     }
}

Technically if ViewModel has its own pauseable bus, then problem is solved. 🙄

@nicolasjafelle
Copy link

@nicolasjafelle nicolasjafelle commented Jul 23, 2018

@Zhuinden Isn't that an implementation of an event bus? like Otto from Square?

@Zhuinden
Copy link

@Zhuinden Zhuinden commented Jul 23, 2018

It uses greenrobot/EventBus at EventBus.getDefault().

@charlesng
Copy link

@charlesng charlesng commented Aug 7, 2018

What If we have the static EventProvider and let it do something like EventBus post event. The structure is like the SingleLiveEvent but also use the EventBus Queue Structure.

`
public class EventProvider {

public static class EmitOnceEvent<T> extends MutableLiveData<T> {
    private ArrayList<Observer<T>> observerQueue = new ArrayList<>();
    private final AtomicBoolean mPending = new AtomicBoolean(false);
    @MainThread
    public void observe(LifecycleOwner owner, final Observer<T> observer) {
        observerQueue.add(observer);
        owner.getLifecycle().addObserver(new LifecycleObserver() {
            @OnLifecycleEvent(Lifecycle.Event.ON_DESTROY)
            public void onDestroy() {
                observerQueue.remove(observer);
            }
        });
        super.observe(owner, new Observer<T>() {
            @Override
            public void onChanged(@Nullable T t) {
                if (mPending.compareAndSet(true, false)) {
                    for (Observer<T> observer1 : observerQueue) {
                        observer1.onChanged(t);
                    }
                }
            }
        });
    }


    @MainThread
    public void setValue(@Nullable T t) {
        mPending.set(true);
        super.setValue(t);
    }

    /**
     * Used for cases where T is Void, to make calls cleaner.
     */
    @MainThread
    public void call() {
        setValue(null);
    }

}

private static EventProvider eventProvider;
private EmitOnceEvent<MyEvent> myEvent = new EmitOnceEvent<>();

private EventProvider() {

}

public static EventProvider getInstance() {
    if (eventProvider == null) {
        eventProvider = new EventProvider();
    }
    return eventProvider;
}

}

`

Sonphil added a commit to ApplETS/Notre-Dame-Android that referenced this issue Aug 15, 2018
… the About screen has been opened at least once

MoreFragment reacts to the his viewmodel LiveData and navigated to the AboutActivity. However, sometime (i.e. on configuration change) the fragment would subscribes to the LiveData again, get the value and triggers the navigation again. The event should only consume once! Here more information about this issue :
android/architecture-components-samples#63
https://medium.com/google-developers/livedata-with-snackbar-navigation-and-other-events-the-singleliveevent-case-ac2622673150
@BugsBunnyBR
Copy link

@BugsBunnyBR BugsBunnyBR commented Aug 26, 2018

Have you folks seem this ?
https://issuetracker.google.com/issues/94056118

It seems that the support to this kind of thing should be in the framework.

@rvdsoft
Copy link

@rvdsoft rvdsoft commented Oct 10, 2018

I have created an improved version of @JoseAlcerreca's SingleLiveEvent that supports multiple observers. No need for a custom observer class or value wrapper.

public class SingleLiveEvent<T> extends MutableLiveData<T> {

    private LiveData<T> liveDataToObserve;
    private final AtomicBoolean mPending = new AtomicBoolean(false);

    public SingleLiveEvent() {
        final MediatorLiveData<T> outputLiveData = new MediatorLiveData<>();
        outputLiveData.addSource(this, currentValue -> {
            outputLiveData.setValue(currentValue);
            mPending.set(false);
        });
        liveDataToObserve = outputLiveData;
    }

    @MainThread
    public void observe(@NonNull LifecycleOwner owner, @NonNull Observer<T> observer) {
        liveDataToObserve.observe(owner, t -> {
            if(mPending.get()) {
                observer.onChanged(t);
            }
        });
    }

    @MainThread
    public void setValue(T value) {
        mPending.set(true);
        super.setValue(value);
    }
}

@vrendina
Copy link

@vrendina vrendina commented Oct 17, 2018

@guelo @Zhuinden I attempted to address this problem by creating a variant of the UnicastSubject that lets you subscribe with another observer, taking the place of the first observer. So in your ViewModel you have an instance of the QueueSubject and then you push messages into it. When the view subscribes it will drain the queue of messages to the view.

I'd like to modify it so you can support multiple observers, which might be useful if you have let's say an activity and fragment that share the same view model and both want to observe the message queue.

https://github.com/vrendina/RxQueue

@alexakasanjeev
Copy link

@alexakasanjeev alexakasanjeev commented Oct 20, 2018

When I tried using SingleLiveEvent in my code, it is giving me error
observe(LifecycleOwner, Observer<T>)' in 'com.example.com.SingleLiveEvent' clashes with 'observe(LifecycleOwner, Observer<? super T>)' in 'androidx.lifecycle.LiveData'; both methods have same erasure, yet neither overrides the other

@rvdsoft
Copy link

@rvdsoft rvdsoft commented Oct 20, 2018

I forgot to add the override annotation. Also in the previous version, only one observer was invoked in the onStart lifecycle event if new data was set while lifecycle owner was in background. Fixed that by doing postValue instead of setValue.

public class SingleLiveEvent<T> extends MutableLiveData<T> {
    private MediatorLiveData<T> liveDataToObserve;
    private final MutableLiveData<Boolean> mPending = new MutableLiveData<>();

    public SingleLiveEvent() {
        liveDataToObserve = new MediatorLiveData<>();
        liveDataToObserve.addSource(this, currentValue -> {
            liveDataToObserve.postValue(currentValue);
            mPending.postValue(null);
        });
    }

    private boolean isPending() {
        return mPending.getValue()!=null;
    }

    @MainThread
    @Override
    public void observe(@NonNull LifecycleOwner owner, @NonNull Observer<T> observer) {
        liveDataToObserve.observe(owner, t -> {
            if(isPending()) {
                observer.onChanged(t);
            }
        });
    }

    @MainThread
    @Override
    public void setValue(T value) {
        mPending.setValue(true);
        super.setValue(value);
    }

    @MainThread
    public void call(){
        setValue(null);
    }
}
@alexakasanjeev
Copy link

@alexakasanjeev alexakasanjeev commented Oct 21, 2018

That still doesn't work

error

@rvdsoft
Copy link

@rvdsoft rvdsoft commented Oct 21, 2018

@alexakasanjeev you seem to be using a different version of the library than me. simply change Observer<T> to Observer<? super T> in the SingleLiveEvent class.

@avegrv
Copy link

@avegrv avegrv commented Feb 19, 2019

I have published example with EventsQueue, see here
https://github.com/avegrv/android-architecture-components-example

In short version:

// A set of typical events
interface Event
class ShowToast(val textRes: Int) : Event

class EventsQueue : MutableLiveData<Queue<Event>>() {

    @MainThread
    fun offer(event: Event) {
        val queue = value ?: LinkedList()
        queue.add(event)
        value = queue
    }
}

// We divide the state into the screen state and events. Usually a fragment has two subscriptions.
observe(viewModel.user, this::onUserChanged)
observe(viewModel.event, this::onEventReceived)

inline fun <reified T : Event> Fragment.observe(
        eventsQueue: EventsQueue,
        noinline block: (T) -> Unit
) {
    eventsQueue.observe(
            this.viewLifecycleOwner,
            Observer<Queue<Event>> { queue: Queue<Event>? ->
                while (queue != null && queue.isNotEmpty()) {
                    block.invoke(queue.poll() as T)
                }
            }
    )
}

// An example of custom event handling. Just shows snack bars.
private fun onCommandReceived(event: Event) {
    when (event) {
        is ShowToast -> {
            showMessage(getString(command.textRes))
        }
    }
}
@Sottti
Copy link

@Sottti Sottti commented Jul 14, 2019

The ViewModel doesn't contain a reference to the Activity, what's the best way to implement this feature?

The MVVM pattern in Android should solve and talk at a higher level (abstracted from concrete implementations) how to deal with this very common situation. It takes 30 seconds to explain the solution in an MVP approach but looks like in MVVM is even difficult to think about all the different corner cases. It sounds very convoluted in comparison with MVP.

Should someone starting with Android development writing a first app that shows a Snackbar be dealing with LiveData/RxJava and creating wrappers or extensions to be able to achieve it?

Can someone please read the conversation top to bottom keeping in mind that the aim is to do something like showing a Snackbar and saying it makes perfect sense with a straight face?

You'd think a few years on the road with MVVM the solutions are solid and settle, but just a search on Google shows that everyone is fighting with it in a different way..

@Zhuinden
Copy link

@Zhuinden Zhuinden commented Jul 14, 2019

MVP is just incorrect implementation of MVVM. In MVP, the View exposes its behavior directly to the outside world, making the Presenter coupled to it.

The ViewModel should be a separate component, responsible for its own things. Therefore, to show a snackbar, what you want to know is not that you want to show a snackbar (why does a ViewModel even know about a Snackbar???), it should emit an event (that will be displayed in View as a in such a way that it ensures that this is handled by someone, but only once (*though then the question is whether it is the ViewModel's responsibility to know that the View has already handled this particular event. Hmm. Regardless...)

The solution imo is something like this:

// ViewModel
    private val mutableHostDisconnectedEvent: EventEmitter<Unit> = EventEmitter()
    val hostDisconnectedEvent: EventSource<Unit> = mutableHostDisconnectedEvent

    ...
                if (previousConnected && !currentConnected) {
                    mutableHostDisconnectedEvent.emit(Unit)
                }
    ... 

and in the Fragment:

// Fragment
        compositeNotificationToken += viewModel.hostDisconnectedEvent
            .startListening { _ ->
                showLongToast(R.string.alert_host_disconnected)
            }

except as it ought to use LiveData, the subscription management should go with observe(this,. Except in EventEmitter, I didn't start relying on Rx/LiveData, so eh.

It'd be possible if I wanted to.

@pablichjenkov
Copy link

@pablichjenkov pablichjenkov commented Jul 14, 2019

In reality, the ViewModel might not have reference to the View but someone must have to.
If you want to receive a callback notification some living scope emitting the event has to have reference to you as a listener, I don't know of a different way.
In practicality, one end up having the state of the View in the ViewModel rather than in each repository.
In many cases I found myself directly holding a LiveData instance in the ViewModel itself, then other repository streams update this livedata instance that dispatch the event on the view.
So the ViewModel holds a reference to the View through Livedata directly or indirectly.

In my opinion, sorry Google, don't use LiveData. Looking at it's internal implementation, as of (July 2019), it is highly risky to miss concecutives events. The class comments stay that clear. If you don't care about that use it but be very cautious on you use case.
I think a better approach to the event messaging programming and navigation logic is better to use the Actor model. I leave below a link to a sample app using the Actor style of programming, you are welcome to participate to enrich the concept:

https://github.com/pablichjenkov/Android-Actor

@Sottti
Copy link

@Sottti Sottti commented Jul 14, 2019

@Zhuinden Arguably your definition of MVP. I could say the View has to meet the behaviour requirements specified by the Presenter on a particular interface... so the presenter is just couple to a behaviour defined and created as it needs... As well, given that the View is the only one that is able to capture screen events, isn't the View exposing behaviour to the VM in MVVM anyway?

Any opinions on this approach?

Does Flow have any cool feature that could be useful in these scenarios?

@Zhuinden
Copy link

@Zhuinden Zhuinden commented Jul 15, 2019

, it is highly risky to miss concecutives events.

LiveData is like BehaviorRelay (BehaviorSubject) in Rx.

If you care about consecutive events, then that's not what you should be using in the first place.

As well, given that the View is the only one that is able to capture screen events, isn't the View exposing behaviour to the VM in MVVM anyway?

If the View describes its own events via an interface so it exposes things like onBlahClicked(), then you don't need to know the VM by exact type.

Although that is something that MVP should also be doing, the P or VM difference lies in whether P can directly call "configuration"-like methods on the View, or if it's a separate entity (VM) that cares nothing about the view other than emitting its own events (exposing observables of data / commands)

Any opinions on this approach?

I guess it could work, although it seems tricky that you need to explicitly call both setEventReceiver on the concrete type, AND still use observe like a regular LiveData. So if you just exposed it as regular LiveData, then it would not work.

@mustafaozhan
Copy link

@mustafaozhan mustafaozhan commented Apr 23, 2020

Here is solution for mutability, same as in LiveData/MutableLiveData,

open class SingleLiveData<T> : LiveData<T>() {
    protected val pending = AtomicBoolean(false)

    @Throws
    @MainThread
    override fun observe(owner: LifecycleOwner, observer: Observer<in T>) {
        if (hasActiveObservers()) {
            throw SingleLiveDataException("Multiple observers registered.")
        }

        super.observe(owner, Observer {
            if (pending.compareAndSet(true, false)) {
                observer.onChanged(it)
            }
        })
    }
}
@Suppress("RedundantOverride")
class MutableSingleLiveData<T> : SingleLiveData<T>() {

    public override fun postValue(value: T) {
        super.postValue(value)
    }

    @MainThread
    public override fun setValue(t: T?) {
        pending.set(true)
        super.setValue(t)
    }

    @MainThread
    operator fun invoke(param: T) {
        value = param
    }
}
class SingleLiveDataException(message: String) : Throwable(message)

ViewModel usage

    private val _effect = MutableSingleLiveData<CalculatorEffect>()
    val effect: SingleLiveData<CalculatorEffect> = _effect

in fragment, and it can be observed like this

private fun initEffect() = viewModel.effect
    .reObserveSingle(viewLifecycleOwner, Observer { viewEffect ->
        // Your Logic
    })

fun <T> SingleLiveData<T>.reObserveSingle(owner: LifecycleOwner, observer: Observer<T>) {
    removeObserver(observer)
    observe(owner, observer)
}
@Zhuinden
Copy link

@Zhuinden Zhuinden commented Apr 23, 2020

I saw this just today https://www.reddit.com/r/androiddev/comments/g6kgfn/android_databinding_with_livedata_holds_old_values/foabqm0/ and I think it's slightly less intrusive, as instead it just auto-clears the stored value when it is consumed. But you don't need any new types or new Observer subclasses to make it work.

@erlangp
Copy link

@erlangp erlangp commented Apr 29, 2020

how about use PublishSubject from rx :)

@Zhuinden
Copy link

@Zhuinden Zhuinden commented Apr 29, 2020

@erlangp PublishSubject doesn't retain the event if the event is emitted exactly during a config change and the view is not observing

I use this class: https://github.com/Zhuinden/event-emitter/blob/2d072e2aa7984a582379cdf40b9d58e665ba6840/event-emitter-sample/src/main/java/com/zhuinden/eventemittersample/utils/LiveEvent.kt#L65

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.