-
Notifications
You must be signed in to change notification settings - Fork 128
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: Add codemirror syntax highlighting back in and move to a static color #1080
Conversation
🦋 Changeset detectedLatest commit: de24d96 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
should we just move to prism? |
Job #375: Bundle Size — 2.06MiB (+2.2%).Warning Bundle contains 5 duplicate packages – View duplicate packages Warning Bundle introduced 2 new packages: @lezer/yaml, @codemirror/lang-yaml – View changed packages Bundle metrics
Bundle size by type
View job #375 report View geoff/doc-1464-highlighting-in-r... branch activity View project dashboard |
im down for this! I still say let's merge this since it fixes a bunch of bugs and we can do follow up work to remove the couple of lang highlighting we have : ) |
wtb preview app @hanspagel this PR has the same bundle issue |
ae76474
to
a3b2e0a
Compare
Preview deployed to https://43dc58bf-d8d1-433a-9c5f-660c5700c11f--scalar-deploy-preview.netlify.app |
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.
Awesome PR! I’ve added some minor details that came to mind when reviewing.
Anyway, I’m too stupid to see what the added CodeMirror plugins do. If you could enlighten me, that would be great.
But let’s get this in!
Ah, and I just saw @marclave had another fix for the selection color here: scalar/packages/use-codemirror/src/hooks/useCodeMirror.ts Lines 260 to 263 in 3108786
Should we add this to the PR here instead of hard coding the color? 🤔 |
woah i had no idea this was in main! we noticed css variables dont work in selection selectors edit does this happen for you? |
… value for selection color
a021325
to
a1a8ba5
Compare
This approach does not work. I think just stale code in Marcs branch |
For some reason the ::selection does not seem to accept our .dark-mode, .light-mode variables. For now we will hard code a color.
This also adds syntax highlighting back in.