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

Get rid of "Test/Aggregation Definition" #247

Merged
merged 15 commits into from
Sep 17, 2018
Merged

Conversation

annethyme
Copy link

@annethyme annethyme commented Aug 16, 2018

Suggesting a different split of Applicability/Expectations sections for Atomic/Composed rules to get rid of headings "Test Defintion" and "Aggregation Definition"

Closes #237


Preview | Diff

Suggesting a different split of Applicability/Expectations sections for Atomic/Composed rules to get rid of headings "Test Defintion" and "Aggregation Definition"
Anne Thyme added 3 commits August 16, 2018 15:06
Deleted some duplicate sections and moved up "Atomic rules list" to match the rule structure outline
Replaced words "aggregate"/"aggregated"/"aggregation" in context of composed rules, so that it is now only used in relation to ACT Data Format (Output Data)
Copy link
Collaborator

@moekraft moekraft left a comment

Choose a reason for hiding this comment

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

Hi Anne, Nice work. I wonder if the paragraph beginning with "A composed rule defines how the outcomes from its atomic rules are used to determine a single outcome for each applicable test target. ..." should be moved to the applicability section for composed rules as well. This would address Shadi's comment as well.

Anne Thyme added 2 commits August 16, 2018 16:54
Moved the paragraph "A composed rule defines how the outcomes from its atomic rules are used to determine a single outcome for each applicable test target. ..." from Rule Types section to Applicability for Composed Rules section as per Shadi's and Moe's comments, and deleted paragraph on disabling rules due to accessibility support concerns, since it already differs from what Siteimprove is working on.
@annethyme
Copy link
Author

@moekraft, thanks for the comment! Should be fixed now. It also handles Shadi's comment around the same thing.

@moekraft
Copy link
Collaborator

@annethyme Hi Anne, I ran the code through Bikeshed and found a broken target. Under Aspects under test there is a statement that reads [=Atomic rules=] MUST list the aspects used in the Test Definition. Since the heading Test Definition has been removed, this anchor is no longer valid. Can you take a look at the Aspects section. I think it needs updating to reflect your changes. Thanks much! Moe

@maryjom
Copy link
Collaborator

maryjom commented Aug 20, 2018

@annethyme I assigned to you so you will be notified to look at Moe's last comment posted 3 days ago.

@annethyme
Copy link
Author

@maryjom: I fixed 3 references to "test definition", so @moekraft's comment should be handled now.

#test-applicability changed to #applicability
Change #test-applicability to #applicability
Change #test-expectations to #expectations
@moekraft
Copy link
Collaborator

Diff of changes in PR https://services.w3.org/htmldiff?doc1=https%3A%2F%2Fw3c.github.io%2Fwcag-act%2Fact-rules-format.html&doc2=https://api.csswg.org/bikeshed/?url=https://raw.githubusercontent.com/w3c/wcag-act/82dd23f1d2e5494025791ae1a2a876b08d7f1162/act-rules-format.bs#applicability

Please note that I needed to resolve conflicts with the updates to change composed rule to composite rule. @annethyme Please ensure that I did not break your changes during the resolving of conflicts. Thanks, Moe

Anne Thyme added 2 commits September 12, 2018 12:48
Removed rogue line break
Fixed another rogue line break
@kengdoj
Copy link
Collaborator

kengdoj commented Sep 12, 2018

Suggest including that Rule Identifiers and Rule Descriptions are required in the Composite rules' Atomic Rule List under section 9.

Agree with @cpandhi about Example 5 under section 11.2.

With rule identifiers, Example 5 could be:
For each test target, the outcome of one of the following rules is passed :
Video elements have an audio description (video-audio-desc)
Video elements have a text alternative (video-text-alt)

Remove duplicate "Video elements have an audio description"
Add rule ids to composite rule example.
<ul>
      <li>Video elements have an audio description (video-audio-desc)</li>
      <li>Video elements have a media alternative (video-text-alt)</li>
    </ul>
@moekraft moekraft merged commit d8f13d7 into w3c:master Sep 17, 2018
moekraft added a commit that referenced this pull request Sep 17, 2018
Generating latest draft following merge of PR #242 and #247
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants