-
-
Notifications
You must be signed in to change notification settings - Fork 490
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: 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tbd: maybe we should use plain text or javascript as default
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 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
@@ -3,7 +3,7 @@ | |||
"private": true, | |||
"version": "0.17.1", | |||
"scripts": { | |||
"dev": "vite", | |||
"dev": "vite --host", |
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.
does this do anything when not passing a host?
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 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. |
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 |
@areknawo Great work! Could you add a |
tests/src/end-to-end/ariakit/ariakit.test.ts-snapshots/ariakit-emoji-picker-chromium-linux.png
Show resolved
Hide resolved
group: "blockContent", | ||
marks: "", | ||
code: true, | ||
defining: true, |
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.
Does defining
do anything specific here?
|
||
return false; | ||
}, | ||
Tab: ({ editor }) => { |
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.
Shift+Tab to unindent would also be great!
This feature was sponsored by CellaJS 💖
This PR adds a code block with syntax highlighting (using prosemirror-highlight) to the core.
closes #721
closes #147
closes #1099
closes #721