-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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: implemented onDrop callback function #3960
base: master
Are you sure you want to change the base?
Conversation
Text edit events
Zoom to fit api
On drop event
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/excalidraw/excalidraw/765e5L4ZcKoGZW4i2y51bFLCCS8j |
fixed zoom to max when empty, and target null
Update App.tsx
@dwelle, I don't disagree with you, however, I took onPaste as the template. https://www.npmjs.com/package/@excalidraw/excalidraw#onPaste |
Ok, so it seems you're more aware of some of our codebase than I am. It seems I've approved the PR (#3420), but missed this. @ad1992 thoughts? My opinion: |
src/components/App.tsx
Outdated
await this.props.onDrop(event); | ||
return; |
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.
looks like right now the drop action will always be prevented (since we just return ) if present which is incorrect.
Right now we are preventing paste action if the onPaste
returns true but I do agree with @dwelle we should allow false
instead to prevent to be semantically more correct.
We can make the onPaste
change in a separate PR, let's fix the onDrop
in this PR.
@@ -184,6 +184,9 @@ export interface ExcalidrawProps { | |||
data: ClipboardData, | |||
event: ClipboardEvent | null, | |||
) => Promise<boolean> | boolean; | |||
onDrop?: ( | |||
event: React.DragEvent<HTMLDivElement>, | |||
) => Promise<boolean> | 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.
) => Promise<boolean> | boolean; | |
) => Promise<boolean | void> | boolean | void; |
Co-authored-by: David Luzar <luzar.david@gmail.com>
Text edit events
@zsviczian I think you've mistakenly merged in the zoomToFit code into this PR |
@zsviczian whats the status of this PR ? |
I've implemented this in the Obsidian fork and have been using this since last year september... I still think this should be part of the official package. |
@zsviczian could you rebase the PR with latest master and. clean up the PR as it contains some unnecessary changes like upgrading deps, editing |
Very similar to onPaste callback.