fix: support native aria property reflection #6456
Conversation
| ariaExpanded: string | null; | ||
| ariaSelected: string | null; | ||
| } | ||
| } |
There was a problem hiding this comment.
This augments some of the aria properties as nullable. TypeScript marks them as string only but this is incorrect and causing issues when trying to set them as null. Once this is fixed in TS we can remove the overloaded Element interface.
This is impacting other Design systems as well
microsoft/TypeScript#46456
microsoft/TypeScript#45047
| } | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
This is a patch to support the aria property reflection for firefox
https://wicg.github.io/aom/caniuse.html
https://wicg.github.io/aom/aria-reflection-explainer.html
coryrylan
force-pushed
the
topic/core-aria-reflect
branch
from
07ee3c9 to
085b55d
Compare
| @@ -101,7 +101,8 @@ export class CdsInternalOverlay extends CdsBaseFocusTrap implements Animatable { | |||
| // renderRoot needs delegatesFocus so that focus can cross the shadowDOM | |||
| // inside an element with aria-modal set to true | |||
| /** @private */ | |||
| static get shadowRootOptions() { | |||
| static get shadowRootOptions(): any { | |||
| // any is used until TS 4.4.x adopted through other @cds/* libraries. Can be removed in 6.0 | |||
There was a problem hiding this comment.
I set these to any so the 5.x @cds/* libraries continue to work in the older TS verions. Once we branch for 6.x we can remove the any and use the updated types that TS provides.
There was a problem hiding this comment.
Zero concerns about leaving any here as long as we want.
| @@ -101,7 +101,8 @@ export class CdsInternalOverlay extends CdsBaseFocusTrap implements Animatable { | |||
| // renderRoot needs delegatesFocus so that focus can cross the shadowDOM | |||
| // inside an element with aria-modal set to true | |||
| /** @private */ | |||
| static get shadowRootOptions() { | |||
| static get shadowRootOptions(): any { | |||
| // any is used until TS 4.4.x adopted through other @cds/* libraries. Can be removed in 6.0 | |||
There was a problem hiding this comment.
Zero concerns about leaving any here as long as we want.
coryrylan
force-pushed
the
topic/core-aria-reflect
branch
from
83148f8 to
44c6a0e
Compare
| let ariaRegistered = false; | ||
|
|
||
| function isNode() { | ||
| return (globalThis as any)?.process?.env?.JEST_WORKER_ID !== undefined; // Jest and JSDom breaks on property reflection |
There was a problem hiding this comment.
hack: Jest throws since it doesn't seem to support aria properties correctly, once the shim is no longer required for firefox we can delete it
| @@ -111,6 +105,11 @@ export class CdsAlertGroup extends LitElement { | |||
| `; | |||
| } | |||
|
|
|||
| connectedCallback() { | |||
| super.connectedCallback(); | |||
| this.role = 'region'; | |||
There was a problem hiding this comment.
Other files we are just setting role = 'region' as a property on the class. Why are we doing it in connectedCallback here?
There was a problem hiding this comment.
I had it as a property as you describe but then hit issues with Jest, I can double check to see if the isNode check prevents the issue here
Signed-off-by: Cory Rylan <[email protected]>
coryrylan
force-pushed
the
topic/core-aria-reflect
branch
from
44c6a0e to
f7febaa
Compare
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
The newer aria reflection AP enables elements to set an aria property and have it auto reflect to the attribute.
This means we do not need the property decorators to handle the reflection on
aria*properties and need to adjust the types to match TypeScript in the newer version 4.3.4+. To support the newer versions of TS we need to adjust and remove overrides on any native aria property type. We also need a shim for firefox as it does not fully support aria property reflection yet.https://wicg.github.io/aom/caniuse.html
https://wicg.github.io/aom/aria-reflection-explainer.html
What is the new behavior?
Components can now use the native aria properties and fallback to a shim for Firefox. This aligns and fixes the mismatched types with TypeScript. See microsoft/TypeScript#46456 for more details
Unblocks part of the Angular 13 update #6455
Does this PR introduce a breaking change?
Other information