Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
This repository was archived by the owner on Feb 26, 2023. It is now read-only.

Support instance state for views#1911

Merged
WonderCsabo merged 3 commits into
androidannotations:developandroidannotations/androidannotations:developfrom
dodgex:1312_InstanceState_support_for_ViewsCopy head branch name to clipboard
Dec 10, 2016
Merged

Support instance state for views#1911
WonderCsabo merged 3 commits into
androidannotations:developandroidannotations/androidannotations:developfrom
dodgex:1312_InstanceState_support_for_ViewsCopy head branch name to clipboard

Conversation

@dodgex

@dodgex dodgex commented Dec 10, 2016

Copy link
Copy Markdown
Member

This PR is based on the work of @simopete.

What is the goal?

Adding support of @InstanceState to custom views.

What is changed from the original work?

  • Update copyright header
  • Added a test case

@dodgex dodgex requested a review from WonderCsabo December 10, 2016 13:04
}

@InstanceState
String stateTest = "does it work?";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the field before the constructors.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

savingView.stateTest = "it works!";
Parcelable savedInstanceState = savingView.onSaveInstanceState();

ViewWithInstanceState_ resotringView = (ViewWithInstanceState_) ViewWithInstanceState_.build(context);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

import android.view.View;

@EView
public class ViewWithInstanceState extends View {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need an extra non aa custom view? shouldn't it be enough to have custom save/restore in this view class?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this case into the current one. hope it is ok?

super(context);
}

public ViewWithInstanceState(Context context, AttributeSet attrs) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This constructor is not needed.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@WonderCsabo WonderCsabo merged commit 314f3e3 into androidannotations:develop Dec 10, 2016
@WonderCsabo WonderCsabo added this to the 4.2 milestone Dec 10, 2016
@dodgex dodgex deleted the 1312_InstanceState_support_for_Views branch December 10, 2016 17:28

@Zhuinden Zhuinden left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@dodgex

dodgex commented Feb 10, 2019

Copy link
Copy Markdown
Member Author

@Zhuinden not sure what exactly you mean? could you explain the issue you have and maybe suggest what should happen instead ? Thanks!

Did you guys ever fix this, or has it been broken for 2.5 years?

If there is an issue with that, I'm sure that it is not fixed yet, as i never heard of it before.

@WonderCsabo

Copy link
Copy Markdown
Member

@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);
    }

@Zhuinden

Copy link
Copy Markdown

Ok in that case it should most likely work, albeit there is a chance that extending RecyclerView could have problem if bundle.setClassLoader(RecyclerView.class.getClassLoader()) isn't called 🤔

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.