-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
DropZone: Convert to TypeScript #43962
Conversation
Size Change: +60 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
@@ -149,7 +182,7 @@ export default function DropZoneComponent( { | |||
} ); | |||
|
|||
return ( | |||
<div ref={ ref } className={ classes }> | |||
<div { ...restProps } ref={ ref } className={ classes }> |
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.
export default function DropZoneProvider( { | ||
children, | ||
}: { | ||
children: React.ReactNode; |
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.
Not going to extract this to the types.ts file because this is deprecated.
* @param {Object} props Named parameters. | ||
* @param {boolean} [props.isDisabled] Whether or not to disable the drop zone. | ||
* @param {(e: DragEvent) => void} [props.onDragStart] Called when dragging has started. | ||
* @param {(e: DragEvent) => void} [props.onDragEnter] Called when the zone is entered. | ||
* @param {(e: DragEvent) => void} [props.onDragOver] Called when the zone is moved within. | ||
* @param {(e: DragEvent) => void} [props.onDragLeave] Called when the zone is left. | ||
* @param {(e: MouseEvent) => void} [props.onDragEnd] Called when dragging has ended. | ||
* @param {(e: DragEvent) => void} [props.onDrop] Called when dropping in the 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.
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.
LGTM 🚀
Just flagging some unexpected behavior from interacting with the component on Storybook (although I don't think that any of this was introduced by the changes from this PR):
Kapture.2022-09-09.at.19.35.18.mp4
- The blue overlay takes the whole canvas, instead of the area where the component renders initially
- The blue overlay doesn't always animate smoothly when opening
- The blue overlay doesn't always close when leaving the drop area with the pointer
const [ isDraggingOverElement, setIsDraggingOverElement ] = useState(); | ||
const [ type, setType ] = useState(); | ||
...restProps | ||
}: WordPressComponentProps< DropZoneProps, 'div', false > ) { |
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.
Should this component be polymorphic?
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 could be potentially, but not in the current form because the div
used in the code is a raw div element and not a polymorphic one (like View
or styled.div
).
Thanks, I'll file some issues once the Storybook is live 👍 |
Part of #35744
What?
Converts DropZone to TypeScript, and adds a story.
Why?
Better docs and devex.
Testing Instructions
npm run storybook:dev
and see stories for DropZone in Docs view.