Feature/move #3739
Feature/move #3739
Conversation
| }, | ||
|
|
||
| moveResources() { | ||
| const resources = JSON.parse(JSON.stringify(this.selectedFiles)) |
There was a problem hiding this comment.
strange gymnastics ? is that a trick to do a deep copy ?
would it make sense to only gather the file ids ?
There was a problem hiding this comment.
This copies only the values without the watchers, etc. related to the vuex state. By doing this we are safe that we don't get any errors later on in case of modifying the values + we do not modify the original state. It looks a bit odd but I haven't yet found in any vue issue/forum different way to do this. Well, I saw one with lodash but I'd prefer to avoid that 🤷
| }, | ||
|
|
||
| target() { | ||
| return JSON.parse(JSON.stringify(this.$route.query.target)) |
There was a problem hiding this comment.
See comment above
There was a problem hiding this comment.
should we maybe create our own utility function then ? the name of the function will already be explicit enough so we know what and why this is used
apps/files/src/components/LocationPicker/MoveSidebarMainContent.vue
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Found one bug:
- start a move operation on a resource within
Home(doesn't happen on subfolders) - cancel the location picker
- sidebar is empty, except for the ownCloud logo.
Menu items re-appear on page reload or when navigating into a subfolder.
The javascript console shows this:
In general, this is really nice quality work! 🚀
| desc: this.$gettext('Multiple resources could not be moved'), | ||
| status: 'danger' | ||
| }) | ||
| console.error('Move failed:', errors) |
There was a problem hiding this comment.
It's left on purpose. Until we have added the logic to notifications to "stack" and then display all of the resources that failed, this is a "workaround" to be able to still see all the errors separately.
| :key="item.viewId" | ||
| :item="item" | ||
| :name="$_ocTrashbin_fileName(item)" | ||
| :display-preview="false" |
There was a problem hiding this comment.
Why hide the thumbnails within the trash bin? 🤔
There was a problem hiding this comment.
The thumbnails are not yet being displayed in trashbin at all and I do not think implementing that should be part of this PR so disabling them until it's done is good enough here IMO.
Instead of two separate components for move and copy, we can use on location picker component and pass the action as a query
|
Seems that the breadcrumbs step is failing on mobile resolution. I'll investigate |
|
Found the issue with breadcrumbs. The tests found them twice on mobile resolution which resulted in error. Until we fix the breadcrumbs in ODS, I adjusted it to look for the mobile ones only if the "normal" ones are not found. |
There was a problem hiding this comment.
Nice!
I found one tiny issue, but that might be unrelated to this PR:
-
During move, navigate into a folder structure like this
-
Navigate back by clicking on the folder name with a blank. The blank converts to
%20after navigating to the folder. Only happens in the title breadcrumb, not in the regular breadcrumbs we have above the files list.
Description
We've added move action to the files list. It is possible now to move folders and files.
The move is executed via a new page called location picker.
Related Issue
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes