-
Notifications
You must be signed in to change notification settings - Fork 125
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
Issue899 accname from heading #1018
Conversation
I don't understand the use case for including The same is true if the container with a role of The proposal here says that the first descendant element with a role of
Would both the |
I think the issue of nesting of elements that can receive accessible name by heading needs discussion. In general it seems that a heading should only provide an accessible name for one element. So in your case only the |
to @LJWatson's point about It's not in the listing of elements in the issue? and seems |
@jongund said:
I agree about being conservative with this. Whatever route is taken, I think the spec text needs to be unambiguous on the matter though. |
@LJWatson @scottaohara @jnurthen @mcking65 I added a restrictions for naming only the closest ancestor of elements that can be named with headings to address the nesting issue. I also added |
Apologies @jongund and @mcking65, i was completely mistaken on thinking an element with So now, as Matt did, I'm questioning my ask to include |
@cookiecrook Seems like we could use two potentially different definitions for name from heading:
|
Option 1 is way too prescriptive and will not solve the issue for most of the suggested use cases. As one anecdotal example, Facebook articles, when they use headings (many FB articles do not), never place them as the first child of the article. Often, even the first leaf node with content is also not a heading... It's an image, or a link, or another control. Option 2 is a little better, but "~first heading" would be better than "~nearest in the DOM hierarchy." For example: article
|
Also, first heading may be more performant than nearest by depth, since the latter requires loading more content to calculate the comparison. |
… landmakrs and other roles
@LJWatson @scottaohara @cookiecrook Updated name from heading to account for first child requirement for landmark roles and more general requirement for other roles. |
Trying to get a clean diff with all the previously discussed changes, before recycling all the review requests.
@scottaohara @accdc This may be ready for re-review now. |
I think it needs all the encapsulation and legend stuff pulling out of it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some suggested edits and notes.
I don't GitHub is properly showing all the changes for the index file though. So I'm going to need to do another pass on this.
@jnurthen @cookiecrook - I'm afraid GitHub is all wonky on this PR. There are a bunch of resolved comments with changes that @cookiecrook made that aren't showing up in the file diff for me, nor do they appear in the preview diff of the spec. Is this just an issue for me (user error?), or is this happening for others as well? As mentioned, I did my first pass based on what was available in the file diff this morning, but I think I need to come back to this later. I kinda wonder if a fresh PR is in order here detangle this and help mitigate the merge conflict of the aria.js file? |
I'm thoroughly confused by the code spaghetti and can't pick it apart either. If someone could please condense and resubmit this I would appreciate it. |
I have taken assignment and am working through that as time allows. I will consider the fresh PR suggestion. |
@scottaohara wrote:
That might be accurate. A lot of what I did was back out changes that were unnecessary, including all the whitespace diffs and redundant guidance/requirements that was already covered elsewhere. If you have a specific change that is still in the branch but not showing up in the diff, please let me know. That'd push me over to start a new PR entirely. Note: I've turned this back into a Draft PR until I resolve the merge conflicts. |
I tried rebasing this onto main but with the myriad of commits (especially stuff that was later reverted) I’m not sure that is a good idea. IMO closing this and creating a new PR would be the best course of action. |
Co-authored-by: Scott O'Hara <[email protected]>
Co-authored-by: Scott O'Hara <[email protected]>
This PR is abandoned in favor of #1860 and can be closed. |
Update 2023-Jan-25: This PR abandoned in favor of #1860 and can be closed.
No longer attached to w3c/accname#138 (Note: PR modified by @cookiecrook)
Proposed definition of accessible name from heading
Proposed list of roles that support name from heading
Non-Landmark Roles Authoring Techniques
alertdialog
role authoring requirementsarticle
role authoring requirementsdialog
role authoring requirementsLandmark Roles Authoring Techniques
complementary
role authoring requirementsnavigation
role authoring requirementsregion
role authoring requirements💥 Error: 500 Internal Server Error 💥
PR Preview failed to build. (Last tried on Jan 20, 2021, 10:59 PM UTC).
More
PR Preview relies on a number of web services to run. There seems to be an issue with the following one:
🚨 Spec Generator - Spec Generator is the web service used to build specs that rely on ReSpec.
🔗 Related URL
If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.
Preview | Diff