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

DocDiff: color highlights should override text color #389

Open
thibaudcolas opened this issue Sep 29, 2024 · 2 comments
Open

DocDiff: color highlights should override text color #389

thibaudcolas opened this issue Sep 29, 2024 · 2 comments
Labels
Feature New feature

Comments

@thibaudcolas
Copy link

I’m trying to provide feedback for the new DocDiff feature as part of the new Addons, please let me know if there’s a better venue for that.

Details

Expected Result

I’d expect DocDiff highlights to be readable regardless of what theme I use with Read the Docs. Demo where I tried this on: Wagtail API v2 Configuration Guide - Authentication - PR #12363

Actual Result

With a dark theme we get:

dark theme diffdoc

Additionally the d shortcut was pretty hard to find (had to look through the various announcements to figure out what I might be missing, and then figure out where DocDiff is documented). I was expecting a toggle within the flyover menu.

@humitos humitos transferred this issue from readthedocs/readthedocs.org Sep 30, 2024
@humitos
Copy link
Member

humitos commented Sep 30, 2024

I’m trying to provide feedback for the new DocDiff feature as part of the new Addons, please let me know if there’s a better venue for that.

Thanks for your feedback. I transferred the issue to the addons repository.

I’d expect DocDiff highlights to be readable regardless of what theme I use with Read the Docs

I don't think we can auto-detect the colorscheme of the documentation and adapt its colors based on that. However, we could provide a way to manually configure these colors. We are using CSS variables for similar configurations for other addons. Would that work in your case?

Additionally the d shortcut was pretty hard to find (had to look through the various announcements to figure out what I might be missing, and then figure out where DocDiff is documented). I was expecting a toggle within the flyover menu.

Yes. We are already aware of this and we are working to integrate these new features in the flyout so they are easier to discover.

@thibaudcolas
Copy link
Author

Thank you @humitos! I wasn’t expecting the implementation to auto-detect the color scheme, just override the text color in addition to the background. Though if you wanted to do some auto-detection this would be partly possible with the color-scheme meta.

CSS variables sound great too, but I’d also be very happy to just see the text color overridden to black so at least the text is readable without requiring theme customizations.

@humitos humitos added the Feature New feature label Oct 8, 2024
@humitos humitos changed the title DocDiff color highlights should override text color DocDiff: color highlights should override text color Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature
Projects
None yet
Development

No branches or pull requests

2 participants