-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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: pass localElements to restore and restoreElement API's and bump versions of duplicate elements on import #3797
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/excalidraw/excalidraw/7MAe3witCGE1FNopV3yA6X2fRHgF |
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 update the docs as well ?
# Conflicts: # src/packages/excalidraw/CHANGELOG.md
</pre> | ||
|
||
See [`restoreAppState()`](https://github.com/excalidraw/excalidraw/blob/master/src/packages/excalidraw/README.md#restoreAppState) about `localAppState`, and [`restoreElements()`](https://github.com/excalidraw/excalidraw/blob/master/src/packages/excalidraw/README.md#restoreElements) about `localElements`. |
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 are mentioning in line 751 to check the definition of restoreElements and restoreAppState so this can be removed
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.
I'm not sure it's obvious enough those arguments are related to those functions. IMO the API docs should be self-contained.
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.
Ok let's keep it 👍🏻 , I think we should instead explain every arg here though redundant instead of redirecting to other restore methods to make it more clear but we can do that later.
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.
updated the pr title to use feat instead of fix
🎉 |
that PR title tho :) — my 80-char terminal won't be pleased |
fix #3795
unsure of whether to supply
localElements
in some cases, but that also applies to currentlocalAppState
.