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

add a11y information for contrast ratio in background-images and fix issue in the example #33190

Merged
merged 5 commits into from
May 22, 2024

Conversation

glmvc
Copy link
Contributor

@glmvc glmvc commented Apr 21, 2024

Description

On the reference page for the background-image CSS property, there is an example "Layering background images" where the text is hard to read because of the background images.

On other pages, such as background-color, there is information about contrast ratio (Accessibility concerns).

I've added that information for background-image as well, and fixed the contrast ratio issue in the example below.

Motivation

Such aspects should be stated and applied consistently.

Additional details

/

Related issues and pull requests

@glmvc glmvc requested a review from a team as a code owner April 21, 2024 10:56
@glmvc glmvc requested review from chrisdavidmills and removed request for a team April 21, 2024 10:56
@github-actions github-actions bot added Content:CSS Cascading Style Sheets docs size/s 6-50 LoC changed labels Apr 21, 2024
Copy link
Contributor

github-actions bot commented Apr 21, 2024

Preview URLs

External URLs (3)

URL: /en-US/docs/Web/CSS/background-image
Title: background-image

(comment last updated: 2024-05-22 09:12:42)

Copy link
Contributor

@chrisdavidmills chrisdavidmills 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 for a super-useful update, @glmvc!

I have left a few suggestions for you to consider, but this is mostly looking good.

@@ -83,8 +91,10 @@ Note that the star image is partially transparent and is layered over the cat im

```css
p {
font-weight: bold;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is still difficult to read. I think white text with a black text shadow (I used text-shadow: 0.07em 0.07em 0.05em black;) is more legible

files/en-us/web/css/background-image/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/background-image/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/background-image/index.md Show resolved Hide resolved
files/en-us/web/css/background-image/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/background-image/index.md Outdated Show resolved Hide resolved
Co-authored-by: Chris Mills <chrisdavidmills@gmail.com>
@glmvc glmvc requested a review from chrisdavidmills May 5, 2024 09:16
Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

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

LGTM, thanks again @glmvc, and apologies for taking so long to respond to this.

Note: I made a couple of changes to the example text color and shadow as I felt it was still pretty hard to read.

@chrisdavidmills chrisdavidmills merged commit 4a2d790 into mdn:main May 22, 2024
8 checks passed
@glmvc glmvc deleted the patch-1 branch May 23, 2024 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:CSS Cascading Style Sheets docs size/s 6-50 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants