Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply badge state restoration on the view itself #478

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Apply badge state restoration on the view itself #478

wants to merge 6 commits into from

Conversation

kevinthecity
Copy link

@kevinthecity kevinthecity commented Sep 2, 2016

What?

If you try to use a badge after a restore instance state, you receive an error that the view state was improperly restored (causing a crash)

This is most easily visible (and frustrating) when trying to use the library in conjunction with Instant Run, which causes view state restoration a lot, but also happens on rotation. I've modified the BadgeActivity to apply the badge in onResume() which will also crash without this fix.

Reproduction steps

In the BadgeActivity, move the setBadgeCount() call into onResume(), and then rotate the device. You should receive this error:

AndroidRuntime  E  FATAL EXCEPTION: main
 E  Process: com.example.bottombar.sample, PID: 22353
 E  java.lang.RuntimeException: Unable to start activity ComponentInfo{com.example.bottombar.sample/co
    m.example.bottombar.sample.BadgeActivity}: java.lang.IllegalArgumentException: Wrong state class,
    expecting View State but received class android.os.Bundle instead. This usually happens when two v
    iews of different type have the same id in the same hierarchy. This view's id is id/tab_nearby. Ma
    ke sure other views do not use the same id.
 E      at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2416)
 E      at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2476)
 .....
 E      at android.app.ActivityThread.main(ActivityThread.java:5417)
 E      at java.lang.reflect.Method.invoke(Native Method)
 E      at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:726)
 E      at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:616)
 E  Caused by: java.lang.IllegalArgumentException: Wrong state class, expecting View State but receive
    d class android.os.Bundle instead. This usually happens when two views of different type have the
    same id in the same hierarchy. This view's id is id/tab_nearby. Make sure other views do not use t
    he same id.
 E      at android.view.View.onRestoreInstanceState(View.java:14771)
 E      at com.roughike.bottombar.BottomBarTab.onRestoreInstanceState(BottomBarTab.java:555)
 E      at android.view.View.dispatchRestoreInstanceState(View.java:14746)
 E      at android.view.ViewGroup.dispatchRestoreInstanceState(ViewGroup.java:3121)
 E      at android.view.ViewGroup.dispatchRestoreInstanceState(ViewGroup.java:3127)
 .....

This is happening because the view state is not being restored on the custom view itself.

How to fix

This PR implements the solution proposed by this Stack Overflow answer. Modified unit tests to work with the new changes as well.

You can test this by running the PR, opening the badge activity, and rotating the screen. It should no longer exhibit the crash.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.6%) to 55.275% when pulling 01c8aab on kevinthecity:badge-state-restoration into 3118116 on roughike:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.6%) to 55.275% when pulling 15ec61d on kevinthecity:badge-state-restoration into 3118116 on roughike:master.

@kevinthecity
Copy link
Author

Guess it's not the number of tests but the actual code coverage. O well! I think this is still worth merging, we took out some tests from the BottomBarTabTest that were no longer relevant (the onsavedInstanceState ones)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.6%) to 55.275% when pulling 097414a on kevinthecity:badge-state-restoration into 3118116 on roughike:master.

@kevinthecity
Copy link
Author

@roughike Did you end up fixing this in a different way?

@yombunker
Copy link
Collaborator

@kevinthecity is this still a thing in the latest version?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants