Add loyalty card filter

[TheLastProject]

Fixes #278

[TheLastProject]

I feel this is ready for review now :)

[brarcher]

Thanks for proposing some changes for this feature!

To help me in keeping this feature alive (and preventing breakage), are there new Robolectric tests which you could add to test the feature?

[TheLastProject]

I've added a filtering test and addressed your other comments. I believe I have hereby addressed all your concerns. Please tell me if there's anything else you feel needs changing.

[brarcher]

Tests look good, I appreciate it.

I loaded this onto a device to try it out. I've question about the search bar and when it is viewable. One pattern I've seen in other application is to have a menu item for search, that when clicked opens the search box. An example is in this tutorial:

https://youtu.be/9OWmnYPX1uc?t=57

I do not think that the search interface needs to be as complex that as that example. Maybe only hide the search bar until the search icon is clicked, then let it fill the app bar:

There is an example in another app that I wrote here:

https://github.com/brarcher/budget-watch/blob/master/app/src/main/java/protect/budgetwatch/TransactionActivity.java#L109

The relevant part of that example is setting up the search view in onCreateOptionsMenu(). The way your search filters the list on each character should be fine.

What do you think?

[TheLastProject]

That makes sense, especially because you need to manually focus the search bar anyway. I'll take a look at that.

[TheLastProject]

I've switched to using a SearchView, which indeed looks much better. Sadly, I can't figure out how to make the tests work at all. Got any pointers on how to send something to the search input?

[brarcher]

I looked into how to test this a little. In the test, retrieving the menu item SearchView is straight forward:

final Menu menu = shadowOf(mainActivity).getOptionsMenu();
MenuItem searchItem = menu.findItem(R.id.action_search);
searchItem.expandActionView();
SearchView searchView = (SearchView) searchItem.getActionView();

The tricky part is getting the query text listener. There is no getOnQueryTextListener() method on SearchView. If one had the listener, one could call its methods directly and simulate text input and check that it causes the correct response, namely that the card list is properly filtered.

To get at the listener, one approach is to make a Shadow.

http://robolectric.org/extending/

There is no Shadow of SearchView. One could create a simple one that only intercepted the setOnQueryTextListener() call and saved the result, then provided the result in the test.

How do you feel about creating a small Shadow for SearchView and adding it in a separate file along with the tests?

[TheLastProject]

Sounds like a plan, I'll try my best. Quite new to Android so may take me some time

[TheLastProject]

I made an attempt in https://github.com/brarcher/loyalty-card-locker/pull/320/commits/691497e24bc37a37916de0fd8f03e9393b4515ab but no matter how much I try I can't get it to work. It does seem to use the shadow class, but I just get a NullPointerException now during onCreateOptionsMenu :(

[brarcher]

I gave it an attempt as well, starting from your efforts. I find that when the SearchView shadow is being used the menu item no longer has its action view. This is the reason for the NullPointerException. Tinkering with it and looking online I could not figure out why this is the case.

Although it does not fix this issue, I think your shadow was close. In order to send the listener to the real object, one would need a reference to the real object:

@RealObject private SearchView realSearchView;

then pass the listener along:

@Implementation
    protected void setOnQueryTextListener(SearchView.OnQueryTextListener listener) {
        onQueryTextListener = listener;
        realSearchView.setOnQueryTextListener(listener);
    }

You gave it a shot, and I appreciate that. Go ahead and remove the last few test commits, and I'll accept the pull request.

[brarcher]

Oh, there were a few small comments about the implementation. Could you resolve those? Then I'll accept the branch.

[TheLastProject]

I addressed your comments and I added a "filter" variable to persist the filter through resuming and so we can have some tests anyway. I have also tested manually to make sure that change didn't cause any regressions.

[brarcher]

Thanks for contributing this feature!