-
Notifications
You must be signed in to change notification settings - Fork 59
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(Excalidraw): Add support for Excalidraw #122
base: main
Are you sure you want to change the base?
Conversation
|
||
export const getHTML = async (url) => { | ||
const svg = await getImage(url); | ||
return `<a style="box-shadow: none" href="${url}">${svg |
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.
This box-shadow
thing was to hide the underline on the links on the Excalidraw blog. Maybe this should be removed or made more generic?
@trevorblades @j-f1 Like I already said in excalidraw/gatsby-embedder-excalidraw#2, I think it would be better if we had one of the following options instead of using
Not only is I'd love to have an @excalidraw transformer built into the plugin, but I don't think using Maybe @kentcdodds has some thoughts on this too? |
I think it should be pretty simple to put this into a separate package and then: module.exports = {
// Find the 'plugins' array
plugins: [
{
resolve: `gatsby-transformer-remark`,
options: {
plugins: [
{
resolve: `gatsby-remark-embedder`,
options: {
customTransformers: [
require('name-of-excalidraw-embedder-module'),
],
services: {
// The service-specific options by the name of the service
},
},
},
// Other plugins here...
],
},
},
],
}; I really don't think that this should be added into core when it requires installing puppeteer. |
@kentcdodds I'd love to have it built into the plugin by default, but the puppeteer dependency is making me hesitant too. If there's a way we could figure this out with the @excalidraw team, I'm all in! |
I think it makes sense to keep this as a separate package as long as it relies on puppeteer. I'm not sure if it's on the roadmap or something that they'd want to do, but if Excalidraw supports oEmbed or public image URLs in the future, we can pick this back up and create a dependency-free implementation. The (nice?) thing about this method is that it outputs raw SVG code in the markup rather than pointing to an image/embed link hosted on another domain, so it only needs to make a network request to Excalidraw once during the build phase. |
@travorblades having an endpoint that returns the svg markup is fine by me too. That can also be achieved when at the same time being compliant to the oembed structure/standard |
One security concern is that if Excalidraw renders the drawing on the server, it needs to be passed the decryption key (which goes against end-to-end encryption), whereas the Puppeteer method never sends that information to the server. Theoretically we could implement this as a Node module so we don’t need Puppeteer but that’s a ways down the line. |
@j-f1 It maybe could be easier to be oembed compliant instead? 🤔 Just doing a call to https://excalidraw.com/oembed/url=XXX which returns a JSON object having an |
Anything server side would still need to decrypt the stored drawing though, right? |
@j-f1 That's what's happening at this moment too when clicking the button (like you do with |
No that happens on the build server — the end to end encryption means the excalidraw server never sees drawing content right now. |
@j-f1 The |
I don’t quite understand. “Build server” here means wherever you run Gatsby. |
@j-f1 I think we're talking next to each other, could you maybe DM me on Twitter to explain it to me please? |
It would be amazing if there was a solution without puppeteer. let me know if i can help. |
This branch adds support for Excalidraw links using the technique from https://github.com/excalidraw/gatsby-embedder-excalidraw. It closes #121
Checklist: