carpentries / instructor-training

Instructor Training
https://carpentries.github.io/instructor-training/
Other
175 stars 290 forks source link

Check images for dark mode compatibility #1715

Open astroDimitrios opened 3 months ago

astroDimitrios commented 3 months ago

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

brownsarahm commented 3 months ago

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

astroDimitrios commented 3 months ago

@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 commented 3 months ago

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 commented 3 months ago

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 commented 2 months ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 3 weeks ago

I have opened https://github.com/carpentries/varnish/pull/153, which could use some extra testing, to add support for dark mode images :)