-
Notifications
You must be signed in to change notification settings - Fork 137
feat: add FileDropZone component #2487
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
base: main
Are you sure you want to change the base?
Conversation
|
Preview is ready. |
|
Visual Tests Report is ready. |
e93907d to
8173352
Compare
|
The state of the component is not reset with such a case: Screen.Recording.2025-11-12.at.17.59.06.mov |
| export type UseFileInputProps = { | ||
| onUpdate?: (files: File[]) => void; | ||
| onChange?: (event: React.ChangeEvent<HTMLInputElement>) => void; | ||
| multiple?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The multiple prop was not initially added, as the hook gives the basic configuration for the native input. That is, if the consumer needs multiple, then you need to add it yourself, otherwise it turns out to be a strange proxy that passes the prop through and returns it in the same form without using it in any way in calculations.
| role: 'button', | ||
| }; | ||
|
|
||
| export interface DroppableProps extends Required<typeof DROP_ZONE_BASE_ATTRIBUTES> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We usually name props according to this logic <ComponentOrHookName> + details. This is necessary so that all types have their own name space + it improves DX when working with lib types
| ``` | ||
|
|
||
| The `useDropZone` hook provides props for an element to act as a drop zone and also gives access to the dragging-over state. | ||
| Additionally, the hook supports a more imperative approach: it can accept a `ref` for cases where you don't have direct access to the HTML element you want to make a drop zone. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you describe the minimum usage examples?
|
|
||
| export interface UseDropZoneState { | ||
| isDraggingOver: boolean; | ||
| getDroppableProps: () => DroppableProps; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seems to be no reason to make this as function
| } | ||
|
|
||
| function typeMatchesPattern(actualMimeType: string, expectedMimeTypePattern: string): boolean { | ||
| const actualMimeTypeParts = actualMimeType.split('/'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we assuming that only files will be used?
| description?: string; | ||
| buttonText?: string; | ||
| icon?: IconData | null; | ||
| errorIcon?: IconData | null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need more than one icon property?
| import i18n from '../i18n'; | ||
|
|
||
| type FileDropZoneButtonProps = { | ||
| className?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to me that it would be convenient to have the children property so that explicitly using the button in the component layout, you can explicitly define the text in the button itself and not in its parent
buttonText is convenient to use as a parent зкщзукен if you don't explicitly use FileDropZoneButton
| import {useFileZoneContext} from '../FileDropZone.Provider'; | ||
|
|
||
| type FileDropZoneDescriptionProps = { | ||
| className?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same with the children property as in FileDropZoneButton
| import i18n from '../i18n'; | ||
|
|
||
| type FileDropZoneTitleProps = { | ||
| className?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same with the children property as in FileDropZoneButton
|
|
||
| export interface FileDropZoneProps extends Pick<BaseInputControlProps, 'validationState'> { | ||
| accept: UseDropZoneAccept; | ||
| onAdd: (files: File[]) => void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We usually use the name onUpdate for such callbacks, this is a familiar pattern for users of our library
No description provided.