[feature/localsearch] Local account-wide search using custom queries #933
Conversation
|
|
felix-schwarz
changed the title
- replace all/folder text in scope selection with icons on iOS 13+
- turn localizedName from a function into a property - add sortPropertyName to simplify construction of custom OCQuerys - ClientQueryViewController - add sort property, sort order and result limits to OCQueryCondition - refresh search query when changing sort order and peak results
… on a view - ServerListTableViewController - add user activity to allow state restoring to server list (previously only *always* to the last opened bookmark) - remove auto login code for pre-iOS 13 (in iOS 13 that's handled by scene/state restoration) - add inline status reporting for database migrations - SceneDelegate: add support to state restore to server list - ClientQueryViewController - switch to search tokenizer to construct OCQueryCondition - make sure sort / search options are no longer hidden when showing large "No results" message - ClientRootViewController - add support for migration progress reporting / OCCoreBusyStatusHandler passing - fix OCFileProviderServiceStandby leak if OCCoreManager returns a request with an error - FileProvider extension - return an error when trying to access an account whose database needs migration - handle case where no OCCore is available - MessageView: add option to show messages with insets at the edges - NSDate+ComputedTimes: - simplified computation of beginning of days, weeks, months and years, with support for offsets - add unit test - OCQueryCondition+SearchSegmenter: - segmentation of search queries into terms and "keywords" - supports placing terms in "" as well as unclosed " - keyword support to filter for: - files (:file), folders (:folder), - images (:image), video (:video) - time frames (:today, :week, :month, :year) - dynamic time frames (:7d, :2w, :1m, :1y, days:7, weeks:2, month:1, year:1, …) - file types/suffixes (type:jpg) - add unit test
… there could be more results
…ing an item and then navigating back to search - ClientQueryViewController: adapt to QueryFileListTableViewController changes
- add navigationHandler property to allow custom navigation actions to get triggered by the breadcrumb view - ClientItemCell - add reveal button support - ClientQueryViewController / QueryFileListTableViewController - allow providing an item to reveal and highlight - allow specifying if bread crumbs should push new view controllers - show reveal arrows for custom queries
…ning - use search term segmentation also for folder search (via OCQueryCondition+Item)
… least one character before using them
- add concept and property of activeQuery to QueryFileListTableViewController - adopt activeQuery in ClientQueryViewController to prevent QueryFileListTableViewController from controlling the customSearchQuery / run into conflicts - resolve issue where a "Stopped" status was displayed sometimes when returning from a revealed search result
- add date parsing for strings following [year]-[[month][-[day]]] syntax - clarify all other method names - NSString+ByteCountParser: parse strings into byte counts, support TB, TiB, GB, GiB, MB, MiB, B and bytes-without-B - OCQueryCondition+SearchSegmenter - add support for localized keywords in local search - clean up keywords, removing kind-of-duplicates - add new keywords: - after: return items last modified after the given date - before: return items last modified before the given date - on: return items last modified on the given date - smaller: return items less than this size - greater: return items greater than this size
felix-schwarz
changed the title
|
@felix-schwarz at first: thank you for implementing this helpful feature! What do you think? |
|
Not sure if the UI refinement should be a part of a new issue. Here some ideas:
|
- address Xcode / linter warnings
|
@hosy Thanks for the ideas and feedback. I like them a lot! My plan is to implement search in two steps:
I think showing help is a good idea to make these features accessible, but I'd move that to step 2, where it's easier to determine what's still needed after f.ex. adding suggestions. One of the "suggestions" could, btw, also be a "link" that opens an "advanced search" UI, which provides a UI for the supported keywords, composing the keywords for them in the end. |
ownCloudAppFramework/SDK Extensions/OCQueryCondition+SearchSegmenter.m
Outdated
Show resolved
Hide resolved
ownCloudAppFramework/SDK Extensions/OCQueryCondition+SearchSegmenter.m
Outdated
Show resolved
Hide resolved
ownCloudAppFramework/SDK Extensions/OCQueryCondition+SearchSegmenter.m
Outdated
Show resolved
Hide resolved
ownCloudAppShared/Client/File Lists/ClientQueryViewController.swift
Outdated
Show resolved
Hide resolved
ownCloudAppShared/Client/File Lists/QueryFileListTableViewController.swift
Outdated
Show resolved
Hide resolved
| @@ -56,7 +88,7 @@ public class SortBar: UIView, Themeable, UIPopoverPresentationControllerDelegate | |||
| } | |||
|
|
|||
| // MARK: - Constants | |||
| let sideButtonsSize: CGSize = CGSize(width: 44.0, height: 44.0) | |||
| let sideButtonsSize: CGSize = CGSize(width: 22.0, height: 22.0) | |||
There was a problem hiding this comment.
I thought we set this to 44.0 before, because it was not so good to tap, because of the small size before. Please also have a look, if it is aligned with the … of the more button in the file list cell. Before the UI looked a little bit "jumpy".
There was a problem hiding this comment.
That was also my worry initially, but it didn't seem to be an issue in tests when I tried. I chose a smaller size so that the "Reveal" arrow also fits in without excessive white space between the "more dots" and the "reveal" arrow.
In my testing, the dots still aligned well between navigation bar and table view rows:
| More | More + Reveal |
|---|---|
 |
 |
