This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-8428-ebc65b54bc0cbbf6a74b4fc1f8c811488fe0f554 in repository https://gitbox.apache.org/repos/asf/texera.git
commit dcb25efa9e1d4321b7b99459db4ee777f37cbaff Author: Tanishq Gandhi <[email protected]> AuthorDate: Fri Sep 4 19:48:04 2026 +0000 feat(frontend): add a folder option to the upload panel's file picker (#8428) ### What changes were proposed in this PR? The upload panel on a dataset's or model's **Versions & Files** tab said *"Drag & drop file/folder to upload"*, and dragging a folder did work — but the button beside it could only pick files. Uploading a folder was possible only by dragging one in, which rules out a maximised window, a laptop with nothing to drag from, and keyboard use. The panel now offers **Upload Files** and **Upload Folder**. Two buttons rather than one, because a file input cannot do both: a folder can only be chosen when the input carries `webkitdirectory`, and that attribute *switches the dialog's mode* rather than widening it — with it the OS dialog picks only directories, without it only files. Drag & drop can accept either because it is a different API: a drop carries entries that report `isFile`/`isDirectory`, while an input hands back a flat `FileList`. Google Drive, Dropbox and OneDrive all split the same choice in two. The panel keeps one hidden input per mode and both buttons feed the same handler. `fileDropped`'s tail — size limits, conflict resolution, existing-file skipping, the banner — is now a shared `addSelection`, so both routes behave identically. **Before** — one button, files only: <img width="368" height="167" alt="issue7-1-upload-panel-before" src="https://github.com/user-attachments/assets/a6f0a743-0c2b-4adc-bb35-88be8fb05db9" /> **After** — the folder route is reachable without dragging: <img width="368" height="167" alt="issue7-1-upload-panel-after" src="https://github.com/user-attachments/assets/eee6a5be-0368-46dc-ac0b-42e8abf2fad5" /> https://github.com/user-attachments/assets/93fe4a26-1a1d-45d0-92ca-27febdd75dd2 ### Any related issues, documentation, discussions? Closes #8387. ### How was this PR tested? `files-uploader.component.spec.ts` and `version-uploader.component.spec.ts`, 108 passed. Four new cases: - one picker per mode, asserting only the folder input carries `webkitdirectory` - each button opens its own picker, and not the other - **a picked folder keeps its structure** — `readings/2026/a.csv` rather than `a.csv` - a picked loose file uses its bare name, which is what it carries One existing case, `opens the file selector from the drop-zone button`, asserted the button clicked `ngx-file-drop`'s hidden input. That input is no longer the picker, so it is replaced by the two cases above. ``` cd frontend npx ng test --include src/app/dashboard/component/user/files-uploader/files-uploader.component.spec.ts \ --include src/app/dashboard/component/user/version-uploader/version-uploader.component.spec.ts ``` Checked end-to-end against a local stack by picking a real folder through the new button: ``` readings/ ├── notes.md └── 2026/{a.csv,b.csv} staged -> readings/notes.md, readings/2026/a.csv, readings/2026/b.csv ``` Full relative paths, nested folder included — identical to dropping the same folder in. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) --- .../files-uploader/files-uploader.component.html | 40 +++++++-- .../files-uploader/files-uploader.component.scss | 7 ++ .../files-uploader.component.spec.ts | 99 +++++++++++++++++++--- .../files-uploader/files-uploader.component.ts | 34 ++++++++ 4 files changed, 160 insertions(+), 20 deletions(-) diff --git a/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.html b/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.html index e990d536ec..eecd66ad05 100644 --- a/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.html +++ b/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.html @@ -32,20 +32,42 @@ (onFileDrop)="fileDropped($event)"> <ng-template class="ngx-drop-box" - ngx-file-drop-content-tmp - let-openFileSelector="openFileSelector"> + ngx-file-drop-content-tmp> <div class="file-drop-description"> <p>Drag & drop file/folder to upload</p> <!-- <p class="file-drop-hint">Consider zipping large directories for faster uploads</p>--> <p>or</p> - <button - nz-button - nzType="primary" - class="upload-file-button" - (click)="openFileSelector()"> - Browser & Upload Files - </button> + <div class="upload-buttons"> + <button + nz-button + nzType="primary" + class="upload-file-button" + (click)="filePicker.click()"> + Upload Files + </button> + <button + nz-button + nzType="primary" + class="upload-file-button" + (click)="folderPicker.click()"> + Upload Folder + </button> + </div> </div> </ng-template> </ngx-file-drop> + <!-- One input per mode: webkitdirectory makes an input folder-only, so it cannot also offer files. --> + <input + #filePicker + type="file" + multiple + hidden + (change)="filesPicked($event)" /> + <input + #folderPicker + type="file" + multiple + webkitdirectory + hidden + (change)="filesPicked($event)" /> </div> diff --git a/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.scss b/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.scss index 38e4e171f6..8d9375f4ff 100644 --- a/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.scss +++ b/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.scss @@ -63,6 +63,13 @@ overflow-x: auto; } +.upload-buttons { + display: flex; + gap: 8px; + justify-content: center; + flex-wrap: wrap; +} + .upload-file-button { padding: 5px 10px; /* Smaller padding to make the button thinner */ font-size: 0.7em; /* Optionally reduce font size */ diff --git a/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.spec.ts b/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.spec.ts index f895435475..3aae8e61a6 100644 --- a/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.spec.ts @@ -56,6 +56,21 @@ const waitUntil = async (condition: () => boolean): Promise<void> => { throw new Error("condition was not met"); }; +/** A picked file, with the relative path the browser sets when a folder was chosen. */ +const pickedFile = (name: string, relativePath = ""): File => { + const file = new File(["x"], name); + Object.defineProperty(file, "webkitRelativePath", { value: relativePath }); + return file; +}; + +/** Fires a change event carrying `files`, as the picker does. */ +const pick = (component: FilesUploaderComponent, files: File[]): void => { + const input = document.createElement("input"); + input.type = "file"; + Object.defineProperty(input, "files", { value: files, writable: true }); + component.filesPicked({ target: input } as unknown as Event); +}; + const droppedFile = (relativePath: string, file: File): NgxFileDropEntry => ({ relativePath, @@ -285,6 +300,25 @@ describe("FilesUploaderComponent", () => { expect(modals).toHaveLength(1); }); + it("reconciles a picked folder's files by their full path, not their bare name", async () => { + // Two files share a name and differ only by folder, so a bare-name lookup would confuse them. + uploadService.listMultipartUploads.mockReturnValue(of([])); + uploadService.findExistingUploadFiles.mockReturnValue(of([])); + const emitted = new Promise<FileUploadItem[]>(resolve => component.uploadedFiles.subscribe(resolve)); + + pick(component, [pickedFile("jan.csv", "telemetry/2026/jan.csv"), pickedFile("jan.csv", "telemetry/2025/jan.csv")]); + + expect((await emitted).map(item => item.name)).toEqual(["telemetry/2026/jan.csv", "telemetry/2025/jan.csv"]); + expect(uploadService.findExistingUploadFiles).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + expect.arrayContaining([ + expect.objectContaining({ path: "telemetry/2026/jan.csv" }), + expect.objectContaining({ path: "telemetry/2025/jan.csv" }), + ]) + ); + }); + it("passes a non-conflicting file straight through without prompting", async () => { uploadService.listMultipartUploads.mockReturnValue(of(["other.csv"])); uploadService.findExistingUploadFiles.mockReturnValue(of([])); @@ -644,20 +678,63 @@ describe("FilesUploaderComponent rendered", () => { expect(alert()).toBeNull(); }); - it("opens the file selector from the drop-zone button", () => { - // ngx-file-drop hands its `openFileSelector` to the content template by reference, - // so spying on the component's property after render would not be seen. Assert its - // effect instead: it clicks the hidden file input. + it("keeps a picked folder's structure, the same as dropping it", async () => { + const emitted = new Promise<FileUploadItem[]>(resolve => component.uploadedFiles.subscribe(resolve)); + + pick(component, [pickedFile("a.csv", "readings/2026/a.csv"), pickedFile("b.csv", "readings/2026/b.csv")]); + + // The path, not the bare name: the backend lays the version out from these. + expect((await emitted).map(item => item.name)).toEqual(["readings/2026/a.csv", "readings/2026/b.csv"]); + }); + + it("does nothing when the picker is dismissed without choosing", async () => { + const emitted = new Promise<FileUploadItem[]>(resolve => component.uploadedFiles.subscribe(resolve)); + + pick(component, []); + + expect(await emitted).toEqual([]); + }); + + it("uses the bare name for a picked file, which carries no relative path", async () => { + const emitted = new Promise<FileUploadItem[]>(resolve => component.uploadedFiles.subscribe(resolve)); + + pick(component, [pickedFile("loose.csv")]); + + expect((await emitted).map(item => item.name)).toEqual(["loose.csv"]); + }); + + /** The panel's own pickers, in template order: files first, then folder. */ + const pickers = (): HTMLInputElement[] => { const host = fixture.nativeElement as HTMLElement; - const fileInput: HTMLInputElement = host.querySelector("input.ngx-file-drop__file-input")!; - expect(fileInput).not.toBeNull(); - const openDialog = vi.spyOn(fileInput, "click").mockImplementation(() => {}); + return Array.from(host.querySelectorAll<HTMLInputElement>("input[type=file]")).filter( + input => !input.classList.contains("ngx-file-drop__file-input") + ); + }; + + it("offers one picker per mode, since webkitdirectory makes an input folder-only", () => { + const [filePicker, folderPicker] = pickers(); + + expect(filePicker.hasAttribute("webkitdirectory")).toBe(false); + expect(folderPicker.hasAttribute("webkitdirectory")).toBe(true); + expect(filePicker.multiple).toBe(true); + expect(folderPicker.multiple).toBe(true); + }); + + it("opens the matching picker from each button", () => { + const [filePicker, folderPicker] = pickers(); + const openFiles = vi.spyOn(filePicker, "click").mockImplementation(() => {}); + const openFolder = vi.spyOn(folderPicker, "click").mockImplementation(() => {}); + const host = fixture.nativeElement as HTMLElement; + const buttons = Array.from(host.querySelectorAll<HTMLButtonElement>(".upload-file-button")); + + expect(buttons.map(b => (b.textContent ?? "").trim())).toEqual(["Upload Files", "Upload Folder"]); - const button: HTMLButtonElement = host.querySelector(".upload-file-button")!; - expect(button).not.toBeNull(); - button.click(); + buttons[0].click(); + expect(openFiles).toHaveBeenCalled(); + expect(openFolder).not.toHaveBeenCalled(); - expect(openDialog).toHaveBeenCalled(); + buttons[1].click(); + expect(openFolder).toHaveBeenCalled(); }); it("routes a drop on the zone into fileDropped", () => { diff --git a/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.ts b/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.ts index f7d81b30a8..e21a7c4eec 100644 --- a/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.ts +++ b/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.ts @@ -320,6 +320,40 @@ export class FilesUploaderComponent implements OnInit { }); }); + this.addSelection(filePromises); + } + + /** + * Files chosen from the picker. A folder arrives flattened, each file carrying + * webkitRelativePath, which is the structure the drop path gets from the entry tree. + */ + public filesPicked(event: Event): void { + const input = event.target as HTMLInputElement; + const picked = Array.from(input.files ?? []); + // Cleared so choosing the same folder twice running still fires a change event. + input.value = ""; + this.addSelection(picked.map(file => this.toUploadItem(file))); + } + + /** Rejects an oversized file the way the drop path does, so both routes report it alike. */ + private toUploadItem(file: File): Promise<FileUploadItem | null> { + if (file.size > this.singleFileUploadMaxSizeMiB * 1024 * 1024) { + this.notificationService.error( + `File ${file.name}'s size exceeds the maximum limit of ${this.singleFileUploadMaxSizeMiB}MiB.` + ); + return Promise.reject(null); + } + return Promise.resolve({ + file, + name: file.webkitRelativePath || file.name, + description: "", + uploadProgress: 0, + isUploadingFlag: false, + restart: false, + }); + } + + private addSelection(filePromises: Promise<FileUploadItem | null>[]): void { Promise.allSettled(filePromises) .then(async results => { const { ownerEmail, resourceName } = this.getOwnerAndName();
