Skip to content
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

Consistently style code blocks in tsconfig reference #3335

Open
wants to merge 1 commit into
base: v2
Choose a base branch
from

Conversation

mkantor
Copy link
Contributor

@mkantor mkantor commented Feb 15, 2025

Use json tsconfig language identifier for all tsconfig snippets for uniform syntax highlighting on https://www.typescriptlang.org/tsconfig.

The `json tsconfig` language identifier is now used in all tsconfig
snippets.
@jakebailey
Copy link
Member

There's not really a need for this, all code blocks are formatted during build.

@mkantor
Copy link
Contributor Author

mkantor commented Feb 15, 2025

@jakebailey I was inspired to suggest these changes when I noticed that code blocks on the live https://www.typescriptlang.org/tsconfig site are inconsistently highlighted.

For example, this one from explainFiles:

Screenshot 2025-02-15 at 10 39 00 AM

Uses different colors than this one from listEmittedFiles:

Screenshot 2025-02-15 at 10 39 06 AM

I do see now that indentation is irrelevant to what's actually displayed on the page. I could scrub 9ae2236 from this branch.

@mkantor

This comment was marked as outdated.

@mkantor mkantor force-pushed the tsconfig-code-blocks branch from 9ae2236 to 699e65d Compare February 17, 2025 22:16
@mkantor
Copy link
Contributor Author

mkantor commented Feb 17, 2025

I've removed the indentation tweaks, leaving only the changes that will affect syntax highlighting. @jakebailey mind taking another look when you have a chance?

@jakebailey jakebailey added the deploy-preview Enables automatic deployments to preview environments on a PR label Feb 18, 2025
Copy link
Contributor

Azure Static Web Apps: Your stage site is ready! Visit it here: https://victorious-plant-05c166c10-3335.centralus.5.azurestaticapps.net

@jakebailey
Copy link
Member

Looking at the preview, I think this may actually be a regression; the colors appear to have less contrast which may mean that this is somehow undoing some accessibility work we had to do to ensure that contrast is high enough. e.g. here, the top is json tsconfig, the bottom is jsonc:

image

To my eye, everything should really be jsonc. But I don't know that things are fully consistent yet with this PR either, unfortunately.

@mkantor
Copy link
Contributor Author

mkantor commented Feb 19, 2025

I assumed the json tsconfig identifier was the preferred one because it's more specific, but aesthetically (and readability-wise) I agree with you and prefer the lighter keys. I'd be happy to switch them all to jsonc or whatever else is preferred (or maybe the color scheme for json tsconfig ought to be changed?).

@jakebailey
Copy link
Member

To be honest I'm not sure who's in charge of the coloring, other than https://github.com/microsoft/TypeScript-Website/blob/v2/packages/typescriptlang-org/lib/themes/typescript-beta-dark.json which doesn't actually mention json at all, so it's entirely possible that the colors are a fluke.

@orta
Copy link
Contributor

orta commented Feb 19, 2025

probably a side-effect of the hover implementation on the keys of the tsconfig, likely because you've seen the page it links to (as it would be linking back to the tsconfig reference page)

@mkantor
Copy link
Contributor Author

mkantor commented Feb 19, 2025

I hadn't even noticed they were links!

The darker blue comes from here:

Locally I hacked in this override and now I see the lighter blue in those code blocks:

--- a/packages/typescriptlang-org/src/templates/markdown-twoslash.scss
+++ b/packages/typescriptlang-org/src/templates/markdown-twoslash.scss
@@ -137,6 +137,10 @@ pre code a {
   text-decoration: none;
 }
 
+.raised pre code a {
+  color: inherit;
+}
+
 pre .query {
   margin-bottom: 10px;
   color: #137998;

@jakebailey
Copy link
Member

Oh, wow, I didn't know that either. Yeah, we should not let those links change based on clicked status.

@mkantor
Copy link
Contributor Author

mkantor commented Feb 22, 2025

I've opened #3337 to make these links use syntax highlighter colors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy-preview Enables automatic deployments to preview environments on a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants