Skip to content

Conversation

@franknoirot
Copy link
Contributor

@franknoirot franknoirot commented Nov 17, 2025

We've been using the default syntax highlighting theme in light mode and the "oneDark" syntax highlighting theme in dark mode for a long time, and I've got some qualms with them:

Light mode

  1. The comment color is a bit too noticeable and clashes with our UI, a sort of rusty red
  2. Most of the text of KCL programs are either variable declarations or variable references, and all of these are bright blue
  3. Function argument labels have the same appearance as variable declarations

Dark mode

  1. There is very little differentiation between: variable declarations, variable references, function calls, and argument labels
  2. The text is overwhelmingly red which is not really the color palette we use for the app otherwise

Over the weekend I tried out a first pass at a replacement light and dark theme. I'm not sold on my approach but I want to get it out to the team to give their feedback and adjustments, because now I should be able to make changes very easily.

Light

Before

image

After

image

Dark

Before

image

After

image

@vercel
Copy link

vercel bot commented Nov 17, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
modeling-app Ready Ready Preview Comment Nov 18, 2025 11:05pm

@franknoirot
Copy link
Contributor Author

The unit tests are failing because I tried to wrap the unlabeled argument in KCL with a named grammar so I could somehow highlight it differently than labelled arguments, but I didn't find a way to do that at the end of the pipeline. I can remove that change if no one has ideas on how to get that highlighting to work, or if you think it's not worth trying to highlight them differently anyway.

@jtran
Copy link
Contributor

jtran commented Nov 17, 2025

You need to update the unit tests. I'm guessing the ones in this file, packages/codemirror-lang-kcl/test/call.txt, but anything that uses a call with an unlabeled arg in that directory.

@adamchalmers
Copy link
Contributor

adamchalmers commented Nov 17, 2025

Screenshot 2025-11-17 at 11 18 06 AM

My only concern is that this looks a bit weird, I think it's too high contrast between the black and white and cyan. Maybe a darker shade of blue?

Other than that, this looks amazing. Great work!

@franknoirot
Copy link
Contributor Author

Can do @jtran and @adamchalmers!

@franknoirot franknoirot marked this pull request as ready for review November 18, 2025 22:49
@franknoirot franknoirot requested a review from a team as a code owner November 18, 2025 22:49
@franknoirot
Copy link
Contributor Author

@jtran I opted to just drop my UnlabeledArgument because I couldn't get highlighting to work how I wanted for it and I didn't have a very clear idea to begin with. So I think the tests will likely pass now.

@franknoirot
Copy link
Contributor Author

@adamchalmers how is this feeling now? I dropped the chroma and the lightness a bit.

Screenshot 2025-11-18 at 5 39 55 PM

@franknoirot
Copy link
Contributor Author

I did break that E2E test, I'll debug in the AM

@franknoirot franknoirot marked this pull request as draft November 19, 2025 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants