View actions (snackbar, activity navigation, ...) in ViewModel #63
Comments
|
We created something similar in Blueprints called and another version called `SnackbarMessage' 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 |
|
In my implementation I am saving a list of events but in the |
|
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
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 |
|
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 |
Well, I wasn't aware of this implementation detail. I was just saying that to me invoking something like
I cant see why not.
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?
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). |
|
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:
As already rectified on your blog post,
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... |
Yes, it will. This check prevents reentrance in |
|
I have worked a little bit on refactoring the So I have introduced a class SearchState. This class basically represents state and can be "rendered" directly by the view. So instead of 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.
@fabioCollini yes, it does. It works as you can see in my linked example repository. Regarding @JoseAlcerreca questions:
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).
Yes, naming is not great.
Yes, this are two different approaches: "fire and forget" vs. "single state driven by the business logic". 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). |
|
i've implemented similar approach for ui commands and Snackbar example here it would be great to have such functionality out of the box in the upcoming lib release |
|
@deviant-studio your Command has two problems:
|
|
@JoseAlcerreca true. that is why i'm going to use your |
|
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 A seudo implementation of this would be: dispatch(SHOW_PROGRESS)
//do long work
loadPosts()
dispatch(HIDE_PROGRESS)
dispatch(SHOW_RESULTS)In Activity: 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 |
|
@JoseAlcerreca We tried your SingleLiveEvent implementation and are having problems when using it together with I created a naive implementation (in Kotlin) which works for us: https://gist.github.com/etiennewelsch/142ab9e080b0144927fd2486cd34feb9 |
|
@JoseAlcerreca I am successfully using your |
|
@etiennewelsch |
|
@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 |
|
This thing is indeed a interesting discussion. 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 Why I listen to LiveData in a ViewModel you think? fun getUser(): User {
repo.getUser()
}I have a similar approve like @sockeqwe. Yeah a lot of text i know 🙃 But: Are there other solutions? |
|
@StefMa of course you can use the LiveData in the ViewModel, just observe it from the activity. You can create a |
|
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. |
|
@JoseAlcerreca We came across a possible bug in the |
|
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 |
|
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. 🙄 |
|
It uses |
|
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. ` } ` |
… 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
|
Have you folks seem this ? It seems that the support to this kind of thing should be in the framework. |
|
I have created an improved version of @JoseAlcerreca's SingleLiveEvent that supports multiple observers. No need for a custom observer class or value wrapper. |
|
@guelo @Zhuinden I attempted to address this problem by creating a variant of the 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. |
|
When I tried using SingleLiveEvent in my code, it is giving me error |
|
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. |
|
That still doesn't work
|
|
@alexakasanjeev you seem to be using a different version of the library than me. simply change |
|
I have published example with EventsQueue, see here In short version: |
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.. |
|
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 It'd be possible if I wanted to. |
|
In reality, the ViewModel might not have reference to the View but someone must have to. 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. |
|
@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? |
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.
If the View describes its own events via an interface so it exposes things like Although that is something that MVP should also be doing, the P or VM difference lies in whether
I guess it could work, although it seems tricky that you need to explicitly call both |
|
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> = _effectin 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)
} |
|
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. |
|
how about use PublishSubject from rx :) |
|
@erlangp |
How is that even related?! Unless you're trying to show how impotent the "LiveData" library is. |
|
Simple, LiveData was not meant to be an EventBus, and now we are trying to use it as an EventBus. Any solution piggybacking LiveData to model one-off events is basically a hack. |
and others
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.
The text was updated successfully, but these errors were encountered: