-
Notifications
You must be signed in to change notification settings - Fork 88
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(NcRichText): highlight code syntax if language provided #6212
base: master
Are you sure you want to change the base?
Conversation
cc @juliusknorr for possible affect on Office apps |
Signed-off-by: Maksim Sukharev <[email protected]>
Signed-off-by: Maksim Sukharev <[email protected]>
dafd9a9
to
4d2c704
Compare
Signed-off-by: Maksim Sukharev <[email protected]>
Nice one, all good from my side. Only relevant usage is comment rendering in deck, where this works as expected 👍 |
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.
Nice!
But for NcRichText
it costs about 6.6 kB css + 191 kB js (297 -> 488 kb) in production minified build.
We can load it async on the first code block render, but I don't know if it worth it. @susnux What do you think? |
Yes I think it makes sense, sadly that plugin forces to bundle languages. |
Proposal:
In theory when it is loaded p.1 changes and triggers rerender in p.2. |
Signed-off-by: Maksim Sukharev <[email protected]>
228e30f
to
99b5395
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.
Tested, looks good
☑️ Resolves
Supported languages: https://github.com/wooorm/lowlight/blob/main/lib/common.js
🖼️ Screenshots
🚧 Tasks
use-extended-markdown
without providing lang prefix would work as before🏁 Checklist
next
requested with a Vue 3 upgrade