-
Notifications
You must be signed in to change notification settings - Fork 788
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
Bring back the yellow color for "medium" coverage #542
Comments
Ping @gotwarlost @davglass @tmcw |
I'm 👍 on this change, @gotwarlost @tmcw are you two ok with it too? |
Yep, looks good to me! |
Could you submit a PR that I can merge? Sorry for the pedantry but I literally count the minutes that I have time to work on istanbul and would rather focus on getting a 1.x version out instead. |
PR #597 seems to cover it as far as I can tell from my testing. 💝 |
While I am OK with change requested here and implemented in #597 I think this partial solution. I would like to suggest to move to old color scheme. I am not against new flat design but imo old one has better contrast and more saturated colors better say where all is OK and where we have gaps. see hacked screen below guys, what do you think on this? |
As discussed in gotwarlost/istanbul#542, this PR brings back the yellow colour for medium. There are many good points raised in that PR, and this PR, aims to replicate address those concerns for this repo.
As discussed in gotwarlost/istanbul#542, this PR brings back the yellow colour for medium. There are many good points raised in that PR, and this PR, aims to replicate address those concerns for this repo.
After the recent redesign (#434, #439), the color scheme for "medium" coverage changed from a yellow theme (e.g.
#ffe87c
) to a gray/grey theme (e.g.#eaeaea
ʚ
/#666666
):I think this change is confusing to consumers for a number of reasons, and possibly warrants restoring the old yellow theme.
To me, seeing the Gray color immediately made me think that something was WRONG, e.g.:
istanbul ignore
comments were usedHowever, after I stopped for a few minutes to dig through the Istanbul codebase (again), I discovered that is just the new color of choice for "medium" coverage. Confusing!
Some additional reasons why I'm pro-Yellow:
Would you consider changing the color scheme for "medium" coverage back to yellow-based?
I'd recommend using something a little lighter than the old background color (e.g.
#fff4c2
) to more closely align with the lightness of the current green and red background colors, and then using a darker yellow/gold to fill the need of the newly reflective chart-filling color (e.g.#f9cd0b
).Example screenshot:
The text was updated successfully, but these errors were encountered: