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

Boolean attributes #14

Closed
colingourlay opened this issue Jan 22, 2016 · 11 comments
Closed

Boolean attributes #14

colingourlay opened this issue Jan 22, 2016 · 11 comments

Comments

@colingourlay
Copy link
Collaborator

@substack: Is there a way to write boolean attributes, such as checked on <input type="checkbox"/>?I've tried various combinations of keys and values, and I can't get anything passed through to the wrapped hyperscript function.

@ghost
Copy link

ghost commented Jan 22, 2016

I think this is a bug in the parser.

@SomeoneWeird
Copy link

Any update on this?

@ghost
Copy link

ghost commented Mar 5, 2016

Fixed in 2.0.1. This was very non-trivial to fix! Booleans throw a wrench into lots of places in the parser.

@ghost ghost closed this as completed Mar 5, 2016
@feross
Copy link
Collaborator

feross commented Mar 5, 2016

@substack Nice fix! I spent more time than I care to admit trying to fix it too, but gave up. Glad you got it fixed!

@feross
Copy link
Collaborator

feross commented Mar 5, 2016

@substack Sadly, this doesn't seem fully fixed. If the boolean attribute is followed by a normal attribute, it crashes hyperx with Error: unhandled: 8.

Here's a test case:

var test = require('tape')
var vdom = require('virtual-dom')
var hyperx = require('../')
var hx = hyperx(vdom.h)

test('boolean attribute followed by normal attribute', function (t) {
  var tree = hx`<video autoplay volume="50"></video>`
  t.equal(vdom.create(tree).toString(), '<video autoplay="autoplay" volume="50"></video>')
  t.end()
})

@feross feross reopened this Mar 5, 2016
@ghost
Copy link

ghost commented Mar 5, 2016

Fixed in 2.0.2 and I added this case to the test suite. Re-open if there are any more failing cases.

@ghost ghost closed this as completed Mar 5, 2016
@colingourlay
Copy link
Collaborator Author

@feross @substack I spent too long on this too before giving up. I really appreciate you both getting this in!

@arturi
Copy link

arturi commented Apr 4, 2016

Hey! Not sure if I’m doing something wrong, but this works:

hx`<div aria-hidden="${isVisible ? 'false' : 'true'}">content</div>`

And this does not:

hx`<div ${isVisible ? '' : 'aria-hidden="true"'}">content</div>`

So I can toggle between attribute values, but can’t omit an attribute entirely, based on a condition.

@shama
Copy link
Member

shama commented Apr 4, 2016

@arturi AFAIK, that currently isn't possible as the parser needs to know whether it's in a attribute and can't evaluate an expression to determine it. The library that constructs the element should know that a boolean attribute set to false should be omitted, making your first example or more simply: hx

content
`` be the correct practice.

For example in bel, if the attribute is true it gets set to the attribute key itself otherwise is completely omitted.

@arturi
Copy link

arturi commented Apr 4, 2016

All right, got it, thanks! Refactored to ${isVisible} as well.

I’m using yo-yo, actually, which is using bel, but aria-hidden is not on the list of boolean attributes. Do you think it should be?

@shama
Copy link
Member

shama commented Apr 4, 2016

@arturi It likely could be but depends on it's common usage. Looking at the examples on MDN, it looked like it was common to set aria-hidden="true" which means it's not exactly a boolean but set to a string "true" or "false". But I think it would be a good idea to include as a boolean option, at least until someone else brings up a use case saying otherwise. Feel free to send a PR, thanks!

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants