-
Notifications
You must be signed in to change notification settings - Fork 10
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
feat(copy): drag-drop shift modifier to move/copy a document #1079
base: main
Are you sure you want to change the base?
Conversation
addon/components/document-view.js
Outdated
get canDropCopy() { | ||
return Boolean( | ||
this.dragAction === "copy" && | ||
this.args.filters && | ||
this.args.filters.categories, | ||
); | ||
} | ||
|
||
get canDropUpload() { | ||
return Boolean( | ||
this.dragAction === "upload" && | ||
this.args.filters && | ||
this.args.filters.categories, | ||
); | ||
} |
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.
Why do you need to distinguish these two? Couldn't you just do something like this?
get canDropCopy() { | |
return Boolean( | |
this.dragAction === "copy" && | |
this.args.filters && | |
this.args.filters.categories, | |
); | |
} | |
get canDropUpload() { | |
return Boolean( | |
this.dragAction === "upload" && | |
this.args.filters && | |
this.args.filters.categories, | |
); | |
} | |
get canDrop() { | |
return Boolean( | |
["copy", "upload"].includes(this.dragAction) && | |
this.args.filters && | |
this.args.filters.categories, | |
); | |
} |
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.
For the component purpose it could be simplified but in the template I'm depending on separate methods to differentiate between two different translations. See document-view.hbs
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.
I think differentiating by this.dragAction
in the template would be the way to go for this.
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.
Changed
333562d
to
118cffb
Compare
@anehx rebased and added some fixes and comments |
addon/components/document-view.hbs
Outdated
@@ -7,8 +7,7 @@ | |||
> | |||
<div class="uk-flex uk-flex-middle uk-padding-small"> | |||
<DocumentUploadButton | |||
@categoryId={{@filters.categories}} | |||
@afterUpload={{this.refreshDocumentList}} | |||
@categoryId={{@filters.categories}} @afterUpload={{this.refreshDocumentList}} |
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.
Let's revert this formatting change.
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.
Reverted
addon/components/document-view.js
Outdated
get canDropCopy() { | ||
return Boolean( | ||
this.dragAction === "copy" && | ||
this.args.filters && | ||
this.args.filters.categories, | ||
); | ||
} | ||
|
||
get canDropUpload() { | ||
return Boolean( | ||
this.dragAction === "upload" && | ||
this.args.filters && | ||
this.args.filters.categories, | ||
); | ||
} |
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.
I think differentiating by this.dragAction
in the template would be the way to go for this.
118cffb
to
d385a1b
Compare
Adding a new feature to allow copying documents together with the underlying files.
While holding shift you can either drag the document(s) onto the document view (to copy within the same category), or onto a category by dropping it onto the category navigation on the left.
This functionality will rely on projectcaluma/alexandria#757