-
Notifications
You must be signed in to change notification settings - Fork 4
Fixed file drop component to accept multiple files when dragging and dropping #92
Conversation
Codecov Report
@@ Coverage Diff @@
## master #92 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 25 25
Lines 935 939 +4
Branches 131 132 +1
=====================================
+ Hits 935 939 +4
Continue to review full report at Codecov.
|
/** | ||
* Returns `true` if a directory is found in the provided `files` parameter. | ||
*/ | ||
public hasFolder(files: any): boolean { |
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.
☝️ Two things happening here:
- Renamed the method to
hasFolder
to be more transparent about its purpose. - Removed the
files.length > 1
check because you might want multiple files depending on what context this is called in. Removing that line also made this method have one single responsibility.
src/app/public/modules/file-attachment/file-attachment.service.ts
Outdated
Show resolved
Hide resolved
@@ -130,9 +130,15 @@ export class SkyFileDropComponent { | |||
this.acceptedOver = false; | |||
|
|||
if (dropEvent.dataTransfer && dropEvent.dataTransfer.files) { | |||
if (this.fileAttachmentService.verifyDropFiles(dropEvent.dataTransfer.files)) { | |||
this.handleFiles(dropEvent.dataTransfer.files); | |||
if (!this.multiple && dropEvent.dataTransfer.files.length > 1) { |
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.
Do you mind setting these statements' values to well-named variables so that their purposes are easy to read going forward? If you don't like that, maybe some helpful comments above each if
check?
const isCheckingWhateverThisIsChecking = (!this.multiple && dropEvent.dataTransfer.files.length > 1);
const hasDirectory = this.fileAttachmentService.hasFolder(dropEvent.dataTransfer.files);
if (isCheckingWhateverThisIsChecking || hasDirectory) {
return;
}
this.handleFiles(dropEvent.dataTransfer.files);
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.
Good idea. Done.
Addresses #91