Add support v17 Leanback #136
Conversation
|
I merged d353616 to master. It should have been a separate PR. |
| } | ||
| }; | ||
|
|
||
| final SearchEditText.OnKeyboardDismissListener emptyListener = new SearchEditText.OnKeyboardDismissListener() { |
There was a problem hiding this comment.
Why this and not just set null?
There was a problem hiding this comment.
There is no null check when onKeyboardDismiss is called. Please see SearchEditText on line 52. Setting null here will cause null pointer exception.
There was a problem hiding this comment.
Ah, interesting. I just sent a fix for it: http://r.android.com/170991
There was a problem hiding this comment.
Instead of creating this ahead of time, just new it up inside the onUnsubscribe method below. Then we don't pay the cost of allocation (even though it's suuuuuper small) until we actually unsubscribe.
|
Have any chance to review this? |
| subscriber.add(new MainThreadSubscription() { | ||
| @Override | ||
| protected void onUnsubscribe() { | ||
| view.setOnKeyboardDismissListener(emptyListener); |
There was a problem hiding this comment.
Can you add // TODO set to null once http://b.android.com/187101 is released. above this line.
|
Don't forget to rebase and squash when you fix up the things I commented on! This looks great though. Looking forward to it. |
|
|
||
| setContentView(R.layout.lb_search_fragment); | ||
|
|
||
| searchBar = (SearchBar) findViewById(R.id.lb_search_bar); |
There was a problem hiding this comment.
I can't new SearchBar(this) here. searchEditText will be null. In SearchBar class, mSearchTextEditor is created in onFinishInflate.
|
Fixed all the things. Thanks for the feedbacks. Not sure why the CI build failed. |
|
I'll look. There's one of two flaky tests, sadly. On Sun, Sep 27, 2015, 9:17 AM Prat notifications@github.com wrote:
|
|
Are you sure the tests pass? Those are the two that are failing on Travis CI. |
|
Yes, they did when I tested locally. I ran the tests a couple times and everything looked fine. Not sure why they failed on Travis. I will look into this. |
| @@ -5,7 +5,7 @@ android: | |||
| - build-tools-23.0.1 | |||
| - android-23 | |||
| - extra-android-m2repository | |||
| - sys-img-armeabi-v7a-android-15 | |||
| - sys-img-armeabi-v7a-android-17 | |||
There was a problem hiding this comment.
We're not even using this. The script below creates an emulator using API 18.
There was a problem hiding this comment.
Ok. I'll revert it back.
|
Jake, I fixed the test. it turns out one of the child views loading animations and sounds so it took more than 45 seconds to load the activity on some emulators. |
|
Nice work! Thanks. |
|
👍 |
Addresses #60
Also, fixes toString in SearchViewQueryTextEvent.