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

Update aria-state-or-property-permitted-5c01ea.md #1612

Merged
merged 10 commits into from
Jun 24, 2021
Merged

Conversation

ajanec01
Copy link
Collaborator

@ajanec01 ajanec01 commented May 16, 2021

Trying to resolve the issue raised in #1573.

Not exactly sure if this is the right way so would appreciate the input.

Closes issue(s): #1573

Need for Final Call:
1 week

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.

@ajanec01 ajanec01 self-assigned this May 16, 2021
@ajanec01 ajanec01 added Review Call 1 week Call for review for small changes reviewers wanted and removed Review Call 1 week Call for review for small changes labels May 16, 2021
Each test target is either an [inherited][], [supported][], or [required][] [state][] or [property][] of the [semantic role][] of the element on which the attribute is specified. If the element has no [semantic role][], the attribute must be a [global state or property][global].

**Note:** Assessing the value of the attribute is out of scope for this rule.
Each test target is either an [inherited][], [supported][], or [required][] [state][] or [property][] for the element's [semantic role][] or the [WAI-ARIA state or property][] may be used on the language feature. If the element has no [semantic role][], the attribute must be a [global state or property][global].
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm a bit uneasy in leaving "language feature" as it and only explain it in a background note (versus trying to have a definition).
The rule is only targeting HTML and SVG, and I don't think there is a similar "ARIA in SVG" (or other feature document) out there. So maybe we can be a bit more prescriptive here (to make sure and remove the ambiguity). Pointing here to ARIA in HTML and saying that if the target is an HTML element, it may also have stuff described there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I thought about that too. In the end, I decided to leave the "language feature" for everybody else to decide on it. Yeah, I'm not sure if there ever will be an equivalent concept for SVG so we can tighten it.

Copy link
Member

Choose a reason for hiding this comment

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

I think you're trying to pack too much information into one sentence. It might be better to break this up into a list:

Suggested change
Each test target is either an [inherited][], [supported][], or [required][] [state][] or [property][] for the element's [semantic role][] or the [WAI-ARIA state or property][] may be used on the language feature. If the element has no [semantic role][], the attribute must be a [global state or property][global].
One of the following is true for each test target:
- **Global**: the test target is a [global state or property][global]; or
- **Semantic Role**: the test target is an [inherited][], [supported][], or [required][] [state][] or [property][] of the [semantic role][] of the element on which the test target is specified; or
- **language feature**: the test target is allowed as a language feature on the element on which it is specified. Which ARIA state or property may be used on which element is describe in [ARIA in HTML][].

This will also make it easier to add prohibited attributes when ARIA 1.2 makes it to rec.

Each test target is either an [inherited][], [supported][], or [required][] [state][] or [property][] of the [semantic role][] of the element on which the attribute is specified. If the element has no [semantic role][], the attribute must be a [global state or property][global].

**Note:** Assessing the value of the attribute is out of scope for this rule.
Each test target is either an [inherited][], [supported][], or [required][] [state][] or [property][] for the element's [semantic role][] or the [WAI-ARIA state or property][] may be used on the language feature. If the element has no [semantic role][], the attribute must be a [global state or property][global].
Copy link
Member

Choose a reason for hiding this comment

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

I think you're trying to pack too much information into one sentence. It might be better to break this up into a list:

Suggested change
Each test target is either an [inherited][], [supported][], or [required][] [state][] or [property][] for the element's [semantic role][] or the [WAI-ARIA state or property][] may be used on the language feature. If the element has no [semantic role][], the attribute must be a [global state or property][global].
One of the following is true for each test target:
- **Global**: the test target is a [global state or property][global]; or
- **Semantic Role**: the test target is an [inherited][], [supported][], or [required][] [state][] or [property][] of the [semantic role][] of the element on which the test target is specified; or
- **language feature**: the test target is allowed as a language feature on the element on which it is specified. Which ARIA state or property may be used on which element is describe in [ARIA in HTML][].

This will also make it easier to add prohibited attributes when ARIA 1.2 makes it to rec.

@ajanec01 ajanec01 dismissed WilcoFiers’s stale review May 31, 2021 22:56

Applied almost all suggesstion- only made changes to the last point.

@ajanec01 ajanec01 requested review from WilcoFiers and Jym77 May 31, 2021 22:57
Jym77
Jym77 previously requested changes Jun 3, 2021
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.

Looks good 👍

_rules/aria-state-or-property-permitted-5c01ea.md Outdated Show resolved Hide resolved
_rules/aria-state-or-property-permitted-5c01ea.md Outdated Show resolved Hide resolved
_rules/aria-state-or-property-permitted-5c01ea.md Outdated Show resolved Hide resolved
_rules/aria-state-or-property-permitted-5c01ea.md Outdated Show resolved Hide resolved
_rules/aria-state-or-property-permitted-5c01ea.md Outdated Show resolved Hide resolved
@ajanec01 ajanec01 requested a review from Jym77 June 3, 2021 16:08
@ajanec01 ajanec01 dismissed Jym77’s stale review June 3, 2021 16:08

All committed!

@ajanec01 ajanec01 added the Review Call 1 week Call for review for small changes label Jun 15, 2021
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 reviewers wanted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants