feat(core): render ARIA property bindings as attributes #62630
Conversation
Good suggestion, thanks. Done! |
Allow binding to ARIA attributes using property binding syntax _without_ the `attr.` prefix. For example, `[aria-label]="expr"` is now valid, and equivalent to `[ariaLabel]="expr"`. Both examples bind to either a matching input or the `aria-label` HTML attribute, rather than the `ariaLabel` DOM property. Binding ARIA properties as attributes will ensure they are rendered correctly on the server, where the emulated DOM may not correctly reflect ARIA properties as attributes. Reuse the DOM schema registry from the compiler to map property names in type check blocks.
| @@ -17,6 +17,8 @@ import { | |||
| } from '../compilation'; | |||
| import * as ng from '../instruction'; | |||
|
|
|||
| const ARIA_PREFIX = 'aria'; | |||
There was a problem hiding this comment.
optional: does having this as a constant meaningfully improve the code vs using using 'aria' and 4 inline? I don't imagine the prefix ever changing, so things would be a bit more concise without the indirection
(it's probably safe to assume a JS optimizer would do this, so it's probably not a performance concern, just a code style one, but it also wouldn't hurt to double check that)
There was a problem hiding this comment.
Yeah I'm not really familiar with the details of JSCompiler's optimizations so I favored avoiding repetition. For example, surely all 'aria' instances would be folded into the same constant, but what about 'aria-'? My guess is we'd then ship 'aria' and 'aria-' as separate strings, instead of just 'aria' and '-'.
Question Do we have any quick ways to evaluate the output?
Edit: I realize we might not be as concerned with the compiler (although I assume this is reused by the runtime compiler), there is a duplicate version of this function in core.
Edit: I recall that I had originally inlined the length and @crisbeto specifically requested I replace the magic number #62630 (comment)
| return name.charAt(ARIA_PREFIX.length) !== '-' | ||
| ? ARIA_PREFIX + '-' + name.slice(ARIA_PREFIX.length).toLowerCase() | ||
| : name; // Property already has attribute name. |
There was a problem hiding this comment.
| return name.charAt(ARIA_PREFIX.length) !== '-' | |
| ? ARIA_PREFIX + '-' + name.slice(ARIA_PREFIX.length).toLowerCase() | |
| : name; // Property already has attribute name. | |
| return name.startsWith('aria-') ? | |
| name : `aria-${name.slice(4).toLowerCase()}`; |
Is there any reason you couldn't just use startsWith here?
There was a problem hiding this comment.
A (undocumented) precondition of this function is that we know the name begins with aria, because that's the circumstance that leads to performing this mapping.
I'll document this precondition, but if you strongly prefer the startsWith check I can do that instead.
|
This PR was merged into the repository by commit 4138aca. The changes were merged into the following branches: main |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Allow binding to ARIA attributes using property binding syntax without the
attr.prefix. For example,[aria-label]="expr"is now valid, and equivalent to[ariaLabel]="expr". Both examples bind to either a matching input or thearia-labelHTML attribute, rather than theariaLabelDOM property.Binding ARIA properties as attributes will ensure they are rendered correctly on the server, where the emulated DOM may not correctly reflect ARIA properties as attributes.