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

Make locations visible (Intellij) #149

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

daniilsapa
Copy link
Collaborator

@daniilsapa daniilsapa commented Dec 16, 2024

Resolves #140

@illright
I replaced chalk with pinocolours as you suggested in the issue description.

Here you can compare the diagnostics (I used dim method)

Light theme before/after:
Screenshot 2024-12-16 at 15 46 56
Screenshot 2024-12-16 at 15 44 37

Dark theme before/after:
Screenshot 2024-12-16 at 15 46 42
Screenshot 2024-12-16 at 15 45 03

Copy link

changeset-bot bot commented Dec 16, 2024

🦋 Changeset detected

Latest commit: 547bca1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@steiger/pretty-reporter Minor

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

@daniilsapa
Copy link
Collaborator Author

The font looks a bit lighter, so it's a bit more readable in the dark theme now. Do you think it's enough?

@daniilsapa
Copy link
Collaborator Author

Also, here's the difference for VS Code

Dark theme before/after:
Screenshot 2024-12-16 at 15 55 40
Screenshot 2024-12-16 at 15 58 09

Light theme before/after:
Screenshot 2024-12-16 at 15 56 18
Screenshot 2024-12-16 at 15 57 26

@illright
Copy link
Member

I like it more in the dark mode of IntelliJ, but I like it less in the light mode of VS Code, seems a bit too pale there. I'll also play around some more. My main target here is sufficient contrast between the background and the text, we can use https://webaim.org/resources/contrastchecker/ to check

@daniilsapa
Copy link
Collaborator Author

daniilsapa commented Dec 16, 2024

Yeah, makes sense, I'll play around with the contrast a bit and send you some screenshots. I also thought about coloring the location according to severity, e.g. dark yellow/red for warn/error.

@daniilsapa
Copy link
Collaborator Author

Here you can see the variant of locations colored according to the severity:

Error:
Screenshot 2024-12-17 at 00 46 47
Screenshot 2024-12-17 at 00 48 02

Warning:
Screenshot 2024-12-17 at 00 48 35
Screenshot 2024-12-17 at 00 48 53

@illright
Copy link
Member

Nah, this ain't it either. The red color is too attention-grabbing, we don't want the path to draw attention away from the error text

@daniilsapa
Copy link
Collaborator Author

Also, #757575 looks quite contrasty against both white and black backgrounds. But unfortunately, picocolors does not allow us to set arbitrary hex values for colors. Additionally, we could make the font bolder, it helps with readability a bit

Intellij
Screenshot 2024-12-17 at 01 31 37
Screenshot 2024-12-17 at 01 31 52

VS Code
Screenshot 2024-12-17 at 01 32 59
Screenshot 2024-12-17 at 01 33 29

@illright
Copy link
Member

Just played around with it myself, seems like #757575 is pc.dim, which doesn't pass the contrast check against the default dark theme of VS Code. pc.gray, however, is #919191, and that's better

https://webaim.org/resources/contrastchecker/?fcolor=919191&bcolor=1E1E1E

@@ -1,18 +1,19 @@
import { relative } from 'node:path'
import figures from 'figures'
import terminalLink from 'terminal-link'
import chalk from 'chalk'
import picocolors from 'picocolors'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (non-blocking): I think the convention is to import is as pc, also helps make the code shorter

Suggested change
import picocolors from 'picocolors'
import pc from 'picocolors'

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I'll shorten the variable name

@daniilsapa
Copy link
Collaborator Author

Just played around with it myself, seems like #757575 is pc.dim, which doesn't pass the contrast check against the default dark theme of VS Code. pc.gray, however, is #919191, and that's better

Yes, pc.gray looks better for the default dark theme of VS Code. But it does not look readable in dark themes of IntelliJ. Also, these colors have different values in different themes. Here's a table where you can see all color values and their contrast rates. pc.gray has better overall results but looks very unreadable on dark themes in IntelliJ. pc.dim is bad everywhere, but I would say it's "average bad", it has values that are a bit lower than the pass criteria almost everywhere.

Theme Dim (actual color, contrast rate) Gray (actual color, contrast rate)
VS Code Dark (bg #1e1e1e) #737373, 3.51:1 #919191, 5.28:1 - pass
VS Code Dark Modern (bg #181818) #707070, 3.58:1 #828282, 4.62:1 - pass
VS Code Light (bg #ffffff) #a0a0a0, 2.61:1 #666666, 5.74:1 - pass
VS Code Light Modern (bg #f8f8f8) #a0a0a0, 2.46:1 #6C6C6C, 4.94:1 - pass
Intellij Dark (bg #1e1f22) #757679, 3.62:1 #4a4c51, 1.91:1
IntelliJ Dracula (bg #2b2b2b) #858688, 3.88:1 #4f5156, 1.78:1
Intellij Light (bg #ffffff) #7f7f7f, 4:1 #6d707d, 4.92:1 - pass

I'll try to do something with pc.gray in IntelliJ

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.

Diagnostic locations look unreadable in IntelliJ in dark mode
2 participants