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

Rule update: "aria-hidden with focusable content" #439

Merged
merged 8 commits into from
Apr 10, 2019

Conversation

annethyme
Copy link
Collaborator

@annethyme annethyme commented Mar 14, 2019

"focusable" --> "Participate in sequential focus navigation"

Closes issues:

"contain" --> "have descendants in the flat tree"

Closes issue:

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”.

"focusable" --> "Participate in sequential focus navigation", "contain" --> "have descendants in the flat tree"
@@ -26,7 +26,7 @@ The rule applies to any element with an `aria-hidden="true"` attribute.

### Expectation

None of the target elements are [focusable](#focusable), nor do they contain a [focusable element](#focusable).
None of the target elements participate in [sequential focus navigation](https://www.w3.org/TR/html/editing.html#sec-sequential-focus-navigation), nor do they have [descendants](https://www.w3.org/TR/dom41/#concept-tree-descendant) in the [flat tree](https://drafts.csswg.org/css-scoping/#flat-tree) that participate in [sequential focus navigation](https://www.w3.org/TR/html/editing.html#sec-sequential-focus-navigation)
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing stop at the end.

@annethyme annethyme dismissed kasperisager’s stale review March 18, 2019 14:58

Fixed. Please review again.

Rules Progress automation moved this from Needs Review to CFC (Reviewer Approved) Mar 19, 2019
Copy link
Collaborator

@jeeyyy jeeyyy left a comment

Choose a reason for hiding this comment

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

👍

@annethyme annethyme changed the title Update rule: "aria-hidden with focusable content" Rule update: "aria-hidden with focusable content" Mar 20, 2019
@annethyme
Copy link
Collaborator Author

@audreymaniez@JKODU, @kasperisager, @ShadowBB:

After some internal discussions, I have added a test case with tabindex="-2".

I hope you can still approve.

Rules Progress automation moved this from CFC (Reviewer Approved) to Needs Review Mar 23, 2019
Copy link
Member

@carlosapaduarte carlosapaduarte left a comment

Choose a reason for hiding this comment

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

Typo in the applicability: decendent > descendent

Copy link
Collaborator

@audreymaniez audreymaniez left a comment

Choose a reason for hiding this comment

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

Great!

@annethyme annethyme dismissed carlosapaduarte’s stale review March 25, 2019 10:11

Typo fixed. Please review again.

Rules Progress automation moved this from Needs Review to CFC (Reviewer Approved) Mar 25, 2019
@DagfinnRomen
Copy link
Collaborator

What is the difference between this (#439) and #449?

@annethyme annethyme added Review call 2 weeks Call for review for new rules and big changes and removed Agenda item labels Mar 26, 2019
Copy link
Member

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

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

Not sure i'm keen on the wording of "participate in sequential focus navigation". I'm open to a plain language improvement there, but besides that, all good.

@annethyme
Copy link
Collaborator Author

@DagfinnRomen, #439 and #449 solve two different issues for the same rule. They should not be seen as competing suggestions. In an ideal world, it would have been great if they were handled together, but since there are no conflicts between the two, I think it works fine to review and merge them separately.

@annethyme
Copy link
Collaborator Author

@WilcoFiers, I have changed "participate in [sequential focus navigation]" to "are part of [sequential focus navigation]" in hope to make it a bit easier to read.

Anne Thyme and others added 2 commits April 10, 2019 13:59
* master:
  Rule update: "HTML page has a title" (#440)
  Rule update: "aria attribute allowed" and "aria required states and properties" (#436)
  Changed succescriteria from 4.1.2 to 1.3.1 (#449)
  Rename SC1-2-Video-description-track.md to SC1-2-video-description-track.md
  Glossary: add "Accessibility Support" to "Semantic role" term (#442)
  SC1-1-1-filename-is-valid-accessible-name (#263)
  Added assumption for SC3-1-2-lang-valid (#413)
  Update SC4-1-1-unique-id.md
  fix: update applicability
  fix: revert definitions
  fix: update glossary
  fix: applicability
@kasperisager kasperisager merged commit c23409b into master Apr 10, 2019
Rules Progress automation moved this from CFC (Reviewer Approved) to Done Apr 10, 2019
@kasperisager kasperisager deleted the rule-update-SC4-1-2-aria-hidden-focus branch April 10, 2019 12:11
kasperisager added a commit that referenced this pull request Apr 26, 2019
* develop: (76 commits)
  Update SC1-1-1-image-has-name.md (#468)
  chore: update test case names to be hash and not have outcome (#485)
  chore: update styles
  chore: fix styling and spacing on various resolutions
  chore: fix typo in rules
  chore: update dependencies
  chore: add unique id to all rules (#478)
  fix: move footer from nav
  fix: update cache version
  fix: bust dependency cache
  chore: WCAG ACT RULES CG Website Update (#437)
  chore: update site publish config
  Replace auto-wcag with act-r in readme
  Update _config.yml
  Rule update: "aria-hidden with focusable content" (#439)
  Rule update: "HTML page has a title" (#440)
  Rule update: "aria attribute allowed" and "aria required states and properties" (#436)
  Changed succescriteria from 4.1.2 to 1.3.1 (#449)
  Rename SC1-2-Video-description-track.md to SC1-2-video-description-track.md
  Glossary: add "Accessibility Support" to "Semantic role" term (#442)
  ...
@annethyme annethyme removed this from Published in Rules Progress Jul 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Review call 2 weeks Call for review for new rules and big changes Rule Update Use this label for an existing rule that is being updated Rule Use this label for a new rule that does not exist already
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants