Skip to content

Conversation

WilcoFiers
Copy link
Member

@WilcoFiers WilcoFiers commented Jun 22, 2020

Basic rule checking that images have a descriptive accessible name. This rule is a counterpart to Image not in the accessibility tree is decorative.

Need for Final Call: 2 weeks

Copy link
Member

@carlosapaduarte carlosapaduarte left a comment

Choose a reason for hiding this comment

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

Mostly minor editorial changes. Good work!

Copy link
Collaborator

@EmmaJP EmmaJP left a comment

Choose a reason for hiding this comment

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

Subject to the changes Carlos has suggested, I'm happy for this to go ahead. I wondered if some of the inapplicable examples fail other rules and whether that should be acknowledged, but happy to leave that thought with the ACT-TF.

Jym77
Jym77 previously requested changes Jun 25, 2020
Copy link
Collaborator

@Jym77 Jym77 left a comment

Choose a reason for hiding this comment

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

Seconding @carlosapaduarte comments and adding a few more editorials.

name: Image accessible name is descriptive
rule_type: atomic
description: |
This rule checks that images with an accessible name describe the topic or purpose.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is grammatically incorrect.
The subject of "describe" is here "images with an accessible name" and not just "accessible name". But the rule checks that the accessible name describes the purpose of the image

Copy link
Collaborator

Choose a reason for hiding this comment

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

"the topic or purpose" could refer to something else on the page, not necessarily the subject of the sentence or the rule. English is wonderful for misinterpretation. "their topic or purpose" is less likely to be misinterpreted.

Co-authored-by: Carlos <carlosapaduarte@gmail.com>
Co-authored-by: Jean-Yves Moyen <jym@siteimprove.com>
@WilcoFiers WilcoFiers dismissed stale reviews from Jym77 and carlosapaduarte June 29, 2020 12:16

Updated, please review

Jym77
Jym77 previously requested changes Jun 29, 2020
name: Image accessible name is descriptive
rule_type: atomic
description: |
This rule checks that images with an accessible name describe their topic or purpose.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I still read the subject of "describe" as being "images with an accessible name" and that makes the sentence very weird (at best).

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @Jym77
My suggestion: "This rule checks that an image's topic or purpose is described by its accessible name."
Other suggestion: "This rule checks that the accessible name of an image describes the image's topic or purpose."

name: Image accessible name is descriptive
rule_type: atomic
description: |
This rule checks that images with an accessible name describe their topic or purpose.
Copy link
Member

Choose a reason for hiding this comment

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

I agree with @Jym77
My suggestion: "This rule checks that an image's topic or purpose is described by its accessible name."
Other suggestion: "This rule checks that the accessible name of an image describes the image's topic or purpose."


## Assumptions

This rule assumes that the language of each test target can be correctly determined (either programmatically or by analyzing the content).
Copy link
Member

Choose a reason for hiding this comment

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

Not really sure that this is needed. The rule checks that the accessible name serves an equivalent purpose to the non-text content. How is identifying the language related?

Also, technically, you want to determine the language of the accessible name, not of the test target (which is an image, and, unless it's an image of text, won't have a language)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not really sure that this is needed. The rule checks that the accessible name serves an equivalent purpose to the non-text content. How is identifying the language related?

<img src="bread.jpg" aria-label="pain" /> may be good or bad depending whether the language is French or English.

@WilcoFiers WilcoFiers dismissed stale reviews from carlosapaduarte and Jym77 June 29, 2020 14:58

updated, please check again

Copy link
Member

@carlosapaduarte carlosapaduarte left a comment

Choose a reason for hiding this comment

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

I'm okay with the updated description.
Also, the assumption won't stop me from approving the rule, but I really do think that it is the language of the accessible name that needs to be determined, not of the image. That's why I'm still requesting changes.

Co-authored-by: Carlos <carlosapaduarte@gmail.com>
@WilcoFiers WilcoFiers added Review call 2 weeks Call for review for new rules and big changes and removed reviewers wanted labels Jun 30, 2020
@WilcoFiers WilcoFiers merged commit f56a03a into develop Jul 17, 2020
@WilcoFiers WilcoFiers deleted the image-acc-name branch July 17, 2020 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Review call 2 weeks Call for review for new rules and big changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants