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

False value cause aria boolean attributes to be removed from the DOM #11053

Closed
falcon03 opened this issue Jan 27, 2020 · 13 comments
Closed

False value cause aria boolean attributes to be removed from the DOM #11053

falcon03 opened this issue Jan 27, 2020 · 13 comments

Comments

@falcon03
Copy link

Version

2.6.11

Reproduction link

https://www.dropbox.com/s/3ltqeent65djedw/problematic-false-aria-values.zip?dl=0

Steps to reproduce

  1. Extract the provided zip archive and enter the produced "problematic-false-aria-values" directory.
  2. Install dependencies (i.e. yarn install).
  3. Run the development server (i.e. yarn serve) and head to its url with your favorite browser.
  4. Inspect the DOM of the page: the aria-checked and aria-expanded attributes won't be there.
  5. Enable the checkbox and/or expand the toggle.
  6. Inspect the DOM of the page: this time both aria-checked and aria-expanded will be there as expected.
  7. Disable the checkbox and collapse the toggle.
  8. Inspect the DOM of the page: the aria-checked and aria-expanded attributes won't be there, again.

What is expected?

Aria attributes should be present in the DOM even if their value is false.

What is actually happening?

Aria attributes are not present in the DOM when their value is false.


Unlike for html 5 boolean values, removing aria attributes from the DOM when their value is false changes the page semantics and degrades its accessibility, causing markup validation issues as well.

@LinusBorg
Copy link
Member

LinusBorg commented Jan 27, 2020

I didn't check out the repro as i don't have access to dropbox.

But:

  • A value of false is the only way to tell Vue to actually remove an attribute - which you may also want to do with an aria attribute in some situtions.
  • aria-checkedvalues (like all html attribute values) are strings, not booleans: 'true' and 'false', respectively, so you have to set them as strings:

v-bind:aria-checked="checked ? 'true' : 'false'"

I don't think it makes sense to have aria attributes behave differently than other html attributes in this regard.

Also, we need a way to tell Vue "remove that attribute", which is setting it to false.

We're aware that this works a bit differently than vanilla js: setAttribute('aria-checked', false) will set the attribute to aria-checked="false". But so will it do for checked or any other element:

el.setAttribute('aria-selected', false)
<div aria-selected="false">

el.setAttribute('checked', false)
<div aria-selected="false" checked="false">

el.setAttribute('placeholder', false)
<div aria-selected="false" checked="false" placeholder="false">

So Vue treats false differently, but it does so consistently and for a reason.

@posva
Copy link
Member

posva commented Jan 27, 2020

For Aria attributes you have to cast them to string:

<button :aria-pressed="isToggled.toString()">

This is something I thought about but didn't take the time to make sure that

  1. All existing boolean aria attributes take true and false string values
  2. All new aria attributes will follow that rule

If those two points are true, I think it would make for aria attributes on HTML elements to be cast as strings

@falcon03
Copy link
Author

@LinusBorg thanks for your clarification. I do understand the reason behind this behavior, but I think that having to cast ARIA attributes to string manually is tedious and degrades the developer experience. In addition to this, even if they do appear as html boolean attributes conceptually, according to the W3C specification

The suggested mappings for true/false values in HTML 5 use Keyword and enumerated attributes with allowed values of true and false, instead of using the HTML 5 boolean value type.

Together with the fact that if the value is to true you get a statement like attr="value" instead of the preferred HTML 5 syntax, this is the reason why I think that Vue's behavior feels inconsistent.

Also, the fact that an ARIA attribute has the false value can mean something different than completely omitting the attribute (think of aria-expanded, just to mention the one I used in the demo).

@falcon03
Copy link
Author

falcon03 commented Jan 27, 2020

@posva thanks for providing a code example, this is the workaround I am using right now. I think that automatically casting ARIA attribute values to strings makes sense. Regarding your first point I think that the answer is yes (see my previous comment), while the answer to your second point is yes unless W3C suggests different type mappings.

@LinusBorg
Copy link
Member

Also, the fact that an ARIA attribute has the false value can mean something different than completely omitting the attribute (think of aria-expanded, just to mention the one I used in the demo).

That's kind of my point: if we cast false to 'false' for these category of attributes, then users have to be aware that to actually remove an aria attribute, they have to set it to undefined, whereas for all other attributes, setting it to false has the same effect.

But yeah I can see how casting would be helpful in practice. Will reopen just to discuss it further

@LinusBorg LinusBorg reopened this Jan 27, 2020
@Josh68
Copy link

Josh68 commented Apr 21, 2020

I was just looking at an older, closed issue on the same topic: #7422 (comment).

What about just addressing this in the docs? Maybe specifically calling out bindings of aria- attributes.

@jfbrennan
Copy link

jfbrennan commented Jun 4, 2021

This behavior was actually changed in Vue 3, unfortunately, and should be immediately reverted. (See https://v3.vuejs.org/guide/migration/attribute-coercion.html#overview)

I came here looking to file an issue because upgrading to v3 broke most of my templates due to this change. I saw this issue was still open, so I'm adding my feedback.

With Vue 3 it is now a HUGE pain for developers to get boolean attribute bindings working correctly as our code now has to reassign completely legitimate false values to null, which is not only awkward and incorrect programming, but makes for more verbose and ugly code. As much as I love and support Vue, this was a big oversight that needs to be reverted before v3 adoption hits critical mass.

Two use cases to illustrate:
All of my components manage errors using none other than true and false, which then drives parts of the component and its template logic. That validation logic now has to carefully reassign error states to null instead of false (or fill templates will explicit false => null conversions, gross!). There's also the dilemma that not all of these errors are used to toggle boolean attributes, so should they all blindly do the null thing, or do all devs need to learn and understand which errors get bound to bool attributes and null those while not nulling others? What was a clean and reliable solution is now messy and error-prone.

I have a permissions UI where the permissions come from a data store with permission values stored as true and false. All the false permissions now have to be explicitly reassigned to null in order for the UI to be correct. And I have to write more code to convert them back to false before passing them over to my storage API.

This is resulting in some really weird and ugly code. I've never had to do something like this in my 10+ years writing JavaScript.

Non-boolean attributes, in particular those that expect the string "false", should continue to be required to provide a string. Unlike the false to null dookie that's now required, the false to "false" is a very easy and clean false.toString(). This was never a problem for people who read the docs.

For the sake of Vue's ease-of-use and cleanliness, please consider reverting!

In the meantime, guaranteed broken boolean attributes is absolutely a big enough problem to warrant highlighting it in the "Breaking Changes" section of the Migration Guide. I caught this after migration during e2e testing and eventually found "Attributes coercion strategy changed" in the "Other minor changes" section. Please call this out in "Breaking Changes".

@posva
Copy link
Member

posva commented Aug 13, 2021

This is a breaking change improvement so it won't be possible to add to Vue 2 but it's working properly in Vue 3 by automatically casting false in true and only removing attributes in null and undefined 🎉 . More info at https://v3.vuejs.org/guide/migration/attribute-coercion.html#enumerated-attributes

@posva posva closed this as completed Aug 13, 2021
@jfbrennan
Copy link

jfbrennan commented Oct 20, 2021

How it should be:

<x-foo :boolean-attribute="aBoolean" :attribute-with-false-as-string="aBoolean.toString()">

How we have to dance around this new bizarre behavior:

<x-foo :boolean-attribute="aBoolean === false ? null : true" :attribute-with-false-as-string="aBoolean">

@LinusBorg
Copy link
Member

LinusBorg commented Oct 21, 2021

Vue can't know which custom attribute of a web component is meant to be a boolean attribute (as opposed to actual HTML attributes wich are defined as being boolean or not ion the spec). So we treat custom attributes as non-boolean attributes.

But: Most Web component implementations sync custom attributes to DOM properties and vice versa (see here).

For web components that do this, the described problem doesn't really exist, in my experience: When Vue can find a DOM property named booleanAttribute (as per your example above), it will use that to set it (el.booleanAttribute = false), instead of setting the attribute (el.setAttribute('boolean-attribute', false)).

This way, there is no special treatment necessary - the web component will receive a plain boolean value through the DOM property and can decide how to deal with it (remove the attribute, or set it with a stringified value) according to how it defined that property's type.

You seem to be using a web component implementation that doesn't do this, which is unfortunate for for this specific case.

@jfbrennan
Copy link

jfbrennan commented Oct 21, 2021

@LinusBorg your explanation/defense of Vue 2 behavior at the top of this issue (#11053 (comment)) was sound and Vue should have stuck with it. It was one of the few frameworks that Just Worked. React is struggling with this exact issue too (facebook/react#9230) and Vue 3 now conflicts with custom attributes just like React! As someone who made the switch that's really frustrating.

There doesn't seem to be any open discussion about this breaking change and the reasoning behind it (please share if there was), so what happened?

Also, Vue's new behavior is not consistent with JavaScript: setAttribute('foo', null) should result in foo="null" in the DOM, but Vue instead removes the attribute. So Vue hasn't really "fixed" anything with this breaking change, it's only made things more confusing.

Even more, CSS authors have to rewrite attribute selectors to accommodate this change in behavior (same problem in React world facebook/react#9230 (comment)).

It really seems like this breaking change should be reconsidered. The old behavior Just Worked. And what's so confusing about all this, is the solution to the original problem was incredibly easy to solve for: <button :aria-pressed="isToggled.toString()">, to use @posva 's example.

The intuitive treatment of false in Vue 2 should have never been changed to accommodate these less common and easily remedied cases. Please bring it back

@jfbrennan
Copy link

Just spent an hour refactoring some semi-complicated computed properties. Never knew how much of a pain it is to get JS to evaluate to exactly null. Ugh.
This was the wrong tradeoff for Vue to make and as Custom Elements (including the styles to support them) usage inevitably increases this unintuitive requirement will become a constant source of annoyance for devs. Please reconsider.

@posva
Copy link
Member

posva commented Oct 28, 2021

Alright, vue.js GitHub issues are simply not the place to drop your frustration.

The improvement went through an RFC in the rfcs repo, which you are free to check. You can also propose a new discussion to change this behavior but to be honest, you are the only person complaining about the improvement of writing :aria-value="boleanValue" instead of :aria-value="booleanValue.toString()".

@vuejs vuejs locked as resolved and limited conversation to collaborators Oct 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants