Define PR colors as hex colors to make them the same as GitHub's#5480
Define PR colors as hex colors to make them the same as GitHub's#5480stefanhaller wants to merge 2 commits intomasterfrom
Conversation
This is so that they look the same no matter what color palette the terminal is using. (One user complained that the text for the Open state is barely readable, because they are using a palette that has a very pale green.) GitHub uses slightly different colors depending on light vs. dark mode; fortunately they are very close, so hopefully we can ignore this. I picked the ones for dark mode here, on the assumption that this is more common. Also, not all terminals support true hex colors; for example, Terminal.app on macOS doesn't, so it maps the colors to the closest ones in the Xterm-256 palette. This shouldn't be a huge problem, but for some reason it displays draft PRs as something closer to Cyan than grey, and I don't understand why.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Duplication | 1 |
🟢 Coverage 0.00% diff coverage
Metric Results Coverage variation Report missing for bb5d5f21 Diff coverage ✅ 0.00% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (bb5d5f2) Report Missing Report Missing Report Missing Head commit (b3ffa3d) 61908 53896 87.06% Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#5480) 11 0 0.00% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%1 Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.
TIP This summary will be updated as you push new changes. Give us feedback
|
Thank you for the PR and having a look at it. My feeling is actually not that the color of the pill is the issue, but the color of the text inside. I think that the color used for the status text is not what it's supposed to be: 
An additional issue with hardcoding the colors and not using the ones from the config is that the colors in general will look off |
|
It is using the palette color "white" for the text, which is apparently mapped to some shade of grey in your terminal config. I pushed a fixup that changes it to a true white (hex color). I also pushed another branch (
If by "config" you mean the terminal's palette config, then yes, that was my original point. Not sure how much of an issue that is, though. |
|
I tinkered with the colors a bit myself and found that a darker text with the colors from the original look best. I think in the end its about contrast. e.g. with withPrBgColor(state, style.FgBlackLighter.Sprint(stateText(state))),And yes definitely, the terminal's colors should be used |
Yes, I mentioned that option here. It's true that the contrast is better, but maybe only with your terminal config (and mine), might not be that way for everyone. Also, it looks different from GitHub's design, which I find very unfortunate. I'm actually wondering how much readability matters in this case; you will usually tell the state from the color more than from the text, no? (Unless you're color blind, maybe.) |
|
Hm, difficult... Maybe another option would be to drop the pill shape altogether 🤔 I think this feature is the only place where there is such a shape. Just text in the status color then... |
|
That would be a shame though. You get this when you turn off nerd fonts, and I think it looks much worse. I think I'll just keep it the way it is then. It's the best compromise (I remember I came to that conclusion last time I fiddled with it), and the fact that it's not perfect contrast doesn't bother me too much. |
2 participants
This is so that they look the same no matter what color palette the terminal is using. (One user complained that the text for the Open state is barely readable, because they are using a palette that has a very pale green.)
GitHub uses slightly different colors depending on light vs. dark mode; fortunately they are very close, so hopefully we can ignore this. I picked the ones for dark mode here, on the assumption that this is more common.
Also, not all terminals support true hex colors; for example, Terminal.app on macOS doesn't, so it maps the colors to the closest ones in the Xterm-256 palette. This shouldn't be a huge problem, but for some reason it displays draft PRs as something closer to Cyan than grey, and I don't understand why.