feat: Code block #1177
feat: Code block #1177
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@areknawo is attempting to deploy a commit to the TypeCell Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
This looks great!
I think the only things remaining would be to add this block to the nodeconversions / htmlconversions unit tests (we can also do this) and to the default schema showcase (https://blocknote-git-fork-areknawo-main-typecell.vercel.app/basic/all-blocks)
I also noticed that we don't correctly set the language when it's parsed from markdown (you can try this here: https://blocknote-git-fork-areknawo-main-typecell.vercel.app/interoperability/converting-blocks-from-md).
One other thing I'm in doubt of is which languages we should show in the default dropdown. Maybe it makes more sense to limit options to some popular languages by default, what do you think?
| return { | ||
| indentLineWithTab: true, | ||
| supportedLanguages: [ | ||
| { |
There was a problem hiding this comment.
tbd: maybe we should use plain text or javascript as default
There was a problem hiding this comment.
I also wasn't sure about this. Usually, in HTML/MD, code blocks without language simply leave it out, so I made an empty string the default and assigned a Code label to it. I think the empty string makes sense, but label can be changed
There was a problem hiding this comment.
I think we should have a text language (https://shiki.style/languages#plain-text) for plain text.
Then we can still decide to upon creation initialize with plain text vs javascript. Maybe let's do javascript for now because otherwise people don't really notice the syntax highlighting
There was a problem hiding this comment.
Ok, I think I can add defaultLanguage option with text as the default.
For text, txt, plain values, I understand renderHTML should skip them, not to add class="language-text" to the HTML?
| !$from.parent.textContent && | ||
| isTextSelection(selection) | ||
| ) { | ||
| const from = $from.pos - $from.parentOffset - 2; |
There was a problem hiding this comment.
could you explain what's happening here? Or add a comment in the code?
There was a problem hiding this comment.
Do you mean a specific line or the keyboard shortcut as a whole?
As for the shortcut - this one removes an empty code block on DELETE from the inside (common behavior across different WYSIWYG editors that didn't work out-of-the-box in BlockNote). I can add a comment
There was a problem hiding this comment.
The specific line. My reasoning; when I read this code with a manual position fix in the review, I figured there's probably something interesting going on - so I was triggered to ask about this. From your reply that seems to be the case indeed, so let's document it with a comment and turn this into shared knowledge.
TLDR: if there's a "workaround" needed for something, let's try to document that as much as possible in the code for others or your "future self"
| @@ -3,7 +3,7 @@ | |||
| "private": true, | |||
| "version": "0.17.1", | |||
| "scripts": { | |||
| "dev": "vite", | |||
| "dev": "vite --host", | |||
There was a problem hiding this comment.
does this do anything when not passing a host?
There was a problem hiding this comment.
It exposes the dev server on the local network. I didn't mean to leave it, but it's helpful for testing on different devices
I see that you specifically modify rehype handler to use a
Wasn't sure about this, especially with Shiki having support for different language variants (e.g. |
| }; | ||
|
|
||
| export const codeBlockPropSchema = { | ||
| ...defaultProps, |
There was a problem hiding this comment.
@YousefED Should the code block support default props (e.g. textAlignment, backgroundColor, etc.). I've referenced it from other blocks, but I'm not sure if it makes sense here
Cool. newline, not sure, let's remove it (might have been TypeCell related)
What about this list: https://shiki.style/languages -> web bundle, but
What do you think? |
I think it's a good selection. It's always going to be a bit subjective, but it makes sense to shorten it a little bit by default |
This PR adds a code block with syntax highlighting (using prosemirror-highlight) to the core.