-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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] Theme selector #10181
[DOCS] Theme selector #10181
Conversation
✅ Deploy Preview for niobium-lead-7998 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## develop #10181 +/- ##
===========================================
- Coverage 79.71% 79.71% -0.01%
===========================================
Files 455 455
Lines 39646 39646
===========================================
- Hits 31604 31603 -1
- Misses 8042 8043 +1 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
5148f1d
to
b8f7b57
Compare
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! I left a couple of comments related to the code and some others related to specific cases where I think we need to adjust some of the styling for dark mode. cc. @SuperYeehee
docs/docusaurus/src/components/WasThisHelpful/styles.module.scss
Outdated
Show resolved
Hide resolved
Could you clarify what you mean on "Adjust some of the styling for DM?" Are you references like a set of standard variables like we use in our GX theme.ts? |
@SuperYeehee , I'm referring to my comments above☝🏽 |
border-radius: .25rem; | ||
box-shadow: 0 .2rem .5rem var(--pst-color-shadow), 0 0 .0625rem var(--pst-color-shadow) !important; | ||
box-shadow: none !important; |
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.
Does this work without the !important
?
docs/docusaurus/src/css/api_docs/pydata-sphinx-theme-modified.scss
Outdated
Show resolved
Hide resolved
docs/docusaurus/src/css/api_docs/pydata-sphinx-theme-modified.scss
Outdated
Show resolved
Hide resolved
docs/docusaurus/src/css/api_docs/pydata-sphinx-theme-modified.scss
Outdated
Show resolved
Hide resolved
docs/docusaurus/src/css/api_docs/pydata-sphinx-theme-modified.scss
Outdated
Show resolved
Hide resolved
docs/docusaurus/src/css/api_docs/pydata-sphinx-theme-modified.scss
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.
Looks great! I love dark mode! please address some final suggestions but otherwise is good to go.
…layed on the right
356790c
to
3463eb3
Compare
Deploy Preview
Mockups
Jira Ticket
DOCS_DARK_THEME.mp4
EDIT: Also removed the opacity in the navbar (both in desktop at the top side of the screen and in mobile with the sidebar menu), this change applies to the dark AND light theme.
invoke lint
(usesruff format
+ruff check
)For more information about contributing, see Contribute.
After you submit your PR, keep the page open and monitor the statuses of the various checks made by our continuous integration process at the bottom of the page. Please fix any issues that come up and reach out on Slack if you need help. Thanks for contributing!