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
docs: fix no-js dark theme bugs #16612
base: main
Are you sure you want to change the base?
Conversation
|
Name | Link |
---|---|
5005a10 | |
https://app.netlify.com/sites/docs-eslint/deploys/6394d52929a13b0008a86304 | |
https://deploy-preview-16612--docs-eslint.netlify.app | |
To edit notification comments on pull requests, go to your Netlify site settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kecrily has your concern been addressed? If so, please approve and merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Sorry for the many notifications in my inbox that I didn't notice this update. I am looking for tools to improve this situation
I've approved it, but I don't have the authority to merge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@media (prefers-color-scheme: dark) { | ||
pre[class*="language-"] { | ||
color: var(--color-neutral-100); | ||
} | ||
|
||
.token { | ||
&.comment, | ||
.prolog, | ||
.doctype, | ||
.cdata { | ||
color: #8e9fae; | ||
} | ||
} | ||
|
||
.logo-component { | ||
fill: #fff; | ||
} | ||
|
||
#logo-center { | ||
opacity: 0.6; | ||
} | ||
|
||
.rule__categories, | ||
.rule__status { | ||
background: var(--body-background-color); | ||
} | ||
|
||
.algolia-docsearch-suggestion--highlight { | ||
background-color: var(--link-color); | ||
color: var(--color-neutral-900); | ||
} | ||
|
||
.alert { | ||
&.alert--warning { | ||
color: var(--color-rose-300); | ||
background-color: var(--color-rose-900); | ||
border: 1px solid var(--color-rose-300); | ||
} | ||
|
||
&.alert--important { | ||
color: var(--color-warning-300); | ||
background-color: var(--color-warning-900); | ||
border: 1px solid var(--color-warning-300); | ||
} | ||
|
||
&.alert--tip { | ||
color: var(--color-success-300); | ||
background-color: var(--color-success-900); | ||
border: 1px solid var(--color-success-300); | ||
} | ||
} | ||
|
||
.alert__type, | ||
.alert__learn-more { | ||
.alert--warning & { | ||
color: var(--color-rose-200); | ||
} | ||
|
||
.alert--important & { | ||
color: var(--color-warning-200); | ||
} | ||
|
||
.alert--tip & { | ||
color: var(--color-success-200); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a :not([data-theme="light"])
range would have solved the problem @mdjermanovic found.
Details
@media (prefers-color-scheme: dark) { | |
pre[class*="language-"] { | |
color: var(--color-neutral-100); | |
} | |
.token { | |
&.comment, | |
.prolog, | |
.doctype, | |
.cdata { | |
color: #8e9fae; | |
} | |
} | |
.logo-component { | |
fill: #fff; | |
} | |
#logo-center { | |
opacity: 0.6; | |
} | |
.rule__categories, | |
.rule__status { | |
background: var(--body-background-color); | |
} | |
.algolia-docsearch-suggestion--highlight { | |
background-color: var(--link-color); | |
color: var(--color-neutral-900); | |
} | |
.alert { | |
&.alert--warning { | |
color: var(--color-rose-300); | |
background-color: var(--color-rose-900); | |
border: 1px solid var(--color-rose-300); | |
} | |
&.alert--important { | |
color: var(--color-warning-300); | |
background-color: var(--color-warning-900); | |
border: 1px solid var(--color-warning-300); | |
} | |
&.alert--tip { | |
color: var(--color-success-300); | |
background-color: var(--color-success-900); | |
border: 1px solid var(--color-success-300); | |
} | |
} | |
.alert__type, | |
.alert__learn-more { | |
.alert--warning & { | |
color: var(--color-rose-200); | |
} | |
.alert--important & { | |
color: var(--color-warning-200); | |
} | |
.alert--tip & { | |
color: var(--color-success-200); | |
} | |
} | |
} | |
@media (prefers-color-scheme: dark) { | |
:not([data-theme="light"]) { | |
pre[class*="language-"] { | |
color: var(--color-neutral-100); | |
} | |
.token { | |
&.comment, | |
.prolog, | |
.doctype, | |
.cdata { | |
color: #8e9fae; | |
} | |
} | |
.logo-component { | |
fill: #fff; | |
} | |
#logo-center { | |
opacity: 0.6; | |
} | |
.rule__categories, | |
.rule__status { | |
background: var(--body-background-color); | |
} | |
.algolia-docsearch-suggestion--highlight { | |
background-color: var(--link-color); | |
color: var(--color-neutral-900); | |
} | |
.alert { | |
&.alert--warning { | |
color: var(--color-rose-300); | |
background-color: var(--color-rose-900); | |
border: 1px solid var(--color-rose-300); | |
} | |
&.alert--important { | |
color: var(--color-warning-300); | |
background-color: var(--color-warning-900); | |
border: 1px solid var(--color-warning-300); | |
} | |
&.alert--tip { | |
color: var(--color-success-300); | |
background-color: var(--color-success-900); | |
border: 1px solid var(--color-success-300); | |
} | |
} | |
.alert__type, | |
.alert__learn-more { | |
.alert--warning & { | |
color: var(--color-rose-200); | |
} | |
.alert--important & { | |
color: var(--color-warning-200); | |
} | |
.alert--tip & { | |
color: var(--color-success-200); | |
} | |
} | |
} | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can wrap it inside the no-js
class. It will fix the issue I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can wrap it inside the
no-js
class. It will fix the issue I guess.
Sounds good to me. @kecrily what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good for me
Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update. |
bump |
Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update. |
@amareshsm is still working on this. Don't close. |
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[x] Other, please explain:
Fixed no-js (js disabled) dark theme bugs.
What changes did you make? (Give an overview)
When javascript is disabled and the prefers-color-scheme is dark
1. code block texts are not visible.
2. Logo/tag text not inverted based on the theme.