Skip to content

[DropZone] Fix error on dragenter for glb/gltf files #12135

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/popular-knives-guess.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': patch
---

Update DropZone so it does not error on dragenter events for glb/gltf files
40 changes: 26 additions & 14 deletions polaris-react/src/components/DropZone/DropZone.tsx
Original file line number Diff line number Diff line change
@@ -45,7 +45,14 @@ export interface DropZoneProps {
labelHidden?: boolean;
/** ID for file input */
id?: string;
/** Allowed file types */
/** Allowed file MIME types. Note that model subtypes such as model/gltf-binary
* are not recognized by most browsers. In this case, and for other unsupported
* MIME types, you can use file extensions in the accept string instead, for
* example '.glb', but note that this will disable file validation on drag events,
* since the file name is not known until the file is dropped. It is valid to mix
* MIME types and extensions in the accept string.
* @example 'image/jpeg,video/*'
*/
accept?: string;
/**
* Whether is a file or an image
@@ -188,16 +195,25 @@ export const DropZone: React.FunctionComponent<DropZoneProps> & {
const i18n = useI18n();

const getValidatedFiles = useCallback(
(files: File[] | DataTransferItem[]) => {
(event: DropZoneEvent) => {
const files = getDataTransferFiles(event);
const acceptedFiles: File[] = [];
const rejectedFiles: File[] = [];

Array.from(files as File[]).forEach((file: File) => {
!fileAccepted(file, accept) ||
(customValidator && !customValidator(file))
? rejectedFiles.push(file)
: acceptedFiles.push(file);
});
// File names and extensions are only known on drop events, so if the
// 'accept' prop includes any file extensions, we skip validation on
// non-drop events (such as dragenter and dragover) and validate
// once the user drops the file and the file name is known.
if (event.type !== 'drop' && accept?.includes('.')) {
acceptedFiles.push(...(files as File[]));
} else {
Array.from(files as File[]).forEach((file: File) => {
!fileAccepted(file, accept) ||
(customValidator && !customValidator(file))
? rejectedFiles.push(file)
: acceptedFiles.push(file);
});
}

if (!allowMultiple) {
acceptedFiles.splice(1, acceptedFiles.length);
@@ -214,9 +230,7 @@ export const DropZone: React.FunctionComponent<DropZoneProps> & {
stopEvent(event);
if (disabled) return;

const fileList = getDataTransferFiles(event);

const {files, acceptedFiles, rejectedFiles} = getValidatedFiles(fileList);
const {files, acceptedFiles, rejectedFiles} = getValidatedFiles(event);

dragTargets.current = [];

@@ -239,15 +253,13 @@ export const DropZone: React.FunctionComponent<DropZoneProps> & {
stopEvent(event);
if (disabled) return;

const fileList = getDataTransferFiles(event);

if (event.target && !dragTargets.current.includes(event.target)) {
dragTargets.current.push(event.target);
}

if (dragging) return;

const {rejectedFiles} = getValidatedFiles(fileList);
const {rejectedFiles} = getValidatedFiles(event);

setDragging(true);
setInternalError(rejectedFiles.length > 0);
45 changes: 45 additions & 0 deletions polaris-react/src/components/DropZone/tests/DropZone.test.tsx
Original file line number Diff line number Diff line change
@@ -90,6 +90,37 @@ describe('<DropZone />', () => {
expect(spy).toHaveBeenCalledWith(files, acceptedFiles, rejectedFiles);
});

it('calls the onDrop callback with files, acceptedFiles, and rejectedFiles when the accept prop is an extension', () => {
const dropZone = mountWithApp(<DropZone onDrop={spy} accept=".jpg" />);
const testFiles = [{type: 'image/jpeg', name: 'cat.jpg'}];
fireEvent({wrapper: dropZone, testFiles});
expect(spy).toHaveBeenCalledWith(testFiles, testFiles, []);
});

it('calls the onDrop callback with files, acceptedFiles, and rejectedFiles when the mime type is empty', () => {
const dropZone = mountWithApp(<DropZone onDrop={spy} accept=".glb" />);
const testFiles = [{type: '', name: 'cat.glb'}];
fireEvent({wrapper: dropZone, testFiles});
expect(spy).toHaveBeenCalledWith(testFiles, testFiles, []);
});

it('calls the onDrop callback with files, acceptedFiles, and rejectedFiles when the accept prop is a combination of mime types and extensions', () => {
const dropZone = mountWithApp(
<DropZone onDrop={spy} accept="image/jpeg,.glb" />,
);
const testFiles = [
{type: 'image/jpeg', name: 'cat.jpg'},
{type: '', name: 'cat.glb'},
{type: 'image/png', name: 'cat.png'},
];
fireEvent({wrapper: dropZone, testFiles});
expect(spy).toHaveBeenCalledWith(
testFiles,
[testFiles[0], testFiles[1]],
[testFiles[2]],
);
});

it('calls the onDropAccepted callback with acceptedFiles when it accepts only jpeg', () => {
const dropZone = mountWithApp(
<DropZone onDropAccepted={spy} accept="image/jpeg" />,
@@ -413,6 +444,20 @@ describe('<DropZone />', () => {
children: errorOverlayText,
});
});

it('does not render an error on dragenter event when file extensions are accepted', () => {
const dropZone = mountWithApp(
<DropZone
errorOverlayText={errorOverlayText}
accept=".glb"
variableHeight
/>,
);
fireEvent({wrapper: dropZone, eventType: 'dragenter'});
expect(dropZone).not.toContainReactComponent(Text, {
children: errorOverlayText,
});
});
});

describe('onFileDialogClose', () => {