-
Notifications
You must be signed in to change notification settings - Fork 394
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(aom): consolidating the aria dom property names #192
Conversation
@@ -79,15 +79,13 @@ describe('def', () => { | |||
accessKey: null, | |||
ariaActiveDescendant: null, | |||
ariaAtomic: null, | |||
ariaAutocomplete: null, | |||
ariaAutoComplete: null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Casing was off!
ariaBusy: null, | ||
ariaChecked: null, | ||
ariaControls: null, | ||
ariaCurrent: null, | ||
ariaDescribedBy: null, | ||
ariaDisabled: null, | ||
ariaDragged: null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aria-grabbed
, this seems like a typo in the first place.
and aria-dropeffect
are deprecated by spec.
@@ -98,12 +96,12 @@ describe('def', () => { | |||
ariaLevel: null, | |||
ariaLive: null, | |||
ariaMultiSelectable: null, | |||
ariaMultiline: null, | |||
ariaMultiLine: null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Casing was off!
ariaOrientation: null, | ||
ariaOwns: null, | ||
ariaPosInSet: null, | ||
ariaPressed: null, | ||
ariaReadonly: null, | ||
ariaReadOnly: null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Casing was off!
@@ -113,6 +111,17 @@ describe('def', () => { | |||
ariaValueMin: null, | |||
ariaValueNow: null, | |||
ariaValueText: null, | |||
ariaColCount: null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all these were missing.
@ekashida does this affects the interop? I suspect that it does since they have a copy of this list. |
Benchmark comparisonBase commit:
|
Created a PR with the updates for interop. |
Benchmark comparisonBase commit:
|
@byao @davidturissini @pmdartus we might want to port this into the release branch today. |
* refactor(engine): avoid rewrapping of proxies * refactor(engine): prevent leaking double proxies via events or querySelector fix(aom): consolidating the aria dom property names (#192) wip: Remove slotting refactor(engine): conditinally use of native shadowRoot fix(engine): compat should also work with artificial shadow refactor(engine): adding slotting refactor(engine): adding slotting refactor(engine): conditionally piercing and default behaviors refactor(engine): minimizing the polyfilling on shadow-root to only AOM refactor(engine): implementing Pierre's suggestion to avoid patching shadow root prototype
Alignment with spec changes:
w3c/aria#708
Does this PR introduce a breaking change?
It changes a little bit the names of the Aria DOM Properties, and add few missing properties. This only affects folks who are either using any of the 3 changed properties in templates, or are trying to redefine them with get/set. The 3 properties are
ariaAutoComplete
,ariaMultiLine
andariaReadOnly
.Additionally, 2 deprecated properties were removed:
ariaDragged
(which had a typo anyway) andariaDropEffect
.