Support instance state for views #1911
Conversation
| } | ||
|
|
||
| @InstanceState | ||
| String stateTest = "does it work?"; |
There was a problem hiding this comment.
Move the field before the constructors.
| savingView.stateTest = "it works!"; | ||
| Parcelable savedInstanceState = savingView.onSaveInstanceState(); | ||
|
|
||
| ViewWithInstanceState_ resotringView = (ViewWithInstanceState_) ViewWithInstanceState_.build(context); |
| import android.view.View; | ||
|
|
||
| @EView | ||
| public class ViewWithInstanceState extends View { |
There was a problem hiding this comment.
I think we should have another test, which asserts we correctly retain the saved instance information from the super class. So there should be a non-AA Android custom View class, which have onSaveInstanceState() and onRestoreInstanceState(), then an enhanced class should subclass it, and add @InstanceState. We should assert that the state restoration in parent works as well.
There was a problem hiding this comment.
do we need an extra non aa custom view? shouldn't it be enough to have custom save/restore in this view class?
There was a problem hiding this comment.
It is enough, if you can check if the state in the superclass is properly saved, however i think you cannot do that. Maybe you can extend from another View class, which saves state we can assert against, but the most clean would be writing a non-AA custom View.
There was a problem hiding this comment.
I added this case into the current one. hope it is ok?
| super(context); | ||
| } | ||
|
|
||
| public ViewWithInstanceState(Context context, AttributeSet attrs) { |
There was a problem hiding this comment.
This constructor is not needed.
Zhuinden
left a comment
There was a problem hiding this comment.
Interesting... Using Bundle as the Parcelable implementation of View's saved instance state will make it lose the parent classes' persisted states, that's why they created View.BaseSavedState in the first place.
Did you guys ever fix this, or has it been broken for 2.5 years?
|
@Zhuinden not sure what exactly you mean? could you explain the issue you have and maybe suggest what should happen instead ? Thanks!
If there is an issue with that, I'm sure that it is not fixed yet, as i never heard of it before. |
|
@Zhuinden i think you are not correct. The parent class' state is saved and is put into the bundle, later extracted and restored. It should work well. @Override
public Parcelable onSaveInstanceState() {
Parcelable instanceState = super.onSaveInstanceState();
Bundle bundle_ = new Bundle();
bundle_.putParcelable(INSTANCE_STATE_KEY, instanceState);
saveInstanceState(bundle_);
return bundle_;
}
private void saveInstanceState(Bundle bundle) {
bundle.putString("something", something);
}
@Override
public void onRestoreInstanceState(Parcelable state) {
Bundle bundle_ = ((Bundle) state);
Parcelable instanceState = bundle_.getParcelable(INSTANCE_STATE_KEY);
something = bundle_.getString("something");
super.onRestoreInstanceState(instanceState);
} |
|
Ok in that case it should most likely work, albeit there is a chance that extending RecyclerView could have problem if I'd have to test it to know if that happens or not. However, as you do save out the super-state, my initial claim was incorrect, thanks |
This PR is based on the work of @simopete.
What is the goal?
Adding support of
@InstanceStateto custom views.What is changed from the original work?