Code review comment for lp:~ahayzen/music-app/add-sdk-search-support

Revision history for this message
Victor Thompson (vthompson) wrote :

Issues:

1. I'm experiencing crashing at times when searching within a Playlist (SongsPage). I haven't figured out what the output is yet
2. I see the following in an Album's SongsPage (song title's are corrupt): http://i.imgur.com/CTgRlFP.png
3. Filtering in the SongsPage also changes the number of tracks in an Album, if the page is for an album. That doesn't seem like something that should change.
4. Sometimes the searching stops working. Not sure what the cause is here. Entering a search doesn't filter anything if you play around for awhile.

Overall, I think it's confusing to search within the AlbumsPage and SongsPage. The *only* argument I can think of is searching within a Genre. But the same use case can be accomplished by searching the Songs Tab. I also don't see a reason to filter the MusicStart tab. Maybe we should also add a Genres tab, because that's the only other item I think could use some filtering. It seems inappropriate to do so from the Music tab. Removing the SongsPage, AlbumsPage, Music tab, and anything playlist related would seem to fix the first three issues--maybe the fourth as well.

I'll say, filtering for an Artist or Album is probably the greatest usecases of having a search. It's pretty slick.

review: Needs Fixing

« Back to merge proposal