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

fix(theme): single-click toggle with dark default #223 #224

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

AR21SM
Copy link
Contributor

@AR21SM AR21SM commented Mar 8, 2025

Fixed: Issue on page /intro.html #223

chrome-extension___nlipoenfbbikpbjkfpfillcgkoblgpmj_setup-react.html_from.install.1.mp4

@AR21SM
Copy link
Contributor Author

AR21SM commented Mar 8, 2025

Hi @aterrel Could you review this PR?

Best regards,
AR21SM

@melissawm
Copy link

melissawm commented Mar 10, 2025

I don't want to block valuable contributions (and I think this is one!) but I think we need to consider upstream changes when possible. Relying on custom solutions might work now but break in the future when a new jupyterbook release comes out. In this case, there is an example of discussion around the three-clicks pattern here: pydata/pydata-sphinx-theme#1491

In particular, quoting from that converstion pydata/pydata-sphinx-theme#1491 (reply in thread):

I am unsure if you're thinking about disabling it for you as a user or for some documentation you maintain but having the "auto" option is good from an accessibility point of view for those folks that rely on customisation or accessibility supporting settings.

@AR21SM
Copy link
Contributor Author

AR21SM commented Mar 10, 2025

Hi @melissawm,

Thank you for your feedback! I understand the concern about upstream compatibility and the potential risks of custom solutions breaking with future updates.

I reviewed the discussion in pydata/pydata-sphinx-theme#1491, and I see the reasoning behind the three-click pattern and the importance of keeping the "auto" option for accessibility.

Best regards,
AR21SM

@aterrel
Copy link
Member

aterrel commented Mar 11, 2025

I agree with @melissawm we don't want to just get rid of the auto mode. I do like the MDN Docs example given in the thread. It would allow for other switches.

@AR21SM
Copy link
Contributor Author

AR21SM commented Mar 11, 2025

@aterrel Thank you for your valuable feedback!
Should I close this PR for now?
Best regards,
AR21SM

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

Successfully merging this pull request may close these issues.

3 participants