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

Certain props are not converted from camelCase to kebab-case when rendering #5477

Open
vincerubinetti opened this issue Feb 24, 2022 · 10 comments · May be fixed by #7412
Open

Certain props are not converted from camelCase to kebab-case when rendering #5477

vincerubinetti opened this issue Feb 24, 2022 · 10 comments · May be fixed by #7412
Assignees
Labels
🐞 bug Something isn't working

Comments

@vincerubinetti
Copy link

vincerubinetti commented Feb 24, 2022

Version

3.2.31

Reproduction link

github.com
https://archive.softwareheritage.org/browse/origin/directory/?origin_url=https://github.com/vincerubinetti/vue-aria-bug&timestamp=2022-02-24T05:36:51.635332%2B00:00

image

Steps to reproduce

Certain popular aria attributes like label and expanded get correctly converted from camelCase to kebab-case, but others like controls and haspopup do not.

What is expected?

For behavior to be consistent. Either for vue to convert all props back to kebab-case, or at least for it to convert all HTML supported props and not just some of them at random.

What is actually happening?

Only some props are being converted. The behavior is inconsistent and unintuitive.


Related issue:
vuejs/vue#11913

@vincerubinetti
Copy link
Author

vincerubinetti commented Feb 24, 2022

This is also especially problematic when trying to pass a lot of props at once, like this:

Component.vue:

<slot
  aria-label="test string"
  aria-multiselectable="test string"
  aria-expanded="test string"
  aria-controls="test string"
  aria-haspopup="test string"
  aria-activedescendant="test string"
>
</slot>

Using it:

<Component v-slot="props"><div v-bind="props"></div></Component>

Because Vue converts all of those kebab-cases to camelCase when passing it down, but then it only transforms some of them back to kebab-case when spreading them onto the slot element.

As a result, I have to manually run a toKebabCase function on all the v-bind props every time I use the component, which is cumbersome, tedious, and unexpected.

@ygj6 ygj6 added 🐞 bug Something isn't working and removed 🐞 bug Something isn't working labels Feb 24, 2022
@ygj6
Copy link
Member

ygj6 commented Feb 24, 2022

In fact,The ariaHasPopup property of the Element interface reflects the value of the aria-haspopup attribute.So you need use aria-has-popup.

@LinusBorg
Copy link
Member

LinusBorg commented Feb 24, 2022

Not quite. I'd still rate this a bug.

Why are some attributes not hyphenated?

Because we currently don't actively hyphenate attribute values in the patch phase. Usually that's not an issue as people normally use hyphenated attribute names in templates, but it can be an issue when i.e. passing camelCased props in an object to vbind.

Then why are some attributes hyphenated, and others not?

Because Vue prefers to set them via their element property counterparts if they exist. el.ariaLabel does exist, so Vue uses this to set it, which is then reflected as the attribute aria-label by the browser.

But ariaHaspopup does not exist - the property is named el.ariaHasPopup (which is inconsistent as the attribute is indeed called aria-haspopup). so this would work again right now:

<div :ariaHasPopup="true" />

so what's the solution?

The simple solution would be to enforce hyphenation when we add attributes (unless we are in SVGs which can have camelCase attributes).

@LinusBorg LinusBorg added the 🐞 bug Something isn't working label Feb 24, 2022
@LinusBorg LinusBorg self-assigned this Feb 24, 2022
@LinusBorg LinusBorg changed the title Certain aria props are not converted from camelCase to kebab-case when rendering Certain props are not converted from camelCase to kebab-case when rendering Feb 24, 2022
@vincerubinetti
Copy link
Author

vincerubinetti commented Feb 24, 2022

Thank you for the information. It sounds like part of the issue is just the inconsistency of the spec itself. But yeah, if Vue is going to try to do this casing stuff "automagically", I would expect it to at least handle all the standard attributes.


So you need use aria-has-popup.

This of course won't work because linters will throw a fit:

image

And I'm not going to disable that rule or add a bunch of manual attribute exceptions or something. Also, adding an extra kebab to attributes that aren't there in the spec is just asking for trouble down the line. Someone's gonna look at that code, think it's a mistake, rename it to the correct HTML attribute, then cause bad silent errors that latently and inexplicably wreak havoc.


But I was going to say, it sounds like I can just specify attribute names in camelCase as a stopgap. At least that would bring attention to future people viewing my code that this is a special case and you should leave it alone.

But then I tried it and it doesn't seem to work for all of my properties:

<slot
  :id="`select-${id}`"
  role="combobox"
  :ariaLabel="name"
  ariaMultiSelectable="true"
  :ariaExpanded="expanded"
  :ariaControls="`list-${id}`"
  ariaHasPopup="listbox"
  :ariaActiveDescendant="`option-${id}-${highlighted}`"
></slot>
<button id="select-16" role="combobox" aria-label="Filter by Score" aria-multiselectable="true" aria-expanded="false" ariacontrols="list-16" aria-haspopup="listbox" ariaactivedescendant="option-16-0"></button>

(Note that I tried all different combinations of kebab-case, capitalizations, event making the props dynamic vs static. None worked to fix activedescendant or controls).

So it seems like I have to stick with my dirty kebabify function solution for now.

@LinusBorg
Copy link
Member

LinusBorg commented Feb 24, 2022

it seems activedescendant and controls don't have matching element properties, so here my second point from above doesn't take effect.

A question: why don't you provide them in kebap-case in the first place (until we have fixed this)? Are they set up in JS where it's cumbersome to define hyphenated properties on an object?

@lidlanca
Copy link
Contributor

This might be a simpler work around for the slot binding

it should also still work after the proposed pr is merged

<slot
  aria--label="test string"
  aria--multiselectable="test string"
  aria--expanded="test string"
  aria--controls="test string"
  aria--haspopup="test string"
  aria--activedescendant="test string"
>
</slot>

probably not linter friendly

@yyx990803
Copy link
Member

The camel-casing of slot props was introduced for behavior consistency with Vue 2, and unfortunately cannot be changed due to its breaking nature.

A possible workaround is to avoid camel-casing props that start with aria-* or data-*. These are the main possible edge cases for the issue we have here, because absolute majority of native HTML attributes do not contain hyphens. with the exception of accept-charset and http-equiv. The downside is that this still leaves theoretical edge cases, and does not cover SVG attributes (which can be both kebab-case and camelCase).

@LinusBorg
Copy link
Member

I sent a draft PR to solve this. There's still a few things to talk/think through though (hence a draft).

@paul-thebaud
Copy link

Hello,
I'm sorry to ping this issue again, but I would like to know: is the PR review planned soon?

@paul-thebaud
Copy link

Hello @yyx990803 @LinusBorg, I'm here again for a reminder (sorry).
Could you please inspect this issue? Looks like the work is done, only a review is required.
This will fix or avoid major issues with some frameworks passing attributes as camel case strings.
Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment