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: KDS-1055 | Dropzone component #3353

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
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
3 changes: 2 additions & 1 deletion packages/components/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@
"@kaizen/tabs": "^1.5.30",
"@kaizen/tailwind": "^0.5.1",
"@kaizen/typography": "^2.3.6",
"classnames": "^2.3.2"
"classnames": "^2.3.2",
"react-dropzone": "^14.2.3"
},
"devDependencies": {
"@deanc/esbuild-plugin-postcss": "^1.0.2",
Expand Down
67 changes: 67 additions & 0 deletions packages/components/src/Dropzone/Dropzone.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import React from "react"
import { act, configure, fireEvent, render, screen, waitFor } from "@testing-library/react"
import { Dropzone } from "./Dropzone"
import { DROPZONE_AUTOMATION_ID } from "./constants"

configure({
testIdAttribute: "data-automation-id",
})

describe("<Dropzone />", () => {
const setEnableFileUpload = jest.fn()

it("renders", async () => {
render(<Dropzone setEnableFileUpload={setEnableFileUpload} />)
const element = await screen.findByTestId(DROPZONE_AUTOMATION_ID)
expect(element).toBeVisible()
})

it("renders the success icon when an acceptable file has been added", async () => {
const { container } = render(<Dropzone setEnableFileUpload={setEnableFileUpload} />)

const spreadsheetFile = new File(
["(⌐□_□) - file contents is tested in backend"],
"fake-spreadsheet.xlsx",
{
type: "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
}
)

// modify the file size property
Object.defineProperty(spreadsheetFile, "size", { value: 10000 })


const data = mockData([spreadsheetFile])
const element = await screen.findByTestId(DROPZONE_AUTOMATION_ID)
const input = element.querySelector("input")

act(() => {
input && fireEvent.drop(input, data)
})

await waitFor(() => {
expect(container).toHaveTextContent("fake-spreadsheet.xlsx")
})

})
})


/**
* This mock function was referenced from the react-dropzone package docs.
* Resource: https://react-dropzone.js.org/#section-accepting-specific-file-types
* Search 'Testing' for the segment.
*/
function mockData(files: File[]): any {
return {
dataTransfer: {
files,
items: files.map(file => ({
kind: "file",
type: file.type,
getAsFile: () => file,
})),
types: ["Files"],
},
}
}
115 changes: 115 additions & 0 deletions packages/components/src/Dropzone/Dropzone.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
/* eslint-disable no-console */
import React, { Dispatch, SetStateAction, useEffect, useState } from "react"
import classNames from "classnames"
import { useDropzone, FileRejection, DropzoneOptions } from "react-dropzone"
import { ACCEPTED_FILE_TYPES, DEFAULT_MAX_FILE_SIZE_40KB, DROPZONE_AUTOMATION_ID } from "./constants"
import { DefaultMessage } from "./sub-components/DefaultMessage/DefaultMessage"
import { DropzoneErrors } from "./sub-components/DropzoneError/DropzoneError"
import { DropzoneIcon } from "./sub-components/DropzoneIcon/DropzoneIcon"
import { OnDragMessage } from "./sub-components/OnDragMessage/OnDragMessage"
import { SelectedFileMessage } from "./sub-components/SelectedFileMessage/SelectedFileMessage"

type DropzoneProps = {
Copy link
Contributor

Choose a reason for hiding this comment

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

For the API, we're thinking it's best to extend the library's type with some minor tweaks:

  • Add the OverrideClassName type (copy this into your component)
  • Omit the multiple type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What are you thoughts on extending or Picking some of the InputHTMLAttributes as well. This can allow attributes like name when used inside of a form element being passed down to the dropzone component as well

/**
* The setter function that allows a piece of state to be sent back up to the parent component.
* This will be useful for disabling buttons within a [workflow](https://cultureamp.design/guidelines/workflow/) or a submit button within a form.
*/
setEnableFileUpload: Dispatch<SetStateAction<boolean>>;

/** this unit is in bytes: defaults to `40000` = `40kb` */
maxFileSize?: number;

/** Apply tailwind classnames which will be applied to the container */
className?: string;
}

export const Dropzone = ({ setEnableFileUpload, maxFileSize, className }: DropzoneProps): JSX.Element => {
const [selectedFileName, setSelectedFileName] = useState<string>("")
const [dropzoneErrors, setDropzoneErrors] = useState<FileRejection[]>()

const defaultConfigurationOptions: DropzoneOptions = {
noClick: true,
noKeyboard: true,
multiple: false,
maxFiles: 1,
accept: {
"application/type": ACCEPTED_FILE_TYPES,
},
Comment on lines +31 to +37
Copy link
Contributor

Choose a reason for hiding this comment

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

These should all come from props, with defaults added at the prop level.

onDragEnter: () => {
setSelectedFileName("")
},
onDropAccepted(files, event) {
setSelectedFileName(files[0].name || "")
setDropzoneErrors(undefined)
},
onError(err) {
console.error("onError", err);
},
onDrop(acceptedFiles, fileRejections, event) {
console.log("onDrop", acceptedFiles, fileRejections, event);
},
Comment on lines +38 to +50
Copy link
Contributor

Choose a reason for hiding this comment

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

Each of these should trigger the prop version eg.

onDrop(acceptedFiles, fileRejections, event) {
      props.onDrop(acceptedFiles, fileRejections, event);
      ... (internal functions)
},

onDropRejected(fileRejections, event) {
console.warn("onDropRejected", fileRejections, event);
setDropzoneErrors(fileRejections)
},
Comment on lines +51 to +54
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be a special case where we want to check if the consumer has passed in a onDropRejected function and extract their values instead of using our own. Something like this needs to happen:

const consumerErrors = onDropRejected(fileRjections);
setDropzoneErrors(consumerErrors);

}

/**
* Merge default config options with user config options
* in the future we can add more config options
*/
const maxSize = maxFileSize ? maxFileSize : DEFAULT_MAX_FILE_SIZE_40KB
/** prevent an empty file from being uploaded */
const minSize = 1000
const options: DropzoneOptions = {
...defaultConfigurationOptions,
maxSize,
minSize
}

/** initialise the dropzone hook */
const { open, isDragActive, acceptedFiles, getRootProps, getInputProps, } = useDropzone(options)

/**
* Set the parent `enableFileUpload` state to allow the parent component to update its UI
* Note: this logic will change when we allow multiple files in the future
*/
useEffect(() => {
const hasAcceptedFile = acceptedFiles.length >= 1
if (hasAcceptedFile) {
setEnableFileUpload(true)
} else {
setEnableFileUpload(false)
}
}, [acceptedFiles])
Comment on lines +73 to +84
Copy link
Contributor

Choose a reason for hiding this comment

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

This function isn't necessary since the consumer can pass something similar in to onDrop and control what they need to from the outside. eg.

const handleRequiredDropzone = ({ fileRejections }) => {
  acceptedFiles.length >= 1 && setEnableSubmit(true)
}

<Dropzone onDrop={handleRequiredDropzone} />


return (
<div className={classNames(className)}>
<div
data-automation-id={DROPZONE_AUTOMATION_ID}
Comment on lines +87 to +89
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an outdated method of applying test id's as the react-testing-library opts to use data-test-id

Having said that we prefer to allow the consumers to pass any extra prop they want through to the component, so we would spread the rest of the props onto your top level wrapping div (once extracting all the props required for the dropzone config).

className={classNames(
{
"rounded-default": true,
"border-w-default border-dashed border-gray-400": true,
"transition-colors": true,
"text-body text-center text-purple-800 transition-colors": true,
"w-100 mb-4": true,
"!border-solid !border-blue-500 !bg-blue-100": isDragActive,
}
)}
Comment on lines +90 to +99
Copy link
Contributor

Choose a reason for hiding this comment

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

You can avoid the need for all the true by passing each string in as a param:

Suggested change
className={classNames(
{
"rounded-default": true,
"border-w-default border-dashed border-gray-400": true,
"transition-colors": true,
"text-body text-center text-purple-800 transition-colors": true,
"w-100 mb-4": true,
"!border-solid !border-blue-500 !bg-blue-100": isDragActive,
}
)}
className={classNames(
"rounded-default",
"border-w-default border-dashed border-gray-400",
"transition-colors",
"text-body text-center text-purple-800 transition-colors",
"w-100 mb-4",
isDragActive &&"!border-solid !border-blue-500 !bg-blue-100"
)}

Copy link
Contributor

Choose a reason for hiding this comment

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

Although, what would be your appetite for converting all your tailwind to vanilla CSS?

{...getRootProps()}
>
<div className="flex items-center justify-center h-[120px] px-24 py-32">
Copy link
Contributor

Choose a reason for hiding this comment

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

We want to avoid any hard coded heights to support users who need to zoom, please change this to the rem equivalent

<input type="file" {...getInputProps()} />
<div>
<DropzoneIcon fileName={selectedFileName} isDragActive={isDragActive} />
{isDragActive && <OnDragMessage />}
{selectedFileName && <SelectedFileMessage fileName={selectedFileName} handleOpenDialog={open} />}
{!isDragActive && !selectedFileName && <DefaultMessage handleOpenDialog={open} />}
</div>
</div>
</div>
{dropzoneErrors && <DropzoneErrors dropzoneErrors={dropzoneErrors} acceptedFileTypes={ACCEPTED_FILE_TYPES} maxFileSize={maxSize} minFileSize={minSize} />}
</div>
)
}
3 changes: 3 additions & 0 deletions packages/components/src/Dropzone/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const ACCEPTED_FILE_TYPES = [".xlsx"]
export const DEFAULT_MAX_FILE_SIZE_40KB = 40000
export const DROPZONE_AUTOMATION_ID = "kaizen-file-dragzone-component"
92 changes: 92 additions & 0 deletions packages/components/src/Dropzone/docs/Dropzone.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import React, { useState } from "react"
import { ComponentMeta, ComponentStory } from "@storybook/react"
import { Button } from "@kaizen/button"
import { Dropzone } from "../Dropzone"
import { DEFAULT_MAX_FILE_SIZE_40KB } from "../constants"

export default {
title: "Components/Dropzone",
component: Dropzone,
parameters: {
controls: { expanded: true },
},
} as ComponentMeta<typeof Dropzone>

export const DefaultStory: ComponentStory<typeof Dropzone> = () => {
const [enableFileUpload, setEnableFileUpload] = useState<boolean>(false)
return <Dropzone setEnableFileUpload={setEnableFileUpload} />
}
DefaultStory.storyName = "Dropzone"

export const ModifiedMaxFileSize: ComponentStory<typeof Dropzone> = ({ maxFileSize }) => {
const [enableFileUpload, setEnableFileUpload] = useState<boolean>(false)
return <Dropzone maxFileSize={maxFileSize} setEnableFileUpload={setEnableFileUpload} />
}

ModifiedMaxFileSize.parameters = {
docs: {
description: {
story: "By default the `Dropzone` component has a `maxFileSize` of `40kb`. This value can be altered and the error messages will related the updated `maxFileSize`",
},
},
}
ModifiedMaxFileSize.args = {
maxFileSize: 100000
}

export const ParentContainer: ComponentStory<typeof Dropzone> = () => {
const [enableFileUpload, setEnableFileUpload] = useState<boolean>(false)
return (
<div>
<Dropzone maxFileSize={DEFAULT_MAX_FILE_SIZE_40KB} setEnableFileUpload={setEnableFileUpload} />
<Button
label="Submit"
disabled={!enableFileUpload}
primary
/>
<code className="bg-gray-300 mt-8 py-6 px-12 block rounded-default">
<small>{`Parent prop enableFileUpload: ${enableFileUpload}`}</small>
</code>
</div>
)
}

ParentContainer.parameters = {
docs: {
description: {
story: "It is required that a piece of state is passed down into the component to allow for the `Dropzone` component determine whether the upload can proceed.",
},
},
}

export const DropzoneContainerStyles: ComponentStory<typeof Dropzone> = ({ className }) => {
const [enableFileUpload, setEnableFileUpload] = useState<boolean>(false)
return (
<div>
<Dropzone maxFileSize={DEFAULT_MAX_FILE_SIZE_40KB} setEnableFileUpload={setEnableFileUpload} className={className} />
<div className="flex">
<Button
label="Cancel"
secondary
classNameOverride="!mr-8"
/>
<Button
label="Upload"
disabled={!enableFileUpload}
primary
/>
</div>
</div>
)
}

DropzoneContainerStyles.parameters = {
docs: {
description: {
story: "Tailwind classes can be applied to the `Dropzone` component container for additional styling.",
},
},
}
DropzoneContainerStyles.args = {
className: "border-solid border-grey-800 rounded-default mt-48 mb-8 p-24",
}
1 change: 1 addition & 0 deletions packages/components/src/Dropzone/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./Dropzone"
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import React from "react"
import { Paragraph } from "@kaizen/typography"
import { Hyperlink } from "../Hyperlink/Hyperlink"

interface DefaultMessageProps {
handleOpenDialog: () => void
}

const DefaultMessage = ({ handleOpenDialog }: DefaultMessageProps): JSX.Element => (
<div className="flex flex-col items-center justify-center">
<span className="inline-block mb-8">
<Paragraph variant="body">
<span className="text-purple-800">Drag and drop file or{" "}</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be a full sentence to support internationalisation as translators require full context to provide an accurate string. The suggestion here would be to write this sentence out in full and wrap the "select a file" in styles that visually hide it from the user. It's okay if the button and this text says the same thing.

<Hyperlink text="Select a file" handleOnClick={handleOpenDialog} />
</Paragraph>
</span>
<Paragraph variant="extra-small"><span className="text-purple-800/70">.xlsx format only</span></Paragraph>
Copy link
Contributor

Choose a reason for hiding this comment

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

We recommend that this should be plain text from the consumer as it would be difficult to generalise this statement when designers may want to abbreviate this message to say "All images" or not say anything at all when it comes to allowing all file types.

Copy link
Contributor

Choose a reason for hiding this comment

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

As a side note, you can use the color and tag props on Paragraph to achieve these styles.

Suggested change
<Paragraph variant="extra-small"><span className="text-purple-800/70">.xlsx format only</span></Paragraph>
<Paragraph variant="extra-small" tag="span" color="dark-reduced-opacity">.xlsx format only</Paragraph>

</div>
)

export { DefaultMessage }
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import React from "react"
import { ErrorCode, FileError, FileRejection } from "react-dropzone";
import { InlineNotification } from "@kaizen/notification";
import { getKaizenDropzoneError, prettifyErrorCode } from "../../utils";

export const DropzoneError = ({ code, message }: FileError): JSX.Element => {
/** updating this specific error code because the language is awkward. Default code from react-dropzone is 'file-invalid-type'. The other error codes are fine. */
const errorCode = code === ErrorCode.FileInvalidType ? "incorrect-file-type" : code
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer we stick with what comes out of the library as something like this transformation is easily overlooked when debugging

const notificationTitle = prettifyErrorCode(errorCode)

return (
<InlineNotification
Copy link
Contributor

Choose a reason for hiding this comment

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

Please replace all InlineNotification with FieldMessage as this is what we use in our form fields.
https://cultureamp.design/storybook/?path=/docs/components-form-subcomponents-field-message--default-kaizen-site-demo

type="negative"
title={notificationTitle}
persistent
noBottomMargin={true}
>
<span
tabIndex={0}
Copy link
Contributor

Choose a reason for hiding this comment

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

To ensure that all error messages are read by the screen reader, this tabIndex needs to be applied to your container. eg.

<div tabIndex={-1}>
    <FieldMessage />
    <FieldMessage />
</div>

Copy link
Contributor

Choose a reason for hiding this comment

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

Also generally this span (and it's remaining props) won't be needed when converted to use FieldMessage

role="alert"
aria-live="polite"
data-automation-id={""}
>
{message}
</span>
</InlineNotification>
)
}

export interface DropzoneErrorsProps {
acceptedFileTypes: string | string[],
Copy link
Contributor

Choose a reason for hiding this comment

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

Should make this only accept a string[] to simplify logic

maxFileSize: number,
/** May implement this in the future. Adding now to cover all four error code provided from react-dropzone library */
minFileSize: number,
dropzoneErrors: FileRejection[]
}

export const DropzoneErrors = ({
acceptedFileTypes,
maxFileSize,
minFileSize,
dropzoneErrors
}: DropzoneErrorsProps): JSX.Element => (
<span>
Copy link
Contributor

Choose a reason for hiding this comment

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

This will need to be a div to accommodate my other comment regarding the tabIndex

{dropzoneErrors.map(dropzoneError => {
const { errors } = dropzoneError
return errors.map((error: FileError) => {
const errorMessage = getKaizenDropzoneError({
errorCode: error.code as ErrorCode,
acceptedFileTypes,
maxFileSize,
minFileSize
})
return (
<span key={`dropzone-error-${error.code}`} className={"block mb-4"}>
<DropzoneError code={errorMessage.code} message={errorMessage.message} />
Copy link
Contributor

Choose a reason for hiding this comment

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

As per my previous comments in this file, it might be easier to use the FieldMessage directly in here rather than creating a new component.

</span>)
})
})}
</span>
)
Loading