Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upShowing Snackbar via MVI #255
Comments
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
sevar83
Jun 18, 2017
I just use some flag in the view that is set to true on the first scroll. After rotation it's not a problem because the presenter is kept and the last rendered (not the initial one) ViewState is emitted again. So the scrolling happens just one time as it should be. This could work also with stuff like animated transitions.
sevar83
commented
Jun 18, 2017
|
I just use some flag in the view that is set to true on the first scroll. After rotation it's not a problem because the presenter is kept and the last rendered (not the initial one) ViewState is emitted again. So the scrolling happens just one time as it should be. This could work also with stuff like animated transitions. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
dimsuz
Jun 18, 2017
Interesting solution with OneShotFlag, but can present a bit of an overhead if there are a lot of such flags.
As for me I usually have my ViewState models have a method clearTransientState() which resets all one-shot state variables to their default values (by calling this.copy()) and it is the first to be called in the state reducer function. Of course, one needs to update this method every time a new one-shot variable is added, but as it is a part of a ViewState data class, I usually do not forget.
dimsuz
commented
Jun 18, 2017
•
|
Interesting solution with As for me I usually have my ViewState models have a method |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
qwert2603
Jun 19, 2017
Contributor
@dimsuz but some flags may be lost in a situation when several ViewStates will be created while Fragment (or Activity) is recreating and only last ViewState will be rendered when View is reattached.
|
@dimsuz but some flags may be lost in a situation when several ViewStates will be created while Fragment (or Activity) is recreating and only last ViewState will be rendered when View is reattached. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
sockeqwe
Jun 19, 2017
Owner
Super interesting conversation! Thanks for sharing your strategies.
I think all of your solutions are valid and work for your use case but not necessarily for the general case.
With that said, I still think that sinking the information, i.e. that you have scrolled to a given position, down as intent through a state reducer is the way I would implement something like this and how I would recommend to implement it. That might seems to be over architected and over complicated and by no means I'm saying that you should change your OneShotFlag solution, or clearTransientState() as described by @dimsuz . I just can speak for myself and for me the most important thing is that the "model" is immutable and driving the state. Also I don't see a major difference to what @dimsuz or @sevar83 recommended: They all have to call 1 extra method (clearTransientState() or some flag in the view). So it's the "same amount of work" compared to firing a dismissSnackBarIntent().
but there may be a sutuation when several new ViewState instances created before SnackbarShownIntent will be triggered, so Snackbar will be shown several times
I think this is just an implementation detail of your UI. If Snackbar is already showing then don't display it again. State Reducer guarantees that the order of those render() events dont matter: at the end you are in the correct state. I'm also experimenting around with the idea of using Rx backpressure to disallow such things like calling render(newState) while "old state is not entirely finished with rendering. In this example "rendering is finished" could mean after snackbar disapeared (might be easier to implement with another base presenter class #255). Not sure if this solves this problem though, but I think it could be useful in the case of RecyclerView Item animations where one render call might trigger an animation of a item and a consecutive render call might cause a that the item that is animating right now will removed. Sure, we could coordinate this in the View layer, but if there would be a way to signalize when view.render() is finished we simply could wait until item animation is finished (first view.render() call) before dispatching the next view.render() call which would remove the item. With unidirectional data flow and Rx backpressure we could get this for free.
but it pollutes StateResucer if there are several such flags.
True but at the same time you get the guarantee that state is always the correct regardless of amount of events happening over time as they all go the same way through state reducer.
|
Super interesting conversation! Thanks for sharing your strategies. With that said, I still think that sinking the information, i.e. that you have scrolled to a given position, down as intent through a state reducer is the way I would implement something like this and how I would recommend to implement it. That might seems to be over architected and over complicated and by no means I'm saying that you should change your
I think this is just an implementation detail of your UI. If Snackbar is already showing then don't display it again. State Reducer guarantees that the order of those render() events dont matter: at the end you are in the correct state. I'm also experimenting around with the idea of using Rx
True but at the same time you get the guarantee that state is always the correct regardless of amount of events happening over time as they all go the same way through state reducer. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
qwert2603
Jun 20, 2017
Contributor
@sockeqwe @dimsuz Thank You for your comments.
Let's imagine the situation when:
- View is detached
- ViewState_1 with
showSnackbar=trueis created - ViewState_2 with
showSnackbar=falseis created, flag is reset as result of callingclearTransientState() - View is attached
- ViewState_2 is rendering.
In such situation flagshowSnackbar=trueis lost.
Another approach when we have intent dismissSnackBarIntent(). There may be a situation when:
- ViewState_1 with
showSnackbar=trueis created - ViewState_1 rendering begin
- ViewState_2 is created and it have
showSnackbar=truelike in ViewState_1 - ViewState_1 rendering end
dismissSnackBarIntent()is triggered- ViewState_2 is rendering. Snackbar is showing again.
Of course, nothing wrong is we show Snackbar twice. But there may be more important events likemoveToAnotherScreenorshowCormirmationDialog.
I'm also experimenting around with the idea of using Rx backpressure to disallow such things like calling render(newState) while "old state is not entirely finished with rendering.
This may solve the problem, but we can't wait seconds for Snackbar's dismissing or animation's finish.
Also I'd like to say that OneShotFlag becomes immutable after first call OneShotFlag#getFlag(vsId: Long). It returns same value for same ViewState_Id. Moreover var readId: Long? = null can be private.
|
@sockeqwe @dimsuz Thank You for your comments.
Another approach when we have intent
This may solve the problem, but we can't wait seconds for Snackbar's dismissing or animation's finish. Also I'd like to say that |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
qwert2603
Jun 20, 2017
Contributor
@sevar83 I used such approach before, but refused because I try to make View as passive as possible. And this is duplicating flags in ViewState and View (represented by Fragment or Activity).
|
@sevar83 I used such approach before, but refused because I try to make View as passive as possible. And this is duplicating flags in ViewState and View (represented by Fragment or Activity). |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
sockeqwe
Jun 22, 2017
Owner
In such situation flag showSnackbar=true is lost.
Sounds like the expected behavior for me OR your state reducer should work differently then and set showSnackbar to whatever you would like it to be. I don't see how this makes any difference in having a view attached or not.
Also you can call dismissSnackBarIntent() immediately in render() once the snackbar has been fired. This is just an implementation detail, and SnackBars and Toast behave differently then regular UI widgets. Something like this:
// View Layer
public void render(MyState state) {
if (state.showSnackBarError) {
SnackBar.make(view, "An error has occurred", LONG).show()
fireSnackBarErrorShownIntent();
}
}So fireSnackBarErrorShownIntent() will set the state properly ((state.showSnackBarError=false) and you are back on the right state (although the SnackBar itself is shown for few more seconds on the screen, but again I think this is ok since it is just a UI implementation detail). Of course fireSnackBarErrorShownIntent() will cause to render() state again but then no UI widget will be update, so no big deal.
Sounds like the expected behavior for me OR your state reducer should work differently then and set showSnackbar to whatever you would like it to be. I don't see how this makes any difference in having a view attached or not. Also you can call // View Layer
public void render(MyState state) {
if (state.showSnackBarError) {
SnackBar.make(view, "An error has occurred", LONG).show()
fireSnackBarErrorShownIntent();
}
}So |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
qwert2603
Jun 27, 2017
Contributor
Sounds like the expected behavior for me
It all depends on use case.
Looks like there is no simple way to solve "Snackbar's problem". Anyway we should write some additional code :)
It all depends on use case. |
qwert2603 commentedJun 17, 2017
•
edited by sockeqwe
Hello! I was really excited by MVI patters and concept of StateReducer particularly. It seems to be very useful and powerful. But there is a problem with some actions that should be executed once. For example, show Snackbar or scroll list to top after refreshing. In some cases this actions should be executed only once, because user doesn't expect that list will be scrolled to top after screen rotation. In comments to Your article (http://hannesdorfmann.com/android/mosby3-mvi-1) I found two ways of making such actions:
whatever.
After some research and experimenting I found some solution when such action will be executed once and only once. I called them
OneShotFlag. Here is what have I come to:Output is:
Class
OneShotFlagrepresents action that should be executed once. It hasreadIdfield where stored ID of ViewState that was being rendered when action was executed. So (as we can see infun getFlag(vsId: Long)) this action will not be executed when we will render another VS.Moreover if later we will render VS with
VS.id == OneShotFlag.readIdaction will be executed again. So we still have Undo/Redo support while renderind ViewStates.This is experiment and I'm looking forward for Your comments and critique. Does it seems to be good approach?