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 suggestion role #1134

Merged
merged 21 commits into from
Feb 13, 2020
Merged

Add suggestion role #1134

merged 21 commits into from
Feb 13, 2020

Conversation

aleventhal
Copy link
Contributor

@aleventhal aleventhal commented Dec 11, 2019

Relates to ARIA Annotations, issue #749


Preview | Diff

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Copy link
Contributor

@joanmarie joanmarie left a comment

Choose a reason for hiding this comment

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

In addition to addressing the wording-related comments and questions, please also add suggestion to the appropriate list under https://w3c.github.io/aria/#roles_categorization

Copy link
Contributor

@carmacleod carmacleod 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 the words are ok, but I think this role needs an example to fully explain it.
I had to look at the explainer to understand where the markup would go (it's embedded in the content).
I think the -cat/+dog code example from the explainer would be helpful here.

Copy link
Contributor

@carmacleod carmacleod left a comment

Choose a reason for hiding this comment

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

Thanks for the example, @aleventhal!
Looks good!
+1

@joanmarie
Copy link
Contributor

Looks good to me overall, but as I'm sitting here implementing support for this role in Orca, I have a question: Your text says:

The suggested change must be present as child content, as an insertion, a deletion, or one of each.

Can there be other children, like a paragraph explaining why the proposed insertion and/or deletion should be made? Or a time stamp? Or am I guaranteed to only find insertion and/or deletion children?

Arguably Orca should handle both scenarios because authors. But it would be nice to know (and perhaps see text in the spec stating) what the right usage looks like. Also, if we do want to prevent authors from putting non-insertion/deletion children in a suggestion, adding this statement to the spec should then cause it to be checked by validators.

@aleventhal
Copy link
Contributor Author

@joanmarie PTAL, I updated the text to make this more clear.

Looks good to me overall, but as I'm sitting here implementing support for this role in Orca, I have a question: Your text says:

The suggested change must be present as child content, as an insertion, a deletion, or one of each.

Can there be other children, like a paragraph explaining why the proposed insertion and/or deletion should be made? Or a time stamp? Or am I guaranteed to only find insertion and/or deletion children?

Arguably Orca should handle both scenarios because authors. But it would be nice to know (and perhaps see text in the spec stating) what the right usage looks like. Also, if we do want to prevent authors from putting non-insertion/deletion children in a suggestion, adding this statement to the spec should then cause it to be checked by validators.

index.html Outdated Show resolved Hide resolved
Co-Authored-By: Carolyn MacLeod <[email protected]>
Copy link
Contributor

@joanmarie joanmarie 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! Thanks for adding clarity regarding other possible children!!

Copy link
Contributor

@mcking65 mcking65 left a comment

Choose a reason for hiding this comment

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

No big concerns, but I think it is very important that the normative language be specific, especially wrt the responsible party. Most of my suggestions are related to that concern.

index.html Outdated Show resolved Hide resolved
index.html Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
aleventhal and others added 3 commits February 7, 2020 09:14
Co-Authored-By: Matt King <[email protected]>
Co-Authored-By: Matt King <[email protected]>
Co-Authored-By: Matt King <[email protected]>
@aleventhal
Copy link
Contributor Author

@mcking65 I think I got everything, but I needed to do some polish after as there was an incomplete sentence snippet that remained. Can you take a look?

index.html Outdated Show resolved Hide resolved
Co-Authored-By: Carolyn MacLeod <[email protected]>
Copy link
Contributor

@mcking65 mcking65 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. I just made one minor editorial suggestion.

index.html Outdated Show resolved Hide resolved
Co-Authored-By: Matt King <[email protected]>
@jnurthen jnurthen merged commit 2a047fd into master Feb 13, 2020
carmacleod pushed a commit that referenced this pull request May 7, 2020
* Relates to ARIA Annotations, issue #749
Co-Authored-By: Carolyn MacLeod <[email protected]>
Co-Authored-By: Matt King <[email protected]>
@pkra pkra added this to the ARIA 1.3 milestone Jan 10, 2022
@pkra pkra mentioned this pull request Jan 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants