architecture-components-samples
architecture-components-samples copied to clipboard
View actions (snackbar, activity navigation, ...) in ViewModel
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.
We created something similar in Blueprints called SingleLiveEvent
https://github.com/googlesamples/android-architecture/blob/dev-todo-mvvm-live/todoapp/app/src/main/java/com/example/android/architecture/blueprints/todoapp/SingleLiveEvent.java
and another version called `SnackbarMessage'
https://github.com/googlesamples/android-architecture/blob/dev-todo-mvvm-live/todoapp/app/src/main/java/com/example/android/architecture/blueprints/todoapp/SnackbarMessage.java
We're thinking whether it should be part of the library.
Thoughts?
Edit: a blog post about events, SingleLiveEvent and a possibly better approach: https://medium.com/google-developers/livedata-with-snackbar-navigation-and-other-events-the-singleliveevent-case-ac2622673150
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 :smile: .
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).
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!
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!
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.
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 ...
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
@sockeqwe since your blog post has attracted so much attention, can you confirm the following? It seems that your solution:
- requires more logic in the activity (onDismissed listener and a null check)
-
setLoadMoreErrorShow
should be calledresetLoadMoreState
, or to use the original,setLoadMoreShown
, not sure why you added "Error" there. - 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().
@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...
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.
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:
- 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).
- setLoadMoreErrorShow should be called resetLoadMoreState, or to use the original,setLoadMoreShown, not sure why you added "Error" there.
Yes, naming is not great.
- 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?
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
@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.
@JoseAlcerreca true. that is why i'm going to use your SingleLiveEvent
implementation ;)
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.
I love the idea of SingleLiveEvent
but i'm thinking, theres 10~ish callbacks, is it better to have n unique SingleLiveEvent
s or would it be better to have a singleliveevent passing in different event types?
@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
@JoseAlcerreca I am successfully using your SingleLiveEvent
and vote for including it in the library when released.
@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.
@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!
@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 AndroidViewModel
s of events in our business logic. So there is no LifecycleOwner
to pass to observe
.
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? 🤔
@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.
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
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.
@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.
@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?
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.
@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.