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(client): make alt text on images appear in dark mode #9250

Merged
merged 2 commits into from
Sep 28, 2023

Conversation

CBID2
Copy link
Contributor

@CBID2 CBID2 commented Jul 7, 2023

Summary

Fixes #9130

Problem

The alt text of images is not visible due to the way the background color is set

Solution

I made set the background color to transparent for the dark theme. This will make the alt text appear in dark mode.

@bsmth
Copy link
Member

bsmth commented Jul 7, 2023

Thanks a lot for the changes! I tried this fix out myself with an image with a wrong src:

image

image

Looks good, in my opinion. I don't know if it affects any other places where we have images (pngs?) with a transparent background. It would also be great for Yari dev to have a look, also.

@bsmth bsmth added ux make the user experience awesome contrast issues related to low contrast, especially in dark mode labels Jul 7, 2023
@bsmth bsmth requested a review from LeoMcA July 7, 2023 10:29
Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, transparent works, but we should better use the --background-primary CSS variable instead to keep the behavior similar, which is white with the light theme and 90% black with the dark theme.

@caugner caugner changed the title fix: change image's background color to transparent fix(image): make alt text appear in dark mode Sep 28, 2023
@caugner caugner removed the request for review from LeoMcA September 28, 2023 12:42
Co-authored-by: Claas Augner <[email protected]>
@CBID2
Copy link
Contributor Author

CBID2 commented Sep 28, 2023

Thank you, transparent works, but we should better use the --background-primary CSS variable instead to keep the behavior similar, which is white with the light theme and 90% black with the dark theme.

Done @caugner! :)

@caugner caugner changed the title fix(image): make alt text appear in dark mode fix(client): make alt text on images appear in dark mode Sep 28, 2023
Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! 🙌

@CBID2
Copy link
Contributor Author

CBID2 commented Sep 28, 2023

@caugner, my PR did not automatically merge.

@caugner caugner merged commit f432df1 into mdn:main Sep 28, 2023
@CBID2 CBID2 deleted the Making-images'-alt-text-visible branch September 28, 2023 19:56
caugner added a commit that referenced this pull request Oct 26, 2023
#9250 caused a regression by changing the 
background color of all images from white to the default background color, 
causing issues with images containing black elements on transparent background.

This reverts the image background color to white, but sets the text color to the 
default text color for the dark theme to resolve the original issue with alt texts.

Co-authored-by: Claas Augner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contrast issues related to low contrast, especially in dark mode ux make the user experience awesome
Projects
None yet
Development

Successfully merging this pull request may close these issues.

alt text of images not visible in dark theme (a11y)
3 participants