Search UI overhaul

[rdbende]

Done:

• Move search into a stack page in the main view instead of the popover
• Refactor code that should actually belong to the viewmodel

Remaining issue:

• Since the space key is a global shortcut for play/pause, it can't be inserted into the search bar

[rdbende]

Remaining issue:

• Since the space key is a global shortcut for play/pause, it can't be inserted into the search bar

Disabling the action while the search view is open solved the issue.

TODO:

• Add Ctrl+F as shortcut

[geigi]

The new search looks really cool! Thanks for implementing it. It seems like there are two small merge conflicts but other than that it looks good to me :)

[rdbende]

After the PRs related to UI are merged, I should really do #801. Resolving conflicts in the XML files is such a mess :(

@naglis could you give an approval on my conflict resolution?

[naglis]

I did not notice any problems in the conflict resolution :)

A few things I have noticed about the search while testing locally:

1. self._fs_monitor.get_book_online is called twice for each book on each search (IIUC once when SearchViewModel.authors property is called and once for SearchViewModel.readers property).
2. "Hide unavailable books" setting is not respected for book results - if I have it enabled, I still get book results from storage that is offline (NOTE: AFAIR, this feature did not work at all prior to this PR). IIUC this is because in SearchViewModel.search() the books set is built directly from self._model.books, without checking if the books are online.

I believe both issues could be resolved e.g. if the search was further refactored by first getting the "online" books (if "Hide unavailable books" setting is enabled, all books otherwise) and then using it for both computing the set of books that match the query and building the author/reader results as well. This way self._fs_monitor.get_book_online would be called once per book. Not sure if this should be done in a separate PR.

[rdbende]

Codeclimate is complaining about non-existent problems again :/

[rdbende]

A few things I have noticed about the search while testing locally:

1. self._fs_monitor.get_book_online is called twice for each book on each search (IIUC once when SearchViewModel.authors property is called and once for SearchViewModel.readers property).

2. "Hide unavailable books" setting is not respected for book results - if I have it enabled, I still get book results from storage that is offline (NOTE: AFAIR, this feature did not work at all prior to this PR). IIUC this is because in SearchViewModel.search() the books set is built directly from self._model.books, without checking if the books are online.

Fixd.

[rdbende]

@naglis, could you give a final approval, so that GH allows me to merge this?
It blocks many other things and will cause conflicts with other PRs, so it would be good to have it in master. I can do the rest in separate pull requests if needed.

[naglis]

@naglis, could you give a final approval, so that GH allows me to merge this? It blocks many other things and will cause conflicts with other PRs, so it would be good to have it in master. I can do the rest in separate pull requests if needed.

@rdbende I think you need approval from someone with write/admin permissions (not sure how it is configured), as my approvals are gray.

[rdbende]

Huh, yeah. Never mind. Apparently this approval doesn't work if you don't have write access. I didn't know about it 😕

[rdbende]

Anyway, your reviews were useful, so thanks for that!

[geigi]

Finally merged! Thanks for the additional review @naglis and the fixes @rdbende!