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

Check images for dark mode compatibility #1715

Open
astroDimitrios opened this issue Aug 1, 2024 · 8 comments
Open

Check images for dark mode compatibility #1715

astroDimitrios opened this issue Aug 1, 2024 · 8 comments
Labels
status:wait Progress dependent on another issue or conversation

Comments

@astroDimitrios
Copy link

What is the problem?

There is an image in the memory management section that has no background (transparent) and the text is black so can't be seen in dark mode: https://carpentries.github.io/instructor-training/05-memory.html#chunking

Maybe this can have a slightly grey background to make it work with both themes?

Location of problem (optional)

https://carpentries.github.io/instructor-training/05-memory.html#chunking

@ndporter ndporter added help wanted Looking for Contributors high priority Need to be addressed ASAP labels Aug 1, 2024
@brownsarahm
Copy link
Contributor

@froggleston does sandpaper support using different images for dark and light mode?

@astroDimitrios
Copy link
Author

@brownsarahm Not currently. When a user chooses dark mode atm any normal images are darkened slightly and desaturated. It might be possible to have completely different images for light and dark mode but this would require changes to sandpaper and varnish so that images are wrapped in the <picture> tag (and probably more javascript).

I think SVG's if inlined can contain a prefers-color-scheme media query but I haven't tested using any inline svg with the Workbench.

When Mermaid diagrams are in Varnish these automatically respect the colour theme.

@brownsarahm
Copy link
Contributor

that mermaid note is good to know.

I think it's worth merging Karen's PR for now and keeping this open while we make a more holistic plan. @ndporter does that make sense to you?

@ndporter
Copy link
Contributor

ndporter commented Aug 2, 2024

I went ahead and merged @karenword's fix #1716 of replacing the SVG with PNG. We should definitely come back to this problem once there's a more elegant solution. Also, I didn't change the merge to remove the old SVG version (to keep it easily available/editable) but before closing this issue, any unused files should be removed.

@ndporter ndporter added status:wait Progress dependent on another issue or conversation and removed help wanted Looking for Contributors high priority Need to be addressed ASAP labels Aug 2, 2024
@ndporter
Copy link
Contributor

ndporter commented Sep 26, 2024

Adding the mental model diagrams with the ball to the to-do list for this one for more immediate help

EDIT: Also the arrows on the mental maps diagrams (including but not only this one)

@ha0ye
Copy link
Contributor

ha0ye commented Oct 23, 2024

It might also be worth checking if a bit of css for inserting the images or editing on the svg to have a background would be a viable alternative solution. see https://stackoverflow.com/questions/70174816/how-to-change-background-color-in-svg

Not sure it matters greatly, but I like the idea of svg images that scale better and are lighter-weight than png.

@brownsarahm
Copy link
Contributor

i 💯 support the goal of making svgs work better. I think it's reasonable to plan that we design all of the svgs for a light background and then insert it manually for darkmode, but I could be missing use cases

@astroDimitrios
Copy link
Author

astroDimitrios commented Oct 29, 2024

I have opened carpentries/varnish#153, which could use some extra testing, to add support for dark mode images :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:wait Progress dependent on another issue or conversation
Projects
None yet
Development

No branches or pull requests

4 participants