-
Notifications
You must be signed in to change notification settings - Fork 166
Fix: Improve image visibility in dark mode by inverting images #902
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
base: main
Are you sure you want to change the base?
Conversation
Hi @ugec67 since you were also interested in resolving this one, feel free to add your review/suggestions in order to contribute if you're interested. |
Thanks @perminder-17 I'd love to take a look and share my suggestions shortly. |
Great work @reshma045! 🌟 The fix has clearly improved the visibility of images in dark mode especially for diagrams with transparent or dark elements. I checked the screenshots, and the contrast is much better now. Also glad to see that light mode remains unaffected, which is perfect! Just a small thought for future enhancements if we encounter cases where images are already optimized for dark mode, we could consider a more selective approach (like targeting specific classes or containers) to avoid unnecessary inversion. But for now, this solution works well and resolves the issue in #887 effectively. It’s really nice to be part of this with you your fix was clean and thoughtful. Looking forward to contribute on more such improvements! Also a big thanks to @perminder-17 again. |
Thanks for this work @reshma045 and review @ugec67 ! I'm including a video of the dark mode with inversion below, and before we can merge this, I think it would be helpful to think about how to handle unnecessary inversion. For example, in cases of photographs of people; and of thumbnails of artworks; in addition to the case where the image is already optimized. Since @xinemata 's bug report concerned a specific image, what do you think about adding a specific class for images that do need to be inverted, and applying your filter to those images only. That would mean updating this PR to use that logic, and adding that class to the image originally reported - optionally also reviewing other images that would benefit from it? darkmode_invert.mov |
Thank you for the feedback @ugec67 and @ksen0! To make the solution more robust and avoid visual regression, I propose the following update:
Please let me know if this sounds good. Thanks again! |
I think that sounds good @reshma045 ! Thank you |
Hey @ksen0 , @perminder-17 , I've updated the PR to use a specific I also moved the example diagram image to the |
public/images/fes.svg
Outdated
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.
Hi, I was wondering why we need to add a new image. Would it be possible to use the original image instead? Thank you!
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.
Hi @lirenjie95,
Initially, I tried referencing the original image from its location at src/content/contributor-docs/images/fes.svg
, but it wasn’t being served correctly in the browser ,likely because files in src/content/...
aren't exposed publicly at runtime. As a result, the image wasn't rendering on the /contribute/fes_contribution_guide/
page. To resolve this, I moved the image to the public/images
folder, which ensures it's accessible as a static asset at runtime. This made it possible to display the image properly and apply dark mode styling.
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.
We can use import for fes.svg
instead of copying to public. I submitted a PR to your branch about how to handle it: reshma045#1
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.
Thanks, @lirenjie95, I tested it locally and it works perfectly. The import method is much better than moving it to the public folder. Merged it now.
Fix: Use import for fes.svg instead of copying to public
Description:
elements when the site is in dark mode.
This PR resolves the issue where images with transparent or dark elements were barely visible in dark mode. The fix applies a global CSS rule that inverts all
Related Issue:
Closes #887
Screenshots:
Before:

After:
Dark mode:

Light mode:
