[feature/last-picked] Add Recent Locations to location picker
Description
Adds a row of recently picked locations to the Location Picker for quick access.
Screenshots (if appropriate):
| Within same account | Across accounts (media uploads) | Share Extension |
|---|---|---|
Types of changes
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
QA
Test plan:
Reports:
- [X] (1) Move operation shows entries from other spaces https://github.com/owncloud/ios-app/pull/1467#issuecomment-2955428253 [FIXED]
- [X] (2) Removed folder is retained in the list https://github.com/owncloud/ios-app/pull/1467#issuecomment-2955460826 [WONT FIX]
- [ ] (3) No padding in cell https://github.com/owncloud/ios-app/pull/1467#issuecomment-2955692861
- [ ] (4) Cell width expands a lot if display name is longer than usual https://github.com/owncloud/ios-app/pull/1467#issuecomment-2955704542
- [X] (5) Icons in cells https://github.com/owncloud/ios-app/pull/1467#issuecomment-2955734100 [WONT FIX]
Let's QA this one...
(1) [FIXED]
- Copy an item in
Personalto another space in the same account (location saved) - Select
Moveof any item inPersonalto display the folder picker
Current:
location saved in the step 1. is displayed
Expected:
Since moving items between spaces is not allowed, the locations pointing to other spaces shouldn't be displayed to avoid the consequent error.
Is that feasible?
iPhoneXR v18.4
741040e7
(2) [WONT FIX]
Following the sense of (1):
If a folder that is in the list is removed from the account, it is still in the list of picked locations. Shouldn't be removed as well?
It is correctly handled with a message Folder removed, this is just as an improvement.
NOTE: same for renamed and moved folders
iPhoneXR v18.4
741040e7
(3) [FIXED]
An small glitch, in case the folder name exceeds one line in the recent location cell:
There is no padding in the bottom side of the cell.
iPhoneXR v18.4
741040e7
(4) [FIXED]
Related with (3)
If the display name is longer than expected, the cell width expands till infinite and beyond:
It should be ellipsized, just like the folder name
iPhoneXR v18.4
741040e7
(5) (not blocker) [WONT FIX]
I'm not sure if this is a good UX for the icon in the cell:
| Root | Non root | |
|---|---|---|
| Personal | ||
| Other space |
I'd set the same icon for any option inside an space (no Personal) and other for Personal.
root/non-root does not seem to be a meaningful differentiation.
What do you think?
iPhoneXR v18.4
741040e7
@jesmrec I fixed (1), (3) and (4) in code:
- unfit locations are not filtered
- the number of lines is limited to 1 - and its width limited to 240 points
Regarding (2): that deleted recent locations remain available is a compromise. I had to pick between A) a simple implementation, storing recent locations in the respective bookmark's KVS, quickly accessible and immediately ready for rendering without much overhead B) integration into the SDK and DB, so they are fully tracked (including renames), but more CPU + resource overhead to retrieve them
I went with A to keep the implementation simple. If it should turn out to be a bad or the wrong compromise, it can still be transitioned to B.
Regarding (5): The icon for the Personal space's root folder should also be a "space" icon. If it shows a folder, that should only happen for the Personal folder on OC10, where a folder is also used elsewhere in the product for that folder.
The logic here is to always show the icon belonging to what's represented:
- the folder "Neuer Ordner" in the "Personal" space is shown with a folder icon and a space icon below it to indicate which space it is located on
- the root folder of the "Personal" space is shown with a space icon, as it represents the space.
(1) -> fixed (2) -> thanks for the clear explanation... let's check how A works, with B as candidate. (3) -> fixed, but pending of (4) (4) not fixed:
(5) -> thanks for the clear explanation
@jesmrec Thanks for the screenshot! Please check if (4) is fixed with the latest commit.
@jesmrec Thanks for the screenshot! Please check if (4) is fixed with the latest commit.
that's much better @felix-schwarz !
i have another suggestion about the fix. It looks like:
Should't it better to elipsize the long user name as the long folder (with the "..." inline)?
@jesmrec Please try again with the latest commit - that should also use an ellipsis for such long names now.
Edit: Forgot to push the change - so if you tried in the last 10 minutes, please retry.
@felix-schwarz tested with
app: 499ed8fd
sdk: 5399f6b7
are they correct? i don't see the fix. The user's display name overflows and is truncated. Just in case, to be in the same page, this is the long name i expected to be ellipsized:
am i missing something?
@jesmrec Upon further testing and debugging I found conflicting layout instructions that could lead to some ambiguity where it sometimes renders with ellipsis and sometimes they way as seen in your screenshot.
I fixed that with the latest commit. Please check again. It should (finally) be fixed now.
Now is fixed!
did a second quick round and no regressions happened.
For me, this is OK to go.