-
-
Notifications
You must be signed in to change notification settings - Fork 77
Add config to select source directories #2319
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
Conversation
5597abb to
6c75718
Compare
7d742cd to
1de2b1f
Compare
nimishavijay
left a comment
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.
Looking great! :) Only really small comments:
- For the media folders: "Choose a different folder" --> "Add folder" (updated the mockup after you started working on the PR, my bad)
- how would people be able to remove folders? (eg. would the file picker component allow multiselect, so people can unselect folders?)
- Unsure about the wording "Root folder" we could maybe change the icon to a home icon or change the wording to something else, any suggestions @nextcloud/designers? :)
As discussed, the "root" folder should be "Home" and use the home house icon that we also use in the breadcrumbs in Files. |
As stated, this PR does not support the selection of multiple source folders for now. The day we can search in multiple folders, I will follow your above recommendations :).
Updated ! |
1de2b1f to
042082c
Compare
nimishavijay
left a comment
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.
Sounds good! In that case, only change "Media folders" to "Media folder" (folder is singular, not plural :) ) approving to not block! :)
042082c to
4a6986c
Compare
| subname() { | ||
| const slashesCount = (this.path.match(/\//g) ?? []).length | ||
|
|
||
| switch (slashesCount) { | ||
| case 1: | ||
| return '' | ||
| case 2: | ||
| return this.path.split('/').splice(0, 2).join('/') | ||
| default: | ||
| return this.path.split('/').splice(0, 3).join('/') | ||
| } |
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.
What is this sorcery? 😁
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.
To properly display the path of the folder. Let me add a comment.
| Full path | Displayed path |
|---|---|
| / | Home |
| /a | nothing |
| /a/b | /a |
| /a/b/c | /a/b |
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.
More comments the better :)
Signed-off-by: Louis Chemineau <[email protected]>
Signed-off-by: Louis Chemineau <[email protected]>
Signed-off-by: Louis Chemineau <[email protected]>
Signed-off-by: Louis Chemineau <[email protected]>
Signed-off-by: Louis Chemineau <[email protected]>
Signed-off-by: Louis Chemineau <[email protected]>
Signed-off-by: Louis Chemineau <[email protected]>
Signed-off-by: Louis Chemineau <[email protected]>
Signed-off-by: Louis Chemineau <[email protected]>
Signed-off-by: Louis Chemineau <[email protected]>
Signed-off-by: Louis Chemineau <[email protected]>
Signed-off-by: Louis Chemineau <[email protected]>
|
Added some comments and a better error handling when the folder does not exist. |
Signed-off-by: Louis Chemineau <[email protected]>
bd9b7a2 to
b110ae7
Compare
|
🎉🎉🎉 |
Fix #141
TODO
Changes
photosSourceFolderconfig, defaulting to/PhotosUserConfigmixin into a Vuex store and remove the usage of the mixin everywhere.photosSourceFolderoption in the photos search requestphotosSourceFolderphotosLocationto match design mockupsphotosSourceFoldergets updatedphotosLocationFolderwhenphotosLocationis updatedLimitations
Screenshots