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

feat: refresh mappings to latest editor's draft #937

Conversation

jlp-craigmorten
Copy link
Contributor

@jlp-craigmorten jlp-craigmorten commented May 26, 2023

Refreshes the package's mappings for roles (and some other role adjacent mappings) to align with:

(Following clarification of intended version support in #936 and PR comments)

Notable changes:

Other notes:

  • Other than an amended to a, area, and link implicit mapping, no other conditional implicit role mappings have been added. This means there are still outstanding edge-cases where the implicit role will be incorrect, e.g. the role for footer when not a child of a body.
  • There are some inconsistencies between specifications. This have been noted and have used "best judgement" / precedent to decide, happy to change on these.
  • I've not duplicated changes made in feat: support "none" role as a synonym for the "presentation" role #933, so the "none" role has not been introduced here.

@changeset-bot
Copy link

changeset-bot bot commented May 26, 2023

🦋 Changeset detected

Latest commit: 9eafad5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
dom-accessibility-api Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Owner

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

Very much appreciated. I think there was a misunderstanding that tracking accname editors draft implies tracking editors draft of every other spec. The other ones should still be stable. Only accname is draft.

That's what we've been doing and I don't want to change it until there's a concrete reason not to.

tests/wpt-jsdom/to-run.yaml Show resolved Hide resolved
sources/__tests__/accessible-name.js Outdated Show resolved Hide resolved
sources/__tests__/getRole.js Show resolved Hide resolved
sources/getRole.ts Outdated Show resolved Hide resolved
@cmorten
Copy link

cmorten commented May 28, 2023

Very much appreciated. I think there was a misunderstanding that tracking accname editors draft implies tracking editors draft of every other spec. The other ones should still be stable. Only accname is draft.

That's what we've been doing and I don't want to change it until there's a concrete reason not to.

Ah my bad! I’ll take a look next week at reverting back to "stable" specs (though this isn’t perhaps a clear definition, W3C works in a series drafts or recommendations, will use best effort unless you have particular preference on versions to use - even aria-query appears inconsistent on this A11yance/aria-query#498) for aria and aam (may mean a lot fewer changes) and will get round to ref’ing or raising issues for any of the inconsistencies I’ve found in appropriate places/repos.

Thanks for the review 😊

sources/getRole.ts Outdated Show resolved Hide resolved
@jlp-craigmorten
Copy link
Contributor Author

jlp-craigmorten commented May 31, 2023

Having some problems when update the wpt submodule:

  1. Updating DIR in wpt-jsdom with /manual and prefixing the testBasename with manual/ in web-platform-test.cy.js appears to cater for some of the directory changes correctly (but omits the new "non-manual" tests), and can confirm the Cypress tests pass.
  2. The latest WPT appears to hang when running wpt-jsdom with "server at ... is not up yet" errors, see https://dev.azure.com/silbermannsebastian/dom-accessibility-api/_build/results?buildId=15232&view=logs&j=cea8649a-b64b-5e01-19ae-73aebbebedd6&t=04d89b9a-d178-5019-810e-e64d2d0a606e which I'm not yet sure what the resolution is (happens locally for me as well).

For now reverting updating that submodule - where have indicated new WPT failures with comments there are no updates to those tests in latest WPT so think can separate doing that out independently (if happy with that)?

@jlp-craigmorten
Copy link
Contributor Author

@eps1lon think have resolved your comments - appreciate a re-review if / when have time.

@jlp-craigmorten jlp-craigmorten deleted the feat-refresh-to-editors-draft branch April 1, 2024 18:35
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.

3 participants