-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Feature/reader search results #4096
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
mzorz
merged 54 commits into
feature/reader-search-master
from
feature/reader-search-results
May 23, 2016
Merged
Changes from 53 commits
Commits
Show all changes
54 commits
Select commit
Hold shift + click to select a range
1d9c4e0
First pass at reader search service
nbradbury 24a4bf5
Submit search query on suggestion click
nbradbury 5813aed
Empty view for no results now works
nbradbury f9a5b4b
Added "Searching..." message while search is active
nbradbury ca18d72
Search results are now stored in main post table
nbradbury aa4b578
Updated getTagForSearchQuery
nbradbury 41477b5
Added ReaderPostListType.SEARCH_RESULTS
nbradbury 39f2a1f
Search results are now correctly displayed when search finishes
nbradbury 89190a4
Clear existing results before performing search
nbradbury b366ecc
Return max 10 results
nbradbury b543159
Set the autocomplete search threshold to 1
nbradbury 60a7fb2
Move setThreshold() to initial setup
nbradbury dcbfb3e
Show boxes & pages anim when searching
nbradbury 87840c5
Retain search activity title upon rotation
nbradbury 9e5aaf5
No longer removing previous search results
nbradbury c358566
Use custom layout for search suggestions
nbradbury 3f687aa
Apply max suggestions
nbradbury fef6b5c
Clean up getQueryStrings
nbradbury f5696bd
First pass at infinite scroll for search
nbradbury fd35877
Second pass at infinite scroll for search
nbradbury 1dfe743
Third pass at infinite scroll for search
nbradbury bd3551f
Parse score from search results to determine sort order
nbradbury 4367231
Updated comment about timestamps
nbradbury 7d529c4
Change post timestamp to REAL (double)
nbradbury 132fe18
First pass at showing search results in main reader (no separate acti…
nbradbury db99f1e
2nd pass at showing search results in main reader
nbradbury 7112499
3rd pass at showing search results in main reader
nbradbury 5e7d397
Fixed search message for landscape
nbradbury 32f883e
Fixed device rotation issues, now re-using empty view for search message
nbradbury 1363d6f
Simplified post adapter to use the current tag when searching
nbradbury 007ae27
No longer clearing cached searches again
nbradbury 0f1b346
Make sure toolbar showing search keyword is showing when returning to…
nbradbury d4cd5d0
Merge branch 'develop' of https://github.com/wordpress-mobile/WordPre…
nbradbury 72987f1
Use a CursorAdapter rather than a SimpleCursorAdapter for search sugg…
nbradbury 5136967
Enable deleting search suggestions
nbradbury 4032f4c
Renamed suggestion table and dropped counter
nbradbury 769acfb
Increment db version counter
nbradbury ca4c263
Rewrote `setEmptyTitleAndDescription` to better handle search
nbradbury d0912a9
Added `showEmptyView()` and `hideEmptyView()`
nbradbury 11eb8b8
Removed `newInstanceForSearch`
nbradbury 0808f6e
Show search message even in landscape
nbradbury 946eed1
Clear query when searchView is collapsed
nbradbury c973d68
Merge branch 'develop' of https://github.com/wordpress-mobile/WordPre…
nbradbury ffdf77a
Purge all searches when reader db is purged at startup
nbradbury dfca3da
Merge branch 'develop' of https://github.com/wordpress-mobile/WordPre…
nbradbury fcacbc1
Merge remote-tracking branch 'origin/feature/reader-search-master' in…
nbradbury 6662401
Simplified and clarified the usage of SearchPostsEnded
nbradbury 3a42260
Remove cached results before requesting search results
nbradbury d67aabe
Removed todo (separate issue filed)
nbradbury 3bbf7b6
Merge branch 'feature/reader-search-master' of https://github.com/wor…
nbradbury 84a5048
Merge branch 'feature/reader-search-master' of https://github.com/wor…
nbradbury 6c572bc
Don't repopulate search suggestions unless the filter has changed
nbradbury 0cc5bd2
Merge branch 'feature/reader-search-master' of https://github.com/wor…
nbradbury b64f2a8
Ensure `filter` isn't null
nbradbury File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -358,10 +358,6 @@ public void removeOnScrollListener(RecyclerView.OnScrollListener listener) { | |
| } | ||
| } | ||
|
|
||
| public RecyclerView getInternalRecyclerView() { | ||
| return mRecyclerView; | ||
| } | ||
|
|
||
| public void hideToolbar(){ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, might have been left from a previous refactor. Good catch. 👍 |
||
| mAppBarLayout.setExpanded(false, true); | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
17 changes: 9 additions & 8 deletions
17
WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderConstants.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering why this attribute is declared as a
double, being that elsewhere the timestamp can be held as along. The only place I can think of is maybe forcing this is this one https://github.com/wordpress-mobile/WordPress-Android/blob/feature/reader-search-results/WordPress/src/main/java/org/wordpress/android/models/ReaderPost.java#L122 - is that maybe because the API is returning a value there that could grow into a double? if not, then we should probably leave/refactor everything to be treated as long, as per this https://developer.android.com/reference/java/sql/Timestamp.htmlThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, until search came along, that
timestampcolumn really was a timestamp, but it's only purpose has been to determine the sort order of posts.Search results, however, include a "score" - a double which states how relevant each post is to the submitted query - and this determines how posts are sorted. To get this to work within the existing code, I changed the
timestampcolumn to a double and store the "score" in there.And now that I look at it, I confess to not being proud of it :)
What really should happen is for me to rename that
timestampcolumn (and related field in theReaderPostmodel) to better reflect its purpose. If that would address your (perfectly valid) complaint, I'd like to file that as a separate issue to avoid further bloating this PR.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely! Let's file that as a separate issue then and ref it here. I have to confess I doubted of the actual value of mentioning this at all but... when you write near-to-perfect code, it's easier for the reviewer to point fingers on stuff that don't really matter - LOL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Referenced here #4119
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hah! Thanks for the ego boost, but after you've been here for a while you may feel otherwise ;)
Update: About my code, that is!