Conversation
…id into feature/simple_post_search # Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/posts/PostListMainViewModel.kt # WordPress/src/main/java/org/wordpress/android/ui/posts/PostsListActivity.kt # WordPress/src/main/res/layout/post_list_activity.xml
…tting viewmodel on search query.
…id into feature/simple_post_search
- Refactored LiveData sources. - Throttled empty view.
…id into feature/simple_post_search
|
Thanks for the review, @malinajirka ! Looks like ThrottleLiveData fixed the empty screen in post list :) I implemented a majority of your suggestions and left a couple of comments/questions :) |
|
Thank you so much for the changes @khaykov! The code is way simpler now ❤️! I've tested the search and it works like a charm 🎉. I've left one last suggestion and one question/suggestion. But I think it's already ready to be merged. Good job:)! |
WordPress/src/main/java/org/wordpress/android/ui/posts/PostListFragment.kt
Outdated
Show resolved
Hide resolved
WordPress/src/main/java/org/wordpress/android/ui/posts/PostListFragment.kt
Outdated
Show resolved
Hide resolved
| } | ||
| connector = postListViewModelConnector | ||
|
|
||
| initList(null, dataSource, lifecycle) |
There was a problem hiding this comment.
I'm sorry for coming back to this, but I'm still not sure I understand why we can't wrap initList in the start with if (connector.postListType != SEARCH) and get rid of both isEmptySearch calls.
We would get rid of the first initial request with empty searchQuery -> which fetches all the published/private posts on the site even though we throw them away. Wdyt?
|
@SylvesterWilmott Updated the screenshots! @malinajirka I think I addressed all of your comments :) Ready for another round! |
|
Both the feature and the code looks great now 😍, thanks!! I've encountered two minor issues during the last test.
I think neither of the issues is critical. I'm approving this PR. Feel free to merge it and fix the issues in a different PR or fix them here. It's up to you. Thanks! 🙇 |
|
Looks all good now;)! Thanks! |
|
Beta tested this using WPAndroid alpha-177 (12.8 alpha) on Pixel 3 Android 9 and it looks great! 👍
I did expect searching for drafts to work (before I read the PR which notes that only searching for published/private posts work so far) and wanted to note there is a request already open for that at #9183. 😍 |
Fixes #62
This PR adds the ability to search published and private sites on jetpack/wpcom sites.
Implementation
I used Pages Search functionality as a reference. Pages Search is using a dedicated search fragment, ViewModels, and adapters. I originally tried to go the same route but realized that this brings a lot of extra code that we will have to maintain alongside existing Post List, so I decided to try and use existing PostListFragment for search (huge mistake, but too late!)
The current implementation of Post List is pretty robust, and meant to work in a "fire and forget" kind of way, so I had to make a couple (maybe a lot) of minor changes to accommodate special behavior of Search list. I tried not to touch the core of PostListViewModel, so the changes are spread through a couple of helper classes and PostListFragment
Search button logic in the toolbar is mostly copied from Pages List, with a couple of minor changes.
UX
To test:
Make sure the post Search button is not visible for self-hosted sites without a jetpack.
Make sure post search works as expected.
Update release notes:
RELEASE-NOTES.txt.