-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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: [WIP] other way to fix no-js dark theme #18077
Conversation
✅ Deploy Preview for docs-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -132,6 +144,10 @@ html[data-theme="light"] { | |||
--footer-background-color: var(--color-neutral-25); | |||
--outline-color: var(--color-brand); | |||
--img-background-color: #fff; |
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.
[Non-Actionable] Existing code: this --img-background-color
is only used in div.img-container
, which is used in contribute/architecture
and extend/code-path-analysis
pages. In light mode the images on those pages have a white background anyway. So 👍 there's no visual difference from manually adding this to the default :root
styles.
Just noting as it's the only exception to the styles matching up. 🙂
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.
This looks great to me, thanks for the detailed explanation & sending this!
I compared the two pairs of styles: @media
-based and html-data-theme]
-based. They all have the same attributes except for the existing https://github.com/eslint/eslint/pull/18077/files#r1477122882 that doesn't have any issues associated with it.
💯 !
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. |
@harish-sethuraman please don't just remove the "stale" label, we need to know why it's not stale. In this case, we should follow up with @amareshsm to see if he's still working on it. |
Hey Sorry missed this I am working on this. I will complete it in 1 or 2 days. |
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 are you still working on this? |
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. |
This pull request was auto-closed due to inactivity. While we wish we could keep working on every request, we unfortunately don't have the bandwidth to continue here and need to focus on other things. You can resubmit this pull request if you would like to continue working on it. |
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
[ ] Other, please explain:
What changes did you make? (Give an overview)
fixes #18076
Is there anything you'd like reviewers to focus on?