-
Notifications
You must be signed in to change notification settings - Fork 29
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
Enable dark mode #613
Enable dark mode #613
Conversation
828a552
to
05b5486
Compare
Consider having the icons change be a dedicated precursor PR. For dark mode, I think it's plausible we'll want visual regression testing for possibly everything, or at least most of them, in dark mode. That's a big PR, so it's useful to split out "prefactors" (refactors as precursors) |
fc9447a
to
02bee9e
Compare
@Eric-Arellano PTAL. Jupyter looks bad but I don't know if there's a good way to fix that or if it's within the scope of this PR. Furo suffers from the same issue. See jupyter/jupyter-sphinx#218. |
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.
Great work! This is so exciting :)
tests/js/tests.js-snapshots/admonitions-use-Carbon-style-dark-mode-1-linux.png
Outdated
Show resolved
Hide resolved
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.
Code changes look great now! Once Grace approves the admonitions, this is ready to merge
CI failure seems unrelated. It's been consistently happening for the past half hour or so though.
|
@kevinsung this all looks good! |
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.
👏
Fixes #575.
Replaces the existing Carbon icons with different SVGs that are more easily adapted for dark mode by replacing the fill with
"currentColor"
.TODO: