Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings
/ariaPublic

Update required owned elements and required context role#1454

Merged
spectranaut merged 20 commits intomainfrom
req-owned-elements
May 1, 2023
Merged

Update required owned elements and required context role#1454
spectranaut merged 20 commits intomainfrom
req-owned-elements

Conversation

@smhigley
Copy link
Contributor

@smhigleysmhigley commentedApr 13, 2021•
edited by pr-previewbot
Loading

Resolves#1033

Updates "Required Owned Elements" to "Allowed Child Elements", and "Required Context Role" to "Required Parent Role", with added definitions for parent, child, controlled element, and controlling element.

I did some support tests for having intermediate elements in composite widgets, and secondary actions in composite widgets, here:https://cdpn.io/smhigley/debug/vYgLZgK

This is still a POC, so I didn't update all the references to required owned elements/context role. Hopefully that makes it a bit easier to see the relevant diffs for discussion.

Should alsoresolve#1486

PR merge check list

Preview |Diff

Preview |Diff

Copy link
Member

@scottaoharascottaohara left a comment

Choose a reason for hiding this comment

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

since this is still a draft, just a few comments/suggestions. hope they're helpful

@smhigley
Copy link
ContributorAuthor

I updated the PR based on the comments, updated all references to "required owned elements"/"required context role", and moved thearia-controls allowed child stuff to a separate branch. It should be ready for a full review now.

@smhigleysmhigley marked this pull request as ready for reviewApril 30, 2021 06:02
index.html Outdated
<p>If it is necessary to make multiple additions, modifications, or removals within a container element that is already either partially or fully rendered, authors MAY set <code>aria-busy</code> to <code>true</code> on the container element before the first change, and then set it to <code>false</code> when the last change is complete. For example, if multiple changes to a <a>live region</a> should be spoken as a single unit of speech, authors MAY set <code>aria-busy</code> to <code>true</code> while the changes are being made and then set it to <code>false</code> when the changes are complete and ready to be spoken.</p>
<p>If an element with role <rref>feed</rref> is marked busy, assistive technologies MAY defer rendering changes that occur inside the <code>feed</code> with the exception of user-initiated changes that occur inside the <rref>article</rref> that the user is reading during the busy period.</p>
<p>If changes to a rendered <rref>widget</rref> would create a state where the <rref>widget</rref> ismissing <a href="#mustContain">required owned elements</a> during script execution, authorsMUST set <code>aria-busy</code> to <code>true</code> on the <rref>widget</rref> during the update process. For example, if a rendered tree grid required a set of simultaneous updates to multiple discontiguous branches, an alternative to replacing the complete tree element with a single update would be to mark the tree busy while each of the branches are modified.</p>
<p>If changes to a rendered <rref>widget</rref> would create a state where the <rref>widget</rref> ismodifying <a href="#mustContain">allowed child roles</a> during script execution, authorsMAY set <code>aria-busy</code> to <code>true</code> on the <rref>widget</rref> during the update process. For example, if a rendered tree grid required a set of simultaneous updates to multiple discontiguous branches, an alternative to replacing the complete tree element with a single update would be to mark the tree busy while each of the branches are modified.</p>
Copy link
Member

Choose a reason for hiding this comment

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

Can we just remove this. Does setting aria-busy in this state actually do anything?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I'm happy to remove it, I just didn't want to muddy this PR with other changes if they'd be controversial 😅

@jnurthen
Copy link
Member

Please also move the new terms to index.html. The content of terms.html has been moved there. You'll need to pull the changes from main though to pick this up.

smhigley reacted with thumbs up emoji

@smhigley
Copy link
ContributorAuthor

All the previous comments should be addressed.@scottaohara /@aleventhal, how does it look now?

@smhigleysmhigley requested a review fromcynsMay 13, 2021 18:29
@smhigley
Copy link
ContributorAuthor

also pinging@jcsteh because for some reason github isn't letting me add him to the reviewers ¯_(ツ)_/¯

index.html Outdated
<p class="note">An element with a <a href="#subclassroles">subclass role</a> of the 'required owned element' does not fulfill this requirement. For example, the <rref>listbox</rref> rolerequires ownership of anelement using the <rref>option</rref> or <rref>group</rref> role. Although the <rref>group</rref> role is the superclass of <rref>row</rref>, addingan <a>owned</a>element with a role of <rref>row</rref> will not fulfill the requirement that <rref>listbox</rref>owns an<rref>option</rref> ora<rref>group</rref>.</p>
<h3>Allowed Child Roles</h3>
<p>Any <a>element</a> thatis a direct<a>child</a>of the element with this <a>role</a>. For example, an element with the role <rref>list</rref>may ownelements with the role <rref>listitem</rref>, but not elementswith the role <rref>option</rref>.</p>
<p>To determine whether an element is the direct <a>child</a> of a role, <a>user agents</a>MUST ignore any elements withtherole <rref>generic</rref> or <rref>presentation</rref>.</p>

Choose a reason for hiding this comment

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

It's definitely good that we're clarifying what happens concerning generics here. That will clear upimplementer confusion like this.

Again, this is clear if you read the definition of "child", but I wonder whether this could be misconstrued as discussing siblings rather than intervening (depth-wise) elements. Perhaps we could clarify this by saying "intervening elements" here?

Also, I assume this isn't a statement about whether intervening generics get exposed in the a11y tree or not, but rather, only a statement about what is considered valid (thus impacting implicit posinset/setsize calculation, etc.)? Pruning intervening generics would be a much bigger change for implementers which would need further discussion.

Copy link
ContributorAuthor

@smhigleysmhigleyJul 1, 2021•
edited
Loading

Choose a reason for hiding this comment

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

I updated to "intervening elements" -- thanks for the suggestion, I like that better :).

And yup! This is guidance on allowed role structure, and isn't prescriptive of API mappings. The only change is implementers would officially need to support e.g.listbox > generic > option (which most already do), but could do so with or without pruning the generic.

@smhigley
Copy link
ContributorAuthor

Just want to ping@jcsteh,@scottaohara, and@aleventhal on this to see if the updates have addressed past comments as desired :)

Copy link
Member

@scottaoharascottaohara left a comment

Choose a reason for hiding this comment

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

wrote these review comments some time ago... sorry for the delay.

@jnurthen
Copy link
Member

should also resolve#1486

@jnurthen
Copy link
Member

@smhigley would this PR take care of#1300 ?

@smhigley
Copy link
ContributorAuthor

@spectranaut,@scottaohara, and@adampage -- I updated with a lot of the suggestions you mentioned, and some comments. Let me know what you think!

adampage reacted with hooray emoji

Copy link
Contributor

@spectranautspectranaut left a comment

Choose a reason for hiding this comment

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

Looks great!

I don't think there are any changes to the validator tests, right, Sarah? Seems to me like this is all clarification/rewording of existing "author MUST" requirements.

smhigley reacted with heart emoji
@smhigley
Copy link
ContributorAuthor

@spectranaut I think the validator change would be checking something like this:

<div role="listbox">  <div>    <div role="option">hi I'm valid now</div>  </div></div>

@spectranaut
Copy link
Contributor

Oh right what was I thinking, we even talked about this. I just got so wrapped up in the "rewrite" and new terms I forgot about this key change XD

Do you want to do that here, or in a follow up issue? If a follow issue, please make one, otherwise, you can add a validator test here.

smhigley reacted with heart emoji

Co-authored-by: Adam Page <adamfpage@gmail.com>
@smhigley
Copy link
ContributorAuthor

@spectranaut issue is made!#1917

spectranaut reacted with hooray emoji

@spectranaut
Copy link
Contributor

@pkra or@jnurthen do you think we can merge this? :)

Copy link
Contributor

@spectranautspectranaut left a comment

Choose a reason for hiding this comment

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

I spoke too soon once again! The new CI check caught an errant "may", we need to fix that THEN I think it is ready for merge

pkra reacted with thumbs up emoji
Copy link
Member

@jnurthenjnurthen left a comment

Choose a reason for hiding this comment

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

I think a #mustContain was missed

Co-authored-by: James Nurthen <jnurthen@users.noreply.github.com>
@spectranautspectranaut dismissed stale reviews fromcookiecrook and themselfMay 1, 2023 21:51

outdated

@spectranautspectranaut merged commit232ae78 intomainMay 1, 2023
github-actionsbot added a commit that referenced this pull requestMay 1, 2023
SHA:232ae78Reason: push, by spectranautCo-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
jnurthen pushed a commit that referenced this pull requestOct 10, 2023
Updates "Required Owned Elements" to "Allowed Child Elements", and "Required Context Role" to "Required Parent Role", with added definitions for parent, child, controlled element, and controlling element.
@scottaoharascottaohara deleted the req-owned-elements branchJanuary 17, 2025 15:59
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@spectranautspectranautspectranaut left review comments

@jnurthenjnurthenjnurthen approved these changes

@scottaoharascottaoharascottaohara approved these changes

@jcstehjcstehjcsteh left review comments

@pkrapkrapkra approved these changes

@adampageadampageadampage approved these changes

@cookiecrookcookiecrookcookiecrook left review comments

@cynscynsAwaiting requested review from cyns

@aleventhalaleventhalAwaiting requested review from aleventhal

+2 more reviewers

@Jym77Jym77Jym77 left review comments

@WilcoFiersWilcoFiersWilcoFiers approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2023

Development

Successfully merging this pull request may close these issues.

Allowed children of role=tablist includes any role including tabpanel? Clarify "required owned element"

12 participants

@smhigley@jnurthen@pkra@WilcoFiers@cookiecrook@dd8@spectranaut@aleventhal@adampage@scottaohara@jcsteh@Jym77

Comments

[8]ページ先頭
©2009-2026 Movatter.jp