Not static, leak free
consp1racy opened this issue · 5 comments
Hello, I like where this is going. I hope these suggestions help you building a robust tool people can use:
1)
Ditch ìnitWith as it does nothing. Have a public KeyboardWatcher(Activity activity) constructor instead. The client activity will hold a reference to an instance. So you can have multiple activities using keyboard watcher at the same time. Activity will create an instance in onCreate and keep it.
2)
Use a more robust way to get the root view. android.R.id.content can change by calling Activity.setContentView(...). Activity.getWindow().getDecorView().getRootView() will not. See http://stackoverflow.com/a/11553582/2444099. Store the reference right after you get the activity in constructor.
3)
Define private final ViewTreeObserver.OnGlobalLayoutListener viewTreeObserverListener = new GlobalLayoutListener(); You can do this because rootView is resolved at the point of execution.
4)
Use weak references for activity, rootView and onKeyboardToggleListener. As soon as the activity is destroyed, you lose the references and prevent leaks.
5)
Rename bindKeyboardWatcher to setListener. Remove unbindKeyboardWatcher because setListener(null).
6)
Consider having addListener and removeListener instead of setListener.
Thanks for your suggestions. We'll go through them and make sure to improve quality of our code
@consp1racy Thank you for your comment!
I have found issue in 2 point:
On keyboard shown
activityRef.get().findViewById(android.R.id.content).getHeight() = 530
activityRef.get().getWindow().getDecorView().getRootView().getHeight() = 1280
Ah, so the root view takes up the whole screen....
Ok, we can store activity.getWindow().getDecorView().getRootView() in the rootView variable. Then in the layout listener we can access rootView.getChildAt(0) to get the content view which should have its height adjusted as expected.
Merged, thanks for it! :)