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 Accessibility considerations concept #5077

Merged
merged 18 commits into from
Dec 16, 2019
Merged

Conversation

carmacleod
Copy link
Contributor

@carmacleod carmacleod commented Nov 11, 2019

  • definition
  • for anchor element
  • for input element types
  • for button element

@domenic @annevk
Please note that I have created this as a Draft PR.

Questions & notes:

  • As suggested in Add accessibility role mappings to element definitions #3282 (comment), this only adds Accessibility considerations for input, anchor, and button.
  • It also adds the Accessibility considerations concept definition, which I took almost verbatim from dom.html#wai-aria. :)
  • I'm not sure how you want to do the links. I just used anchors. I looked at maybe using data-x-href but nothing is being defined here, so that's not a good fit.
  • I noticed that existing links to the ARIA, ARIAHTML and HTMLAAM specs point to the draft versions on w3c.github.io, and not the published specs at www.w3.org. Just confirming that drafts are preferred?
  • I did not adhere strictly to the 100 char maximum at this time, only so that it is easier for me see what to edit. I will update the line lengths before marking as ready to merge.

/acknowledgements.html ( diff )
/canvas.html ( diff )
/dom.html ( diff )
/edits.html ( diff )
/embedded-content.html ( diff )
/form-elements.html ( diff )
/forms.html ( diff )
/grouping-content.html ( diff )
/iframe-embed-object.html ( diff )
/image-maps.html ( diff )
/input.html ( diff )
/interactive-elements.html ( diff )
/media.html ( diff )
/scripting.html ( diff )
/sections.html ( diff )
/semantics.html ( diff )
/tables.html ( diff )
/text-level-semantics.html ( diff )

- definition
- for anchor element
- for input element types
- for button element
@annevk
Copy link
Member

annevk commented Nov 12, 2019

Draft URLs are preferred, looks okay to me overall.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

This looks so great! Only some minor wording nits, but I'm happy for the work to continue, using this as a template for all the other elements.

source Show resolved Hide resolved
source Show resolved Hide resolved
source Outdated Show resolved Hide resolved
@annevk annevk added the accessibility Affects accessibility label Nov 14, 2019
- remove "on HTML elements" from definition (not needed)
- lowercase "accessibility" in definition
- for anchor, use "If the element has an href attribute:" for consistency with Categories prose
- format lines for 100 char maximum
@carmacleod
Copy link
Contributor Author

carmacleod commented Dec 9, 2019

@domenic @annevk
I've temporarily removed 3 lines of code because their links are not yet in HTML-AAM:

  • line 26446:
    ; <a href="https://w3c.github.io/html-aam/#el-img-empty-alt">for implementers</a>
  • line 16390:
    <dd><a href="https://w3c.github.io/html-aam/#el-hgroup">For implementers</a>.</dd>
  • line 60420:
    <dd><a href="https://w3c.github.io/html-aam/#el-slot">For implementers</a>.</dd>

These links will be good when the following PRs are merged; but merging is currently waiting on an unrelated respec problem:

So I'm marking this as ready for review now because I don't want to hold up this PR.

As soon as the 3 links go in, I'll create a new PR to add the above 3 lines back in.

@carmacleod carmacleod marked this pull request as ready for review December 9, 2019 04:39
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

This looks amazing; thank you so much @carmacleod. I'll hold off merging so you can add those three lines back in? But after you do I'm happy to merge.

@carmacleod
Copy link
Contributor Author

carmacleod commented Dec 14, 2019

The 3 lines are back in, and all links are valid. Ok to merge! Thanks for all the help!
Closes #3282

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility Affects accessibility
Development

Successfully merging this pull request may close these issues.

3 participants