There was a problem hiding this comment.
The problem is on a real device. On simulator everything works fine, but on the iPhone it is difficult to touch.
|
|
||
| for scope in SearchScope.allCases { | ||
| if let image = scope.image { | ||
| searchScopeSegmentedControl.insertSegment(with: image, at: scope.rawValue, animated: false) |
There was a problem hiding this comment.
Does it make more sense to only show the titles? Your chosen images are great, but it looks a little bit to large for the small segmented control UI element. But needs to check, if the width fits for all UI elements in this row on a small iPhone display.
Can you try a build with titles only please?
There was a problem hiding this comment.
I did initially try with titles, but ran into space problems and had issues finding a short word that explains the purpose as well as the icons. "all" fits, but "everywhere" or "all folders" would be better. Same with "folder" vs. "this folder". But then the UI no longer fits.
If you feel the icons are too large, we could also try slightly scaled down versions. What do you think?
There was a problem hiding this comment.
I just tested the label solution on a small screen size (iPhone 6) and it fits. I also discovered, that the Multiselect button needs to be hidden, when search is active, because the UI elements are hidden in this mode. This also give us more space for the search scope segmented control.
Maybe it makes sense to use the label Account instead of All for the global search scope.
If you agree, I can push my changes, but I'm also interested in your thoughts.
Here you can see the result:
There was a problem hiding this comment.
Account and Folder are good choices for labels IMO.
My worry, however, is with iOS 12 on the smallest supported devices (iPhone 5s). Even in English, it's already getting really tight, but in German there are overlaps (notice the / next to the drop down label):
| English | German |
|---|---|
 |
 |
There was a problem hiding this comment.
I also just noticed that we don't change the placeholder text when switching between account and folder scope (always "Search this folder"). Will fix that.
There was a problem hiding this comment.
On the other hand, we can't use the SF Symbols on iOS 12, so would either need to provide our own icons - or drop back to text there. Taking into account that iOS 12 + iPhone 5s should be a really small cohort, the only issue for these is purely cosmetic in nature, and that text communicates the purpose a lot clearer, I came to the conclusion that we should indeed use text here and merged the changes from your branch.
Also fixed the placeholder text.
- add full localization + normalization of keywords - add English and German localization
- remove leftover code
(9) Multiselect possible after search in directory picker [FIXED]When searching in the directory picker (f.ex. during a Move action) and exiting from search, the otherwise unavailable multi select button appears and can be used. |
…ect should be available, defaulting to true - QueryFileListTableViewController, ClientQueryViewController: allow efficient subclassing of relevant parts of the reveal feature - ClientDirectoryPickerViewController: provide implementations of relevant reveal subclassing points, disable multi-select (fixing finding (7) and (9) in #933)
|
Thanks for your help with the QA ;) i keep checking |
|
(1), (2), (4), (6), (7), (9) -> fixed (3) -> not fixed. Sorting options are still alligned to the left. (5) -> won't fix here 1st QA rond complete, will do a second round with iPad (including the (8) check) |
- search segmenter: add support for owner keyword, incl. localization
@michaelstingl @jesmrec: As of 399ed6d, |
|
@jesmrec Re (3): please share a screenshot and mark what part exactly is misplaced. I thought it'd be about the "name | kind | size | …" part of the Sort Bar. |
|
about (3): the sorting bar Then, not a bad behaviour or a blocker, but a difference that can be intended or not (this is my question)
|
|
Checking (8). New shortcuts are correct, but i miss one: |
|
OTOH, the maintenance error is not prevented correctly when an external storage is not accesible (showing cached content) |
changed keyboard command for toggle search order and share item
|
@jesmrec @felix-schwarz regarding (3): regarding (8): |
fixed!!
it is not a problem for me to keep the bar in the left-side. In terms of quality, it does not mean any loss. This is more UX-issue than quality issue. |
|
@jesmrec I think we should try the left aligned version. For me it fits into the current UI design. Most UI elements are left aligned and right aligned. |
|
Ok, let's give a try. As i commented, i only reported this because is a non-reported difference with the base branch. Then, QA is finished (test plan in first message) and no reports are pending |
Description
Local account-wide search using custom queries. An arrow is added to the right of the search results to jump directly to the folder containing the results, scrolls the item into view and highlights it.
Local search segments the input into terms and filter keywords. Some examples:
engineeringengineeringin their nameengineering demosengineeringanddemosin their name"engineering demos"engineering demosin their name"engineering "demosengineeringanddemosin their nametype:pdftype:pdf,movbefore:2021before:2021-02before:2021-02-03after:2020after:2020-02after:2020-02-03on:2020-02-03on:2020-02-03,2020-02-05smaller:200mbgreater:1gbowner:cscherm:file:folder:imageimage/):videovideo/):year:month:week:todayor:d:5d:2w:1m:m:1y:y-[term]-:imageto exclude images, or-wrdto exclude all matches containingwrd. This works with any of the examples above.Other, related changes in this pull request:
Other, unrelated changes in this pull request:
displayNameanduserNamehave at least one character before using them to buildOCBookmark.shortNamefor displaying in the server listTo Do
a database migration screen at app launch (migrating all accounts' databases one by one)-> [FEATURE REQUEST] Database migration of all accounts at launch #938Related Issues
Screenshots (if appropriate):
Types of changes
QA
Test Plan: https://github.com/owncloud/QA/blob/master/Mobile/iOS-app/Version%2011.6/Local%20search.md
Reports:
typenot working [feature/localsearch] Local account-wide search using custom queries #933 (comment) [FIXED]:day[feature/localsearch] Local account-wide search using custom queries #933 (comment) [FIXED]