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

highest-rated-comment has a red background in comment header #4529

Closed
omBratteng opened this issue Jun 29, 2021 · 8 comments · Fixed by #4554
Closed

highest-rated-comment has a red background in comment header #4529

omBratteng opened this issue Jun 29, 2021 · 8 comments · Fixed by #4554

Comments

@omBratteng
Copy link
Contributor

While looking at issues on GitHub, I noticed that some comments has a bright red background header, where it was dark green before.
E.g. this comment
vercel/swr#93 (comment)
image

Refined GitHub seems to somehow change the css variables definition
image

When I disable Refined GitHub, it is back to being the default black header background.

@omBratteng omBratteng added the bug label Jun 29, 2021
@fregante
Copy link
Member

Either you have a faulty style sheet somewhere or you’re victim of a bad push at GitHub. That’s an obvious bug and that code is not part of refined GitHub 😃

@yakov116
Copy link
Member

Same here. GitHub change something since last night my css looks a drop off (all over)

@yakov116
Copy link
Member

Found the issue. if you have the preview enabled.

image

@omBratteng
Copy link
Contributor Author

I can confirm, it is the Dark High Contrast feature. Disabling it brought back the green header.

@fregante fregante changed the title Highest rated comment has a red background in comment header highest-rated-comment has a red background in comment header Jun 30, 2021
@fregante
Copy link
Member

fregante commented Jun 30, 2021

My guess is that, since this is a feature preview, GitHub is using red to be able to spot those colors across the site.

It's also possible that they're moving away from those generic variables so the fact that they're here is a coincidence.

If anyone wants to look into this by replacing the variables with values that work in this theme, feel free, but ensure it works correctly across the themes and that the variables make sense (i.e. don't pick semantic variables that are unrelated, like --color-calendar-graph-day-L2-bg, just because they're green)

Don't open a PR that requires 4 reviews; it's only 4 variables that need to be changed.

@omBratteng
Copy link
Contributor Author

I only had the feature preview enabled, I didn't use the Dark High Contrast theme.
It's most likely an issue from GitHub's end, that they create the variable twice, one with actual correct colour, and one with just red.

I will give feedback on the feature preview, because those css variables looks wrong to me.

	--color-auto-black: red;
    --color-auto-white: red;
    --color-auto-gray-0: red;
    --color-auto-gray-1: red;
    --color-auto-gray-2: red;
    --color-auto-gray-3: red;
    --color-auto-gray-4: red;
    --color-auto-gray-5: red;
    --color-auto-gray-6: red;
    --color-auto-gray-7: red;
    --color-auto-gray-8: red;
    --color-auto-gray-9: red;
    --color-auto-blue-0: red;
    --color-auto-blue-1: red;
    --color-auto-blue-2: red;
    --color-auto-blue-3: red;
    --color-auto-blue-4: red;
    --color-auto-blue-5: red;
    --color-auto-blue-6: red;
    --color-auto-blue-7: red;
    --color-auto-blue-8: red;
    --color-auto-blue-9: red;
    --color-auto-green-0: red;
    --color-auto-green-1: red;
    --color-auto-green-2: red;
    --color-auto-green-3: red;
    --color-auto-green-4: red;
    --color-auto-green-5: red;
    --color-auto-green-6: red;
    --color-auto-green-7: red;
    --color-auto-green-8: red;
    --color-auto-green-9: red;
    --color-auto-yellow-0: red;
    --color-auto-yellow-1: red;
    --color-auto-yellow-2: red;
    --color-auto-yellow-3: red;
    --color-auto-yellow-4: red;
    --color-auto-yellow-5: red;
    --color-auto-yellow-6: red;
    --color-auto-yellow-7: red;
    --color-auto-yellow-8: red;
    --color-auto-yellow-9: red;
    --color-auto-orange-0: red;
    --color-auto-orange-1: red;
    --color-auto-orange-2: red;
    --color-auto-orange-3: red;
    --color-auto-orange-4: red;
    --color-auto-orange-5: red;
    --color-auto-orange-6: red;
    --color-auto-orange-7: red;
    --color-auto-orange-8: red;
    --color-auto-orange-9: red;
    --color-auto-red-0: red;
    --color-auto-red-1: red;
    --color-auto-red-2: red;
    --color-auto-red-3: red;
    --color-auto-red-4: red;
    --color-auto-red-5: red;
    --color-auto-red-6: red;
    --color-auto-red-7: red;
    --color-auto-red-8: red;
    --color-auto-red-9: red;
    --color-auto-purple-0: red;
    --color-auto-purple-1: red;
    --color-auto-purple-2: red;
    --color-auto-purple-3: red;
    --color-auto-purple-4: red;
    --color-auto-purple-5: red;
    --color-auto-purple-6: red;
    --color-auto-purple-7: red;
    --color-auto-purple-8: red;
    --color-auto-purple-9: red;
    --color-auto-pink-0: red;
    --color-auto-pink-1: red;
    --color-auto-pink-2: red;
    --color-auto-pink-3: red;
    --color-auto-pink-4: red;
    --color-auto-pink-5: red;
    --color-auto-pink-6: red;
    --color-auto-pink-7: red;
    --color-auto-pink-8: red;
    --color-auto-pink-9: red;
    --color-fade-fg-10: red;
    --color-fade-fg-15: red;
    --color-fade-fg-30: red;
    --color-fade-fg-50: red;
    --color-fade-fg-70: red;
    --color-fade-fg-85: red;
    --color-fade-black-10: red;
    --color-fade-black-15: red;
    --color-fade-black-30: red;
    --color-fade-black-50: red;
    --color-fade-black-70: red;
    --color-fade-black-85: red;
    --color-fade-white-10: red;
    --color-fade-white-15: red;
    --color-fade-white-30: red;
    --color-fade-white-50: red;
    --color-fade-white-70: red;
    --color-fade-white-85: red;
    --color-global-nav-logo: red;
    --color-global-nav-bg: red;
    --color-global-nav-text: red;
    --color-global-nav-icon: red;
    --color-global-nav-input-bg: red;
    --color-global-nav-input-border: red;
    --color-global-nav-input-icon: red;
    --color-global-nav-input-placeholder: red;

@ghost
Copy link

ghost commented Jul 9, 2021

I've made a PR to fix it, this is what the release will probably look like:

highest-rated-comment-bugfix

@ghost
Copy link

ghost commented Jul 10, 2021

That design was suboptimal so I'll try changing the border instead of the text

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants