Skip to content
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: allow dropping a folder into the dropzone #1066

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Prev Previous commit
Next Next commit
implement code readability suggestions
  • Loading branch information
Glenn Van De Putte committed Feb 29, 2024
commit 3ef7bfd667623232fc66fcfca0a1248741062bf4
117 changes: 57 additions & 60 deletions ember-file-upload/src/system/data-transfer-wrapper.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,63 @@
import type { FileUploadDragEvent } from '../interfaces.ts';

const getDataSupport = {};

interface FutureProofDataTransferItem extends DataTransferItem {
getAsEntry?: () => FileSystemDirectoryEntry;
getAsEntry?: () => FileSystemDirectoryEntry | null;
}

const getDataSupport = {};

const readEntry = (entry: FileSystemEntry): Promise<File> => {
return new Promise((resolve, reject) => {
if (entry.isFile) {
(entry as FileSystemFileEntry).file((fileEntry: File) => {
resolve(fileEntry);
});
} else {
reject('Directory contains nested directories');
}
});
};

const getEntry = (
item: FutureProofDataTransferItem,
): FileSystemDirectoryEntry => {
// In the future this method name might change, so already implementing it like this if needed
// https://developer.mozilla.org/en-US/docs/Web/API/DataTransferItem/webkitGetAsEntry
return (
item.getAsEntry?.() ?? (item.webkitGetAsEntry() as FileSystemDirectoryEntry)
);
};

const readAllFilesInDirectory = (item: DataTransferItem): Promise<File[]> =>
new Promise((resolve, reject) => {
const entry = getEntry(item);
if (!entry) {
reject('Could not read directory');
}

entry?.createReader()?.readEntries(async (entries: FileSystemEntry[]) => {
const readFiles: File[] = await Promise.all(entries.map(readEntry)).catch(
(err) => {
throw err;
},
);
resolve(readFiles.filter(Boolean) as File[]);
});
});

const readDataTransferItem = async (
item: DataTransferItem,
): Promise<File[]> => {
if (getEntry(item)?.isDirectory) {
const directoryFile = item.getAsFile() as File;
const filesInDirectory: File[] = await readAllFilesInDirectory(item);
return [directoryFile, ...filesInDirectory];
} else {
const fileItem = item.getAsFile() as File;
return [fileItem];
}
};

export default class DataTransferWrapper {
dataTransfer?: DataTransfer;
itemDetails?: FileUploadDragEvent['itemDetails'];
Expand Down Expand Up @@ -48,59 +100,6 @@ export default class DataTransferWrapper {
}

async getFilesAndDirectories() {
Copy link
Collaborator

@gilest gilest Feb 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Perf] All of the child functions defined within getFilesAndDirectories do not rely on class state or local variable closure.

Because of this, they only need to be defined once in memory, rather than every time this methodn is called.

Let's move all of the child functions to module scope (you can put them in the top of this file).

I think extracting them also helps with readability of this feature.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved all the functions to the top.

const files: File[] = [];

const readEntry = async (entry: FileSystemEntry): Promise<File> => {
return new Promise((resolve, reject) => {
if (entry.isFile) {
(entry as FileSystemFileEntry).file((fileEntry: File) => {
resolve(fileEntry);
});
} else {
reject('Directory contains nested directories');
}
});
};

const getEntry = (
item: FutureProofDataTransferItem,
): FileSystemDirectoryEntry => {
// In the future this method name might change, so already implementing it like this if needed
// https://developer.mozilla.org/en-US/docs/Web/API/DataTransferItem/webkitGetAsEntry
if (typeof item.getAsEntry === 'function') {
return item.getAsEntry();
}

return item.webkitGetAsEntry() as FileSystemDirectoryEntry;
};

const readAllFilesInDirectory = (item: DataTransferItem): Promise<File[]> =>
new Promise((resolve) => {
getEntry(item)
?.createReader()
?.readEntries(async (entries: FileSystemEntry[]) => {
const readFiles: File[] = await Promise.all(
entries.map(readEntry),
).catch((err) => {
throw err;
});
resolve(readFiles.filter((file) => file !== undefined) as File[]);
});
});

const readDataTransferItem = async (
item: DataTransferItem,
): Promise<File[]> => {
if (getEntry(item)?.isDirectory) {
const directoryFile = item.getAsFile() as File;
const filesInDirectory: File[] = await readAllFilesInDirectory(item);
return [directoryFile, ...filesInDirectory];
} else {
const fileItem = item.getAsFile() as File;
return [fileItem];
}
};

if (this.dataTransfer?.items) {
const allFilesInDataTransferItems: File[][] = await Promise.all(
Array.from(this.dataTransfer?.items).map(readDataTransferItem),
Expand All @@ -113,13 +112,11 @@ export default class DataTransferWrapper {
[],
);

files.push(...flattenedFileArray);
return flattenedFileArray;
} else {
const droppedFiles: File[] = Array.from(this.dataTransfer?.files ?? []);
files.push(...droppedFiles);
return droppedFiles;
}

return files;
}

get files() {
Expand Down
4 changes: 2 additions & 2 deletions ember-file-upload/src/test-support.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,12 @@ interface FileSystemEntryStub {
}

export async function dragAndDropDirectory(
selector: string,
target: string | HTMLElement,
folderName: string,
filesInDirectory: (File | Blob)[],
...singleFiles: (File | Blob)[]
) {
const dropzone = find(selector);
const dropzone = target instanceof HTMLElement ? target : find(target);
assert(`Selector '${dropzone}' could not be found.`, dropzone);
assert(
'All files must be instances of File/Blob type',
Expand Down
Loading