This PR introduces a new rule,no-empty-alt-text which nudges people to add alt text when they set an image to decorative. Settingalt="" to mark decorative images is usually acceptable, but currently leads to unexpected behaviors, specifically on GitHub.

There is currently a known issue on GitHub where all images are rendered inside of an anchor tag. As a result of this, decorative images result in links without an accessible name. Therefore, it's a good idea to always set alt text on markdown images on GitHub. However, this rule isn't really a long term solution and this issue is best fixed in GitHub in the long-term.

⚠️ This rule is SPECIFIC to GitHub's UI markdown behavior and is OFF by default since it's building on top of unideal behavior.

This rule should be removed in the future when we stops wrapping images in anchor tags.

We plan to enable this ingithub/accessibility-alt-text-bot#33.

khiga8 added10 commits

October 5, 2023 22:01

Make sure that HTML that is inlined is supported

0533b65

Add detail

1a5e09e

Update the helpers to support multiple errors in one line

0f707f9

add test support

8cf33cc

Add test cases

488fbb4

Update configg

111d3ac

Update export

012612f

Add new rule

09d02ff

add test cases and update docs

ec36605

Update test matchers

88e1852

khiga8 requested a review fromkendallgassner

October 6, 2023 14:10

khiga8 marked this pull request as ready for review

October 6, 2023 14:10

khiga8 requested a review froma team as acode owner

October 6, 2023 14:10

iansan5653 reviewed

Oct 6, 2023

View reviewed changes

docs/rules/GH003-no-empty-string-alt.md Outdated

		### Incorrect 👎

		```md
		![""](cat.png)

Copy link

Member

iansan5653Oct 6, 2023•
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure about this example. The alt text in the Markdown form is never quoted like an HTML property, so this would result in an image tag like<img alt='""' src="cat.png" />. While an alt text of"" is not useful, it's not empty either. The only way to create a truly empty alt tag in Markdown is with the HTML formalt="", so maybe we should only focus on HTML for this rule.

Copy link

Member

iansan5653Oct 6, 2023

Choose a reason for hiding this comment

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

I think auseful-alt-text rule might be an interesting avenue to explore, where we ban things like "screenshot", "image", "example", '""', etc. But I think that's a separate rule from empty text.

Copy link

ContributorAuthor

khiga8Oct 6, 2023

Choose a reason for hiding this comment

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

Ah! You're right -- thank you for catching that! I'll stick with HTML for this rule!

Copy link

ContributorAuthor

khiga8Oct 6, 2023

Choose a reason for hiding this comment

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

useful-alt-text

Theno-default-alt-text currently flags alternative text like,image,Image,Screen Shot 2022-06-26 at 7 41 30 PM though it's not comprehensive by any means. We could potentially expand on that?

Copy link

Member

iansan5653Oct 6, 2023

Choose a reason for hiding this comment

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

Yeah, I think we definitely could expand on that! It might be interesting to see if we can run a query to figure out what the most common alt texts are across GitHub -- I bet the top 20 or so are all things we'd probably want to lint against.

src/rules/no-empty-string-alt.js Outdated

Comment on lines 19 to 23

		const inlineImages = params.parsers.markdownit.tokens.filter(
		(token) =>
		token.type === "inline" &&
		token.children.some((child) => child.type === "image"),
		);

Copy link

Member

iansan5653Oct 6, 2023

Choose a reason for hiding this comment

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

Per other comment - we probably should limit this rule to just HTML tags

Copy link

ContributorAuthor

khiga8Oct 6, 2023

Choose a reason for hiding this comment

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

Removed markdown syntax in00fc5dd!

src/rules/no-empty-string-alt.js OutdatedShow resolvedHide resolved

test/no-empty-string-alt.test.js Outdated

		expect(results).toHaveLength(0);
		});
		});
		describe("failures", () => {

Copy link

Member

iansan5653Oct 6, 2023

Choose a reason for hiding this comment

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

Not super important, but it might also be useful to have a test for two failures in one line, like:

<imgsrc="cat.png"alt="" /> <imgsrc="dog.png"alt="" />

I think we do handle this correctly with theg flag and iterating over matches per line, but it probably would still be good to test it.

Copy link

ContributorAuthor

khiga8Oct 6, 2023

Choose a reason for hiding this comment

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

Good idea! Done in8f80d4d.

iansan5653 reviewed

Oct 6, 2023

View reviewed changes

src/rules/index.js Outdated

		rules: [
		require("./no-default-alt-text"),
		require("./no-generic-link-text"),
		require("./no-empty-string-alt"),

Copy link

Member

iansan5653Oct 6, 2023

Choose a reason for hiding this comment

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

🤔 Maybe we should call this oneno-empty-alt orno-empty-alt-text?

Copy link

ContributorAuthor

khiga8Oct 6, 2023

Choose a reason for hiding this comment

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

I likeno-empty-alt-text!

Copy link

ContributorAuthor

khiga8Oct 6, 2023

Choose a reason for hiding this comment

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

Renamed inf2d9774.

khiga8 marked this pull request as draft

October 6, 2023 16:17

khiga8and others added10 commits

October 6, 2023 16:37

Update src/rules/no-empty-string-alt.js

e725906

Co-authored-by: Ian Sanders <iansan5653@github.com>

Update src/rules/no-empty-string-alt.js

5b17abf

Co-authored-by: Ian Sanders <iansan5653@github.com>

Revert "Update src/rules/no-empty-string-alt.js"

ff25a34

This reverts commit5b17abf.Reverting because we are using this index.

Fix Regex syntax

9d75081

Remove markdown syntax support

00fc5dd

Update doc to remove markdown syntax

0ba6e3c

Add test case for multiple images in one line

8f80d4d

Related:#85 (comment)

Rename rule to no-empty-alt-text

f2d9774

Add missing bracket

6bf95bb

Update README with the rule

f11f800

khiga8 changed the title~~Introduce ano-empty-string-alt rule~~Introduce ano-empty-alt-text rule

Oct 6, 2023

khiga8 marked this pull request as ready for review

October 6, 2023 21:08

kendallgassner approved these changes

Oct 9, 2023

View reviewed changes

Copy link

Contributor

kendallgassner left a comment

Choose a reason for hiding this comment

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

Should we export a preset for GitHub markdown?

Base automatically changed fromkh-add-more-coverge tomain

October 9, 2023 14:55

iansan5653 approved these changes

Oct 9, 2023

View reviewed changes

docs/rules/GH003-no-empty-alt-text.md OutdatedShow resolvedHide resolved

src/rules/no-empty-alt-text.js OutdatedShow resolvedHide resolved

khiga8and others added2 commits

October 9, 2023 11:02

Update docs/rules/GH003-no-empty-alt-text.md

c60b8a7

Co-authored-by: Ian Sanders <iansan5653@github.com>

Update src/rules/no-empty-alt-text.js

0927174

Co-authored-by: Ian Sanders <iansan5653@github.com>

Copy link

ContributorAuthor

khiga8 commentedOct 9, 2023

Should we export a preset for GitHub markdown?

discussing over Slack!

khiga8 merged commitccc7d23 intomain

Oct 9, 2023

khiga8 deleted the kh-enforce-alt-is-always-set branch

October 9, 2023 15:04

janbrasna mentioned this pull request

Feb 25, 2024

Fix link to GH003#102

Merged

Labels

None yet

3 participants

Movatterモバイル変換

Introduce ano-empty-alt-text rule#85

Introduce ano-empty-alt-text rule#85

Uh oh!

Conversation

khiga8 commentedOct 6, 2023• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

iansan5653Oct 6, 2023• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kendallgassner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

khiga8 commentedOct 9, 2023

Uh oh!

Uh oh!

Introduce a`no-empty-alt-text` rule#85

Introduce a`no-empty-alt-text` rule#85

khiga8 commentedOct 6, 2023•
edited
Loading

iansan5653Oct 6, 2023•
edited
Loading