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: add much more customisable FileUpload component #713

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

karrui
Copy link
Collaborator

@karrui karrui commented May 9, 2024

This PR adds a new FileUpload component that is designed to supersede the Attachment component. This component is much more customisable if needed, and contains a few prop changes as compared to Attachment.

Closes #586 (above and beyond?)

Example usages can be seen in the stories, but replicated here for convenience:

Default usage (with rhf)

const DefaultUsage = () => {
  const { control } = useForm<{ files: File[] }>()

  return (
    <Controller
      control={control}
      name="files"
      render={({ field: { value, onChange, ...field } }) => (
        <FileUpload
          maxFiles={1} // Or more if you want multiple upload. Defaults to 1
          maxFileSize={1 * 1024} // In bytes
          accept="image/*" // Or use mime type and array, see https://zagjs.com/components/react/file-upload
          value={value}
          // Note use of onFileAccept instead of onChange
          // Also has `onFileReject` props for handling rejected files,
          // `onError` props for easy handling of error messages from the rejected files
          onFileAccept={({ files }) => onChange(files)}
          {...field}
        />
      )}
    />
  )
}

With parts ownself modify

export const WithParts = (args: FileUploadRootProps) => {
  return (
    <FileUpload.Root {...args}>
      <FileUpload.Dropzone>
        <FileUpload.DropzoneIcon />
        <FileUpload.Label />
        <FileUpload.DraggingLabel />
      </FileUpload.Dropzone>
      <FileUpload.Context>
        {({ acceptedFiles }) =>
          acceptedFiles.length !== 0 && (
            <FileUpload.ItemGroup>
              {acceptedFiles.map((file, index) => (
                <FileUpload.Item key={`${file.name}-${index}`} file={file}>
                  <FileUpload.ItemPreview type="image/*">
                    <FileUpload.ItemPreviewImage />
                  </FileUpload.ItemPreview>
                  <FileUpload.ItemName />
                  <FileUpload.ItemSizeText />
                  <FileUpload.ItemDeleteTrigger />
                </FileUpload.Item>
              ))}
            </FileUpload.ItemGroup>
          )
        }
      </FileUpload.Context>
      <FileUpload.ErrorText />
      <FileUpload.MaxSizeHelperText />
      {/* MUST be included or the upload dialog will not work */}
      <FileUpload.HiddenInput />
    </FileUpload.Root>
  )
}

@karrui karrui requested a review from jia1 May 9, 2024 11:33
Comment on lines +122 to +124
/**
* @deprecated Use `FileInput` component instead, as that is much more customisable.
*/
Copy link
Member

@jia1 jia1 May 13, 2024

Choose a reason for hiding this comment

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

Does this mean next time, folks who only want the default options got to use all those FileInput.* elements?

@@ -119,6 +119,9 @@ export type AttachmentProps<Multiple extends boolean> =
onRejection?: (rejections: FileRejection[]) => void
} & AttachmentValueProp<Multiple>

/**
* @deprecated Use `FileInput` component instead, as that is much more customisable.
Copy link
Member

@jia1 jia1 May 13, 2024

Choose a reason for hiding this comment

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

Suggested change
* @deprecated Use `FileInput` component instead, as that is much more customisable.
* @deprecated Use `FileUpload` component instead, as that is much more customisable.


return (
<chakra.div __css={styles.errorText} {...rest} ref={ref}>
<Icon as={BxsErrorCircle} />
Copy link
Member

Choose a reason for hiding this comment

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

Should all Icon have aria-hidden? Saw you had it for FileUploadDropzoneIcon

const apiRef = useRef<Api<PropTypes>>()
const api = connect(state, send, normalizeProps)
apiRef.current = api
return api
Copy link
Member

Choose a reason for hiding this comment

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

How does apiRef get utilised / referred to if it isn't returned here?

return errorMessage
}
case 'FILE_TOO_SMALL': {
let errorMessage = 'This file is too small.'
Copy link
Member

Choose a reason for hiding this comment

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

When does this scenario happen...?

* @param fileSizeInBytes the size of the file in bytes to be converted to a readable string
* @returns the human-readable file size string
*/
export const getReadableFileSize = (fileSizeInBytes: number): string => {
Copy link
Member

Choose a reason for hiding this comment

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

getReadableFileSize(0) returns NaN undefined, but Idk how far you want to go w.r.t. error handling cus setting maxFileSize as 0 is obv the OGP dev fault...

image

Copy link
Member

Choose a reason for hiding this comment

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

And I suppose technology is not that advanced yet - 1000 TB gives 1 undefined

return (
<FileUploadRoot
{...props}
maxFiles={maxFiles}
Copy link
Member

Choose a reason for hiding this comment

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

The component (in storybook) silently rejects files when there are more number of files than allowed. Is this expected?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow overriding of text inside Attachment dropzone
2 participants