Skip to content

Commit 8dfdda6

Browse files
segunadebayosegunadebayo
committed
fix: file upload
1 parent c340e65 commit 8dfdda6

File tree

4 files changed

+274
-35
lines changed

4 files changed

+274
-35
lines changed

‎.changeset/file-upload-dropzone-children-click.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@zag-js/file-upload": patch
3+
---
4+
5+
Fix issue where clicking on non-interactive children (icons, text) inside the dropzone doesn't open the file picker.

‎e2e/file-upload.e2e.ts‎

Lines changed: 77 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,53 +1,100 @@
11
import { test, expect } from "@playwright/test"
2-
import { a11y, part, controls, testid } from "./_utils"
32
import path from "node:path"
3+
import { FileUploadModel } from "./models/file-upload.model"
44

5-
const trigger = part("trigger")
6-
const input = testid("input")
5+
let I: FileUploadModel
76

87
test.describe("file-upload", () => {
98
test.beforeEach(async ({ page }) => {
10-
await page.goto("/file-upload")
9+
I = new FileUploadModel(page)
10+
await I.goto()
1111
})
1212

1313
// TBD: fix a11y complaints
14-
test.skip("should have no accessibility violations", async ({ page }) => {
15-
await a11y(page)
14+
test.skip("should have no accessibility violations", async () => {
15+
await I.checkAccessibility()
1616
})
1717

18-
test("should be focused when page is tabbed", async ({ page }) => {
19-
const dropzone = part("dropzone")
18+
test("should be focused when page is tabbed", async () => {
19+
await I.page.click("main")
20+
await I.pressKey("Tab")
21+
await I.seeDropzoneIsFocused()
22+
})
23+
24+
test("should open file picker on trigger click", async () => {
25+
const fileChooser = await I.openFilePicker()
26+
expect(fileChooser).toBeDefined()
27+
})
28+
29+
test("should display chosen file", async () => {
30+
const fileChooser = await I.openFilePicker()
31+
await fileChooser.setFiles(path.join(__dirname, "fixtures/text.txt"))
2032

21-
await page.click("main")
22-
await page.keyboard.press("Tab")
33+
await I.seeItem("text.txt")
34+
await I.seeDeleteTrigger()
35+
})
36+
37+
test("should upload file via trigger click", async () => {
38+
const filePath = path.join(__dirname, "fixtures/text.txt")
39+
await I.uploadFile(filePath)
2340

24-
await expect(page.locator(dropzone)).toBeFocused()
41+
await I.seeFileDetails("text.txt")
42+
await I.seeDeleteTrigger()
2543
})
2644

27-
test("should open file picker on trigger click", async ({ page }) => {
28-
const fileChooserPromise = page.waitForEvent("filechooser")
29-
await page.locator(trigger).click()
30-
const prom = await fileChooserPromise
31-
expect(prom).toBeDefined()
45+
test("should upload file via dropzone click", async () => {
46+
const filePath = path.join(__dirname, "fixtures/text.txt")
47+
await I.uploadFileViaDropzone(filePath)
48+
49+
await I.seeFileDetails("text.txt")
50+
await I.seeDeleteTrigger()
3251
})
3352

34-
test("should display chosen file", async ({ page }) => {
35-
const fileChooserPromise = page.waitForEvent("filechooser")
36-
await page.locator(trigger).click()
37-
const fileChooser = await fileChooserPromise
38-
await fileChooser.setFiles(path.join(__dirname, "fixtures/text.txt"))
53+
test("should upload file via drag and drop", async () => {
54+
const filePath = path.join(__dirname, "fixtures/text.txt")
55+
await I.uploadFileViaDragAndDrop(filePath)
56+
57+
await I.seeFileDetails("text.txt")
58+
await I.seeDeleteTrigger()
59+
})
60+
61+
test("should delete uploaded file", async () => {
62+
const filePath = path.join(__dirname, "fixtures/text.txt")
63+
await I.uploadFile(filePath)
64+
65+
await I.seeFileDetails("text.txt")
66+
await I.deleteFile()
67+
await I.seeNoFiles()
68+
})
69+
70+
test("should have disabled attributes when disabled", async () => {
71+
await I.controls.bool("disabled")
72+
await I.seeTriggerIsDisabled()
73+
await I.seeHiddenInputIsDisabled()
74+
})
75+
76+
test("should not reopen file picker after ESC key", async () => {
77+
await I.page.click("main")
78+
await I.pressKey("Tab")
79+
80+
const tracker = I.trackFileChooserEvents()
81+
const fileChooser = await I.openFilePickerWithKeyboard()
82+
expect(fileChooser).toBeDefined()
83+
expect(tracker.count).toBe(1)
3984

40-
const item = page.locator(part("item"))
41-
const deleteTrigger = page.locator(part("item-delete-trigger"))
85+
await I.pressKey("Escape")
86+
await I.waitForNextFrame()
4287

43-
expect(item).toBeVisible()
44-
expect(await item.innerText()).toContain("text.txt")
45-
expect(deleteTrigger).toBeVisible()
88+
expect(tracker.count).toBe(1)
4689
})
4790

48-
test("should have disabled attributes when disabled", async ({ page }) => {
49-
await controls(page).bool("disabled")
50-
await expect(page.locator(trigger)).toBeDisabled()
51-
await expect(page.locator(input)).toBeDisabled()
91+
test("should not open file picker twice when clicking trigger inside dropzone", async () => {
92+
const tracker = I.trackFileChooserEvents()
93+
const fileChooser = await I.openFilePicker()
94+
expect(fileChooser).toBeDefined()
95+
96+
await I.waitForNextFrame()
97+
98+
expect(tracker.count).toBe(1)
5299
})
53100
})

‎e2e/models/file-upload.model.ts‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
import { expect, type Page } from "@playwright/test"
2+
import { part, testid } from "../_utils"
3+
import { Model } from "./model"
4+
5+
const DROPZONE_SELECTOR = part("dropzone")
6+
7+
export class FileUploadModel extends Model {
8+
constructor(public page: Page) {
9+
super(page)
10+
}
11+
12+
goto() {
13+
return this.page.goto("/file-upload")
14+
}
15+
16+
getDropzone() {
17+
return this.page.locator(part("dropzone"))
18+
}
19+
20+
getTrigger() {
21+
return this.page.locator(part("trigger"))
22+
}
23+
24+
getHiddenInput() {
25+
return this.page.locator(testid("input"))
26+
}
27+
28+
getItem() {
29+
return this.page.locator(part("item")).first()
30+
}
31+
32+
getDeleteTrigger() {
33+
return this.page.locator(part("item-delete-trigger")).first()
34+
}
35+
36+
async focusDropzone() {
37+
await this.getDropzone().focus()
38+
}
39+
40+
async focusTrigger() {
41+
await this.getTrigger().focus()
42+
}
43+
44+
async clickTrigger() {
45+
await this.getTrigger().click()
46+
}
47+
48+
async seeDropzoneIsFocused() {
49+
await expect(this.getDropzone()).toBeFocused()
50+
}
51+
52+
async seeItem(fileName: string) {
53+
const item = this.getItem()
54+
await expect(item).toBeVisible()
55+
await expect(item).toContainText(fileName)
56+
}
57+
58+
async seeDeleteTrigger() {
59+
await expect(this.getDeleteTrigger()).toBeVisible()
60+
}
61+
62+
async seeTriggerIsDisabled() {
63+
await expect(this.getTrigger()).toBeDisabled()
64+
}
65+
66+
async seeHiddenInputIsDisabled() {
67+
await expect(this.getHiddenInput()).toBeDisabled()
68+
}
69+
70+
async openFilePicker() {
71+
const fileChooserPromise = this.page.waitForEvent("filechooser")
72+
await this.clickTrigger()
73+
return await fileChooserPromise
74+
}
75+
76+
async openFilePickerWithKeyboard() {
77+
const fileChooserPromise = this.page.waitForEvent("filechooser")
78+
await this.focusTrigger()
79+
await this.pressKey("Enter")
80+
return await fileChooserPromise
81+
}
82+
83+
async waitForNextFrame() {
84+
await this.page.evaluate(() => new Promise((resolve) => requestAnimationFrame(resolve)))
85+
}
86+
87+
trackFileChooserEvents() {
88+
let count = 0
89+
this.page.on("filechooser", () => count++)
90+
return {
91+
get count() {
92+
return count
93+
},
94+
}
95+
}
96+
97+
async uploadFile(filePath: string) {
98+
const fileChooser = await this.openFilePicker()
99+
await fileChooser.setFiles(filePath)
100+
}
101+
102+
async uploadFileViaDropzone(filePath: string) {
103+
const fileChooserPromise = this.page.waitForEvent("filechooser")
104+
await this.getDropzone().click()
105+
const fileChooser = await fileChooserPromise
106+
await fileChooser.setFiles(filePath)
107+
}
108+
109+
async uploadFileViaDragAndDrop(filePath: string) {
110+
// Use Playwright's setInputFiles to set files on the hidden input
111+
// Then simulate drag and drop events
112+
await this.getHiddenInput().setInputFiles(filePath)
113+
114+
// Now simulate drag and drop by creating a DataTransfer from the input's files
115+
await this.page.evaluate(
116+
async ({ dropzoneSelector, inputSelector }) => {
117+
const dropzone = document.querySelector(dropzoneSelector)
118+
const input = document.querySelector(inputSelector) as HTMLInputElement
119+
if (!dropzone || !input || !input.files || input.files.length === 0) {
120+
throw new Error("Dropzone or input not found, or no files")
121+
}
122+
123+
// Create DataTransfer from the input's files
124+
const dataTransfer = new DataTransfer()
125+
for (let i = 0; i < input.files.length; i++) {
126+
dataTransfer.items.add(input.files[i])
127+
}
128+
129+
// Dispatch dragover event
130+
const dragOverEvent = new DragEvent("dragover", {
131+
bubbles: true,
132+
cancelable: true,
133+
dataTransfer,
134+
})
135+
dropzone.dispatchEvent(dragOverEvent)
136+
137+
// Dispatch drop event - this triggers async file processing
138+
const dropEvent = new DragEvent("drop", {
139+
bubbles: true,
140+
cancelable: true,
141+
dataTransfer,
142+
})
143+
dropzone.dispatchEvent(dropEvent)
144+
},
145+
{
146+
dropzoneSelector: DROPZONE_SELECTOR,
147+
inputSelector: testid("input"),
148+
},
149+
)
150+
151+
// Wait for file item to appear (no timeout needed, waitFor has its own timeout)
152+
await this.getItem().waitFor({ state: "visible" })
153+
}
154+
155+
async seeFileDetails(fileName: string, fileSize?: string) {
156+
const item = this.getItem()
157+
await expect(item).toBeVisible()
158+
await expect(item).toContainText(fileName)
159+
if (fileSize) {
160+
await expect(item).toContainText(fileSize)
161+
}
162+
}
163+
164+
async deleteFile() {
165+
await this.getDeleteTrigger().click()
166+
}
167+
168+
async seeNoFiles() {
169+
await expect(this.getItem()).not.toBeVisible()
170+
}
171+
}

‎packages/machines/file-upload/src/file-upload.connect.ts‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,19 @@ import { type NormalizeProps, type PropTypes } from "@zag-js/types"
55
import { flatArray } from "@zag-js/utils"
66
import { parts } from "./file-upload.anatomy"
77
import * as dom from "./file-upload.dom"
8-
import type { FileUploadApi, FileUploadService } from "./file-upload.types"
8+
import type { FileUploadApi, FileUploadService, ItemType } from "./file-upload.types"
99
import { isEventWithFiles } from "./file-upload.utils"
1010

11-
const DEFAULT_ITEM_TYPE = "accepted" as const
11+
const DEFAULT_ITEM_TYPE: ItemType = "accepted"
12+
13+
const INTERACTIVE_SELECTOR =
14+
"button, a[href], input:not([type='file']), select, textarea, [tabindex], [contenteditable]"
15+
16+
function isInteractiveTarget(element: HTMLElement | null, container: HTMLElement): boolean {
17+
if (!element || element.getAttribute("type") === "file") return false
18+
const interactive = element.closest(INTERACTIVE_SELECTOR)
19+
return interactive != container && contains(container, interactive)
20+
}
1221

1322
export function connect<T extends PropTypes>(
1423
service: FileUploadService,
@@ -94,7 +103,11 @@ export function connect<T extends PropTypes>(
94103
onKeyDown(event) {
95104
if (disabled) return
96105
if (event.defaultPrevented) return
97-
if (event.currentTarget !== getEventTarget(event)) return
106+
107+
const target = getEventTarget<HTMLElement>(event)
108+
if (!contains(event.currentTarget, target)) return
109+
if (isInteractiveTarget(target, event.currentTarget)) return
110+
98111
if (props.disableClick) return
99112
if (event.key !== "Enter" && event.key !== " ") return
100113
send({ type: "DROPZONE.CLICK", src: "keydown" })
@@ -103,8 +116,11 @@ export function connect<T extends PropTypes>(
103116
if (disabled) return
104117
if (event.defaultPrevented) return
105118
if (props.disableClick) return
106-
// ensure it's the dropzone that's actually clicked
107-
if (event.currentTarget !== getEventTarget(event)) return
119+
120+
const target = getEventTarget<HTMLElement>(event)
121+
if (!contains(event.currentTarget, target)) return
122+
if (isInteractiveTarget(target, event.currentTarget)) return
123+
108124
// prevent opening the file dialog when clicking on the label (to avoid double opening)
109125
if (event.currentTarget.localName === "label") {
110126
event.preventDefault()

0 commit comments

Comments
 (0)