Skip to content

Conversation

Jym77
Copy link
Collaborator

@Jym77 Jym77 commented Apr 28, 2020

Update the applicability of "Image filename is accessible name for image" by:

  • only consider img elements, not anything with a role of img (where src has no meaning) (still keeping image buttons);
  • consider any image in the source set, not just the img attribute;
  • explicitly remove empty names that are causing weird issues.

Note: The change in applicability will invalidate all implementations. We'll need to get implementation data anew before re-submitting to TF.

Note: test are failing until #1284 is merged.

Closes issue(s):

Need for Final Call:
This will require a 1 week Final Call (updating applicability).


Pull Request Etiquette

When creating PR:

  • Make sure you're requesting to pull a branch (right side) to the develop branch (left side).

After creating PR:

  • Add yourself (and co-authors) as "Assignees" for PR.
  • Add label to indicate if it's a Rule, Definition or Chore.
  • Close the issue that the PR resolves (and make sure the issue is referenced in the top of this comment)
  • Optionally request feedback from anyone in particular by assigning them as "Reviewers".

How to Review And Approve

  • Go to the “Files changed” tab
  • Here you will have the option to leave comments on different lines.
  • Once the review is completed, find the “Review changes” button in the top right, select “Approve” (if you are really confident in the rule) or "Request changes" and click “Submit review”.
  • Make sure to also review the proposed Final Call period. In case of disagreement, the longer period wins.

@Jym77 Jym77 added the Rule Update Use this label for an existing rule that is being updated label Apr 28, 2020
@Jym77 Jym77 dismissed WilcoFiers’s stale review April 29, 2020 10:17

Example removed.

@Jym77 Jym77 requested a review from WilcoFiers April 29, 2020 10:17
Copy link
Contributor

@kasperisager kasperisager left a comment

Choose a reason for hiding this comment

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

I like it! 👏

Copy link
Member

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

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

Looks good, just please fix the failing test.

Copy link
Member

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

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

Editorial

@Jym77 Jym77 dismissed WilcoFiers’s stale review May 7, 2020 11:14

Tried to answer concerns.

@Jym77 Jym77 requested a review from WilcoFiers May 7, 2020 11:14
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.

Just a minor editorial to correct. Approving it already.

@Jym77 Jym77 dismissed WilcoFiers’s stale review May 12, 2020 13:03

Good suggestion!

@Jym77 Jym77 requested a review from WilcoFiers May 12, 2020 13:03
Copy link
Collaborator

@daniel-montalvo daniel-montalvo left a comment

Choose a reason for hiding this comment

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

The Task Force had an interesting meeting last week. We talked about the use of notes and how we could take notes to a minimum.
https://www.w3.org/2020/05/07-wcag-act-minutes.html

There are Several notes here.

  • First one: I think we could eliminate it, as this is a part of the spec that we are explaining. I am hesitant that we should be explaining these in the rules themselves.
  • The other ones I see them as nice to have, but I also think we can get rid of them. In my vview, what they are explaining is going to become evident once people take a look at the examples.

@Jym77
Copy link
Collaborator Author

Jym77 commented May 13, 2020

  • First one: I think we could eliminate it, as this is a part of the spec that we are explaining. I am hesitant that we should be explaining these in the rules themselves.

Yes. I added it because in the specs, the link from "source set" (at the start of 4.8.4.3) to the algorithm to update it (4.8.4.3.7) is not obvious. It took me some time to make that link myself, and I felt that linking only to "source set", while correct, is not really helping.

  • The other ones I see them as nice to have, but I also think we can get rid of them. In my vview, what they are explaining is going to become evident once people take a look at the examples.

Yes. I added them trying to prevent an upcoming "these are bad examples we shouldn't use them" issue.

I agree that they are not essential (basically, no note is essential, otherwise its content should not be in a note but in the normative text…) and will remove them.

@daniel-montalvo
Copy link
Collaborator

  • First one: I think we could eliminate it, as this is a part of the spec that we are explaining. I am hesitant that we should be explaining these in the rules themselves.

Yes. I added it because in the specs, the link from "source set" (at the start of 4.8.4.3) to the algorithm to update it (4.8.4.3.7) is not obvious. It took me some time to make that link myself, and I felt that linking only to "source set", while correct, is not really helping.

  • The other ones I see them as nice to have, but I also think we can get rid of them. In my vview, what they are explaining is going to become evident once people take a look at the examples.

Yes. I added them trying to prevent an upcoming "these are bad examples we shouldn't use them" issue.

I agree that they are not essential (basically, no note is essential, otherwise its content should not be in a note but in the normative text…) and will remove them.

Good! Will approve once done.

@Jym77 Jym77 requested a review from daniel-montalvo May 15, 2020 07:54
@Jym77
Copy link
Collaborator Author

Jym77 commented May 15, 2020

@daniel-montalvo I've removed the notes.

@Jym77
Copy link
Collaborator Author

Jym77 commented May 20, 2020

Final Call ends on May 20.

@Jym77 Jym77 added Review Call 1 week Call for review for small changes and removed reviewers wanted labels May 20, 2020
@Jym77
Copy link
Collaborator Author

Jym77 commented May 27, 2020

Final Call has ended. Merging.

@Jym77 Jym77 merged commit 94e01fb into develop May 27, 2020
@Jym77 Jym77 deleted the image-filename-feedback-applicability branch May 27, 2020 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Review Call 1 week Call for review for small changes Rule Update Use this label for an existing rule that is being updated

Projects

None yet

Development

Successfully merging this pull request may close these issues.

image filename, consider srcset / picture + source as well [9eb3f6] "Filename is valid accessible name" (9eb3f6) should limit its applicability

5 participants