Shopify/polaris-react

DropZone always returns error when glb/gltf file triggers `dragenter` event

Open

#6,644 创建于 2022年7月15日

在 GitHub 查看
 (18 评论) (9 反应) (1 负责人)TypeScript (1,252 fork)auto 404
BugComponentEngineeringGood first issue

仓库指标

Star
 (6,174 star)
PR 合并指标
 (PR 指标待抓取)

描述

DropZone

  • The validation error dragErrorOverlay for dragenter event relies on accept property or customValidator function property of <DropZone>https://github.com/Shopify/polaris/blob/b541f54498462ce513b9903dfcfcb966f43e0038/polaris-react/src/components/DropZone/DropZone.tsx#L190-L210

  • When accept property is present and its !fileAccepted returns true, customValidator never gets evaluated.

  • fileAccepted always returns false for glb/gltf file because:

    1. When a file is dragentered, it is not a File instance but DataTransferItem instance and DataTransferItem only has kind and type properties. When a glb/gltf dragentered, however,DataTransferItem's type has an empty string; thus an-always validation error. This part is related to a situation when accept property is a mimeType string like model/gltf+json,model/gltf-binary instead of a dot string like .glb. The validation works for files like video/mp4 because DataTransferItem has a proper type.

    2. When accept property is a dot string like .glb,.mp4,.wav, fileAccepted always returns false because DataTransferItem does not have name property. https://github.com/Shopify/polaris/blob/b541f54498462ce513b9903dfcfcb966f43e0038/polaris-react/src/components/DropZone/utils/index.ts#L30 This is a bug that should be fixed separately. all file types not working.

    3. Additionally, File instance of drop event has type property but it is an empty string. name gets correct information tho. file.name validation would work only when accept is a dot string.

  • To avoid dragErrorOverlay showing up for dragenter event, a file validation needs to rely on customValidator and omit accept property

  • This compromises "click and select files" user interaction of System UI outside of browsers because to filter file types on System, it relies on accept property of <input type="file"> tag.

Examples

dragErrorOverlay showing up even tho DropZone actually accepts glb

const [files, setFiles] = useState<File[]>()
const handleDrop = (acceptedFiles: File[]) => setFiles(acceptedFiles)
const fileUpload = !files.length && <DropZone.FileUpload />
const uploadedFiles = files.length > 0 && (
    <Stack vertical>
      {files.map((file, index) => (
        <Stack alignment="center" key={index}>
          <Thumbnail
            size="small"
            alt={file.name}
            source={window.URL.createObjectURL(file)}
          />
          <div>
            {file.name} <Caption>{file.size} bytes</Caption>
          </div>
        </Stack>
      ))}
    </Stack>
);
...
      <DropZone accept="model/gltf-binary" type="file" onDropAccepted={handleDrop}>
        {uploadedFiles}
        {fileUpload}
      </DropZone>

Best practices

The DropZone component should either:

  • skip fileAccepted validation for dragenter event when DataTransferItem.type has an empty string. (with a better documentation otherwise coders implicitly have to use a dot string for accept prop)
  • prioritize customValidator over fileAccepted and give coders a better control. If customValidator is present, just skip fileAccepted. (This is because I would rather have a mimetype <=> a dot string extension conversion in the customValidator and check both file.name and file.type with a specified filetypes as accept prop)

-ps I would like to create a PR but I wanted to make sure that we have a consensus on which solution would be better.

贡献者指南