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

fix: amend breaking changes and reinstate constraints #515

Conversation

jlp-craigmorten
Copy link
Contributor

@jlp-craigmorten jlp-craigmorten commented Jun 17, 2023

Fixes #511
Fixes #512
Relates to testing-library/dom-testing-library#1239

Description

#447 brought in a number of breaking changes to this repo and was released under a minor version, namely:

  1. Removal of the constraints array within related concepts attributes.
  2. Additions / updates according to the unstable W3C Working Draft for HTML-AAM.

This PR looks to partially revert some of these changes (with a few other unrelated corrections and additions) to provide an upgrade path to downstream packages, using the following as references:

In some places I have left the HTML-AAM additions where they could be considered non-breaking additive changes in line with discussion here. Open to changes where feel should keep some of the HTML-AAM changes. I think the key is to resolve (1) which has broken the likes of dom-testing-library.

Summary of changes

  • Added @babel/node as a dev dep so npm run output_as_hack can be used.
  • Updated browserslist db as prompted by console, running npx update-browserslist-db@latest.
  • Removed section element association from section role as unable to find any references in any stable specs nor the HTML-AAM Working Draft.
  • Removed aria-controls constraint on combobox role to fix input type="search" isn't considered of role "searchbox" #511. This is new in HTML-AAM Working Draft, but not in WAI-ARIA 1.2 nor HTML-ARIA W3C Recommendations.
  • Correct to select element constraints involving multiple. I believe the spec has been misinterpreted as multiple being included in the condition regarding a number value. It is a boolean attribute so have corrected constraint wording.
  • Reinstated the constraints arrays for attributes, reverting it's removal in Audited and updated roles source of truth to HTML Accessibility API Mapping 1.0 #447. In some places additional attribute constraints were added to align to the spec.
  • Addition of 'ancestor table element has treegrid role' constraint to gridcell role: "role=gridcell if the ancestor table element is exposed as a role=grid or treegrid". See https://www.w3.org/TR/html-aria/#docconformance.
  • Reinstated the img element specific conditions on alt.
  • Removed aria-multiselectable constraint on datalist for listbox role. This is new in HTML-AAM Working Draft, but not in WAI-ARIA 1.2 nor HTML-ARIA W3C Recommendations.
  • Reinstated the ARIA listitem related concept to the menuitem role as per https://www.w3.org/TR/wai-aria-1.2/#menuitem.
  • Removed constraints on progressbar role for progress element. This is new in HTML-AAM Working Draft, but not in WAI-ARIA 1.2 nor HTML-ARIA W3C Recommendations.
  • Amended the button related concept for the switch role. The related concept is for the ARIA button not HTML, see https://www.w3.org/TR/wai-aria-1.2/#switch.

Update 27/06/2023:

Some of the comments above are incorrect in hindsight (see w3c/html-aria#476 (comment)), specifically regarding constraints or concepts being "new" in HTML-AAM - this was a misinterpretation of the specification which is designed for implementors, not for authors. I have striked through the incorrect comments. Going forward the aria and html in aria specs should be used, not aam.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 17, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 9a036ef:

Sandbox Source
Vanilla Configuration

@coveralls
Copy link

coveralls commented Jun 17, 2023

Pull Request Test Coverage Report for Build 5297714812

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 95.455%

Totals Coverage Status
Change from base Build 5250482787: 0.0%
Covered Lines: 248
Relevant Lines: 253

💛 - Coveralls

package.json Outdated Show resolved Hide resolved
Co-authored-by: Craig Morten <[email protected]>
Copy link
Member

@jessebeach jessebeach left a comment

Choose a reason for hiding this comment

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

I super appreciate the time and care put into this. Let's get this in and put out a minor release to revert the breakage. This is helping me better understand how downstream packages are using this package.

@MatanBobi
Copy link

Hi @jessebeach!
Next time if you want us (testing-library) to run tests prior to release (v6), please don't hesitate to reach out :)

@jessebeach
Copy link
Member

Thank you @MatanBobi! I will take you all up on this offer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants