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

Expose ariaAllowedAttributes on ARIA Element #1721

Merged
merged 7 commits into from
Nov 29, 2024
Merged

Conversation

srnjcbsn
Copy link
Contributor

The logic is moved from R18 implementation and made public.

Should we also have a prohibitedAttributes() function on the Element type, for symmetry?

@srnjcbsn srnjcbsn requested a review from a team as a code owner November 28, 2024 10:42
Copy link

changeset-bot bot commented Nov 28, 2024

🦋 Changeset detected

Latest commit: 4701841

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

This PR includes changesets to release 78 packages
Name Type
@siteimprove/alfa-rules Minor
@siteimprove/alfa-aria Minor
@siteimprove/alfa-act Minor
@siteimprove/alfa-affine Minor
@siteimprove/alfa-applicative Minor
@siteimprove/alfa-array Minor
@siteimprove/alfa-bits Minor
@siteimprove/alfa-branched Minor
@siteimprove/alfa-cache Minor
@siteimprove/alfa-callback Minor
@siteimprove/alfa-cascade Minor
@siteimprove/alfa-clone Minor
@siteimprove/alfa-collection Minor
@siteimprove/alfa-comparable Minor
@siteimprove/alfa-compatibility Minor
@siteimprove/alfa-continuation Minor
@siteimprove/alfa-css-feature Minor
@siteimprove/alfa-css Minor
@siteimprove/alfa-device Minor
@siteimprove/alfa-dom Minor
@siteimprove/alfa-earl Minor
@siteimprove/alfa-either Minor
@siteimprove/alfa-emitter Minor
@siteimprove/alfa-encoding Minor
@siteimprove/alfa-equatable Minor
@siteimprove/alfa-flags Minor
@siteimprove/alfa-fnv Minor
@siteimprove/alfa-foldable Minor
@siteimprove/alfa-functor Minor
@siteimprove/alfa-future Minor
@siteimprove/alfa-generator Minor
@siteimprove/alfa-graph Minor
@siteimprove/alfa-hash Minor
@siteimprove/alfa-http Minor
@siteimprove/alfa-iana Minor
@siteimprove/alfa-iterable Minor
@siteimprove/alfa-json-ld Minor
@siteimprove/alfa-json Minor
@siteimprove/alfa-lazy Minor
@siteimprove/alfa-list Minor
@siteimprove/alfa-map Minor
@siteimprove/alfa-mapper Minor
@siteimprove/alfa-math Minor
@siteimprove/alfa-monad Minor
@siteimprove/alfa-network Minor
@siteimprove/alfa-option Minor
@siteimprove/alfa-parser Minor
@siteimprove/alfa-performance Minor
@siteimprove/alfa-predicate Minor
@siteimprove/alfa-promise Minor
@siteimprove/alfa-record Minor
@siteimprove/alfa-rectangle Minor
@siteimprove/alfa-reducer Minor
@siteimprove/alfa-refinement Minor
@siteimprove/alfa-result Minor
@siteimprove/alfa-sarif Minor
@siteimprove/alfa-selective Minor
@siteimprove/alfa-selector Minor
@siteimprove/alfa-sequence Minor
@siteimprove/alfa-set Minor
@siteimprove/alfa-slice Minor
@siteimprove/alfa-string Minor
@siteimprove/alfa-style Minor
@siteimprove/alfa-table Minor
@siteimprove/alfa-test-deprecated Minor
@siteimprove/alfa-test Minor
@siteimprove/alfa-thenable Minor
@siteimprove/alfa-thunk Minor
@siteimprove/alfa-time Minor
@siteimprove/alfa-toolchain Minor
@siteimprove/alfa-trampoline Minor
@siteimprove/alfa-tree Minor
@siteimprove/alfa-trilean Minor
@siteimprove/alfa-tuple Minor
@siteimprove/alfa-url Minor
@siteimprove/alfa-wcag Minor
@siteimprove/alfa-web Minor
@siteimprove/alfa-xpath 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

@srnjcbsn srnjcbsn force-pushed the expose-aria-allowed-attr branch from b9ce81c to d681986 Compare November 28, 2024 10:43
Copy link
Contributor

@Jym77 Jym77 left a comment

Choose a reason for hiding this comment

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

Looks great. A few nitpicking code style comments.

.changeset/twenty-boxes-complain.md Outdated Show resolved Hide resolved
packages/alfa-aria/src/node/element.ts Outdated Show resolved Hide resolved
packages/alfa-aria/src/node/element.ts Outdated Show resolved Hide resolved
@Jym77
Copy link
Contributor

Jym77 commented Nov 28, 2024

Should we also have a prohibitedAttributes() function on the Element type, for symmetry?

No. YAGNI. We'll move it when we need it…

@Jym77
Copy link
Contributor

Jym77 commented Nov 28, 2024

The Integrate workflows fails because it detects change in the API (the new method) that has not been registered in the docs.

You can extract the new API by using yarn extract packages/alfa-aria and committing the changed file. It can also be done automagically from the PR by commenting !pr extract (note do not push before that workflow ends, or it will fail to push its results…) I'll do that in the next comment.

@Jym77
Copy link
Contributor

Jym77 commented Nov 28, 2024

!pr extract

Move the logic from 'r18/rule.ts' into the alfa-aria package.
@srnjcbsn srnjcbsn force-pushed the expose-aria-allowed-attr branch from d681986 to e7d8b49 Compare November 28, 2024 13:33
Co-authored-by: Jean-Yves Moyen <[email protected]>
@srnjcbsn
Copy link
Contributor Author

The Integrate workflows fails because it detects change in the API (the new method) that has not been registered in the docs.

You can extract the new API by using yarn extract packages/alfa-aria and committing the changed file. It can also be done automagically from the PR by commenting !pr extract (note do not push before that workflow ends, or it will fail to push its results…) I'll do that in the next comment.

I ran it locally and force pushed approximately when you were writing that comment, I think (hadn't noticed you had started reviewing, so I thought I could sneak it in)

@srnjcbsn srnjcbsn force-pushed the expose-aria-allowed-attr branch from e8b53d7 to 4bcaf87 Compare November 28, 2024 14:08
@srnjcbsn srnjcbsn requested a review from Jym77 November 29, 2024 10:07
@srnjcbsn srnjcbsn added this pull request to the merge queue Nov 29, 2024
Merged via the queue into main with commit 53e9682 Nov 29, 2024
4 checks passed
@srnjcbsn srnjcbsn deleted the expose-aria-allowed-attr branch November 29, 2024 13:37
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.

2 participants