-
Notifications
You must be signed in to change notification settings - Fork 129
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
dot, tex, and mermaid blocks #8
Conversation
public/client.js
Outdated
e.setAttribute("fill", "currentColor"); | ||
e.removeAttribute("fill-opacity"); | ||
} | ||
svg.remove(); |
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.
Curious why this is needed? Does renderSVGElement add the element to a virtual DOM somewhere?
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 creates a standalone XML document (using DOMParser, I believe). So we need to remove it from that document before we can add it to the page.
public/client.js
Outdated
bgcolor: "none" | ||
}, | ||
nodeAttributes: { | ||
color: "#00000101", |
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.
Curious why you need to set both the color and opacity here, or is that required by the package?
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.
Graphviz don’t understand proper CSS colors, so I can’t set this to currentColor. So this is a hack to set a color that no one should use in practice, and then replace it after rendering which currentColor. I wanted to set the opacity to exactly zero, but then Graphviz ignores the RGB parts.
HTML code blocks would also help with #32, maybe. ```js
const link = "https://example.com";
```
```html
This is a <a href=${link}>link</a>.
``` |
b7c352c
to
333a052
Compare
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've rebased this PR which had diverged a bit, and added unit tests. LGTM!
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 think it would also be good add mermaid too since other systems have that.
src/markdown.ts
Outdated
@@ -77,26 +78,36 @@ function uniqueCodeId(context: ParseContext, content: string): string { | |||
return id; | |||
} | |||
|
|||
function isLive(language) { | |||
return language === "js" || language === "dot" || language === "tex"; |
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.
What really determines this set of languages and how we can determine it algorithmically so we don't need this hard-coded list here? Maybe this could be pulled into an array at a higher scope? Or should it be part of the parse context?
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.
tracking this suggestion in #113
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 set of languages is determined by getSource
below:
Maybe I could rewrite this code to avoid duplication. 🤔 In the case of tagged languages, we could if desired implement it a bit more declaratively by enumerating a list of tags and raw flags. (Note that the language tag can be different than the implementation tag, as is the case with tex
and tex.block
.)
const template = TemplateParser.parse(input, options); | ||
const source = new Sourcemap(input); | ||
escapeTemplateElements(source, template, raw); | ||
source.insertLeft(template.start, tag + "`"); |
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.
To pass attributes to the tag function, this might need to be something like new Function(make${tag}(JSON.stringify(attrs) ))
or maybe tag + ".bind(" + JSON.stringify(attrs) + ")"
I added Mermaid blocks in the latest commit. |
7d6f337
to
d0876a8
Compare
Adds support for dot (Graphviz) and tex (KaTex) blocks. For Graphviz, I used a newer version and added some hacks to make it work in dark mode out of the box. I’d like to update KaTeX to use ES modules instead of require, too.
Note that adding a dot block with incremental update doesn’t work because the library won’t be in the import map (which has to be set on load). We’ll need to detect this case and trigger a reload instead of an incremental update, or some other solution I haven’t thought of yet.