BookPlayer icon indicating copy to clipboard operation
BookPlayer copied to clipboard

feature[library]: search through books

Open geekygrappler opened this issue 3 years ago • 7 comments

Purpose

  • Add basic search to the library

Linear tasks

#531

Things to be aware of / Things to focus on

  • Not sure if I'm updating the data in the correct way, using updateSnapshot but it works 😅
  • Authors seemed to be under book.details on my simulator. Not sure if this is correct or could change with different data?
  • I hard coded the placeholder "Search by Title or Author". Not sure how the localization works in the app, show me an example and I'll update it.

Screenshots

https://user-images.githubusercontent.com/3627301/136791281-107b3521-0b0e-405a-98f0-9394e14727a8.mp4

geekygrappler avatar Oct 11 '21 11:10 geekygrappler

Regarding localizations, in the BookPlayer folder in Xcode, we have at the top level two Localizable groups(?), go with the second one (see image)

Screen Shot 2021-10-11 at 11 35 46

In there we have the files for each language, you should add an entry at the bottoms of each file with the key and value of the translation. For the time being you can just set them all to english, and I'll add them later to lokalise.com, for our contributors to help with the translation to the other languages 👍

To make use of the new key added, we have an extension on the String class, so you can add a .localized after the string, and it should work, like this:

Screen Shot 2021-10-11 at 11 38 45

GianniCarlo avatar Oct 11 '21 15:10 GianniCarlo

For the filtering criteria, I think we have to first define granularity, if we're just doing a search at that specific level/screen, or if it should go deeper to the items inside the folder. This shouldn't impact performance, we would just query CoreData with the Book class for items with a title/author similar to the searched one.

This is only considering books, but if we want to throw folders into the mix, would we want to filter by title and show two sections? 🤔 for this last point I'm just thinking out loud, and it could be easily moved over to a following PR if the need arises cc @pichfl

GianniCarlo avatar Oct 11 '21 15:10 GianniCarlo

I'll continue reviewing later today @geekygrappler , but I do think that we have to query CoreData directly instead of querying the loaded items, since we won't be filtering all the items correctly now that we are paging the items (initial page size is 13 items). I'll provide more details in later comments

GianniCarlo avatar Oct 11 '21 16:10 GianniCarlo

Yes you are right, the snapshot is just 13 items.

geekygrappler avatar Oct 11 '21 19:10 geekygrappler

I will probably need help to understand how data loading works. I don't understand the difference between Book and SimpleLibraryItem, and which one we need, and how to get hold of the data.

E.g. self.viewModel.getInitialItems(); seems to be an Array of SimpleLibraryItem but I found: self.viewModel.dataManager.getLibrary().items gives... I think an array of Book.

Anyway at the moment I'm stuck on it.

geekygrappler avatar Oct 11 '21 20:10 geekygrappler

This is only considering books, but if we want to throw folders into the mix, would we want to filter by title and show two sections? 🤔 for this last point I'm just thinking out loud, and it could be easily moved over to a following PR if the need arises cc @pichfl

I think we could probably start with a simple filter for the current list/view and extend the search further down the line.

pichfl avatar Oct 12 '21 09:10 pichfl

I will probably need help to understand how data loading works. I don't understand the difference between Book and SimpleLibraryItem, and which one we need, and how to get hold of the data.

Book is the CoreData backed class, and SimpleLibraryItem is the lightweight struct we use for the tableview, so we avoid most of the CoreData baggage.

Look into the definition of the method fetchContents(of:or:limit:offset:) which is used to fetch the items for a given screen in the library. We should either expand or create a new method that has the text to filter with, and add that to the Predicate of the existing core data query. The result should be stored in a different variable than the current items so we don't lose the reference of how many items the user has loaded

Hit me up on Discord and we can go over it at greater detail 👍

GianniCarlo avatar Oct 13 '21 23:10 GianniCarlo

I have no opinions regarding the technical implementation, but I really like the feature - I have quite many books in my library and this kind of function would be nice

Doctor-love avatar Sep 04 '22 10:09 Doctor-love

Closing this PR due to inactivity, as an additional note, to avoid the problems with the animation of the search bar, we can just have a magnifying glass button in the nav bar, and have it push a new screen dedicated to search results

GianniCarlo avatar Sep 29 '22 11:09 GianniCarlo