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

support boolean values for boolean attributes #20

Open
piever opened this issue Feb 21, 2020 · 7 comments · May be fixed by #35
Open

support boolean values for boolean attributes #20

piever opened this issue Feb 21, 2020 · 7 comments · May be fixed by #35

Comments

@piever
Copy link

piever commented Feb 21, 2020

Even though the correct HTML5 thing to do is to use valueless boolean attributes or to not add them at all (i.e. <input type=checkbox checked /> versus <input type=checkbox />), this becomes a bit awkward when generating them programmatically. In particular, if one wants to add observables to Hyperscript to add interactivity (see SimonDanisch/Bonito.jl#35 for instance), things become a bit awkward.

From what I understand, HTML5 actually disallows using "true" or "false" as values for boolean attributes (see here). Could it be possible to support passing checked = false to mean that the attribute is actually not set, and checked = true to set it?

I confess I'm not sure what the attribute should be set to with checked = true, but I imagine anything that conforms to HTML5 would be fine (empty string, or name of the attribute, or maybe even valueless).

@SimonDanisch
Copy link
Collaborator

I guess we could add it here?
https://github.com/yurivish/Hyperscript.jl/blob/master/src/Hyperscript.jl#L208
@yurivish, if you agree that it's the right place, i can move this function:
https://github.com/SimonDanisch/JSServe.jl/blob/f36f3ac7bfc96e1c88f189dcbbfa6eed65e4cf01/src/hyperscript_integration.jl#L72
Over to Hyperscript and check for boolean attributes when rendering!

@yurivish
Copy link
Collaborator

Thanks for this issue! I think adding a check for literal true and false there would be reasonable.

@SimonDanisch I'm wondering whether it might make sense to do this uniformly for all tags, rather than HTML's boolean attributes, since it might be confusing if foo=true is always accepted but behaves differently from case to case. I'm thinking about use cases where you want to conditionally emit custom attributes.

@SimonDanisch
Copy link
Collaborator

I'm thinking about use cases where you want to conditionally emit custom attributes.

Hm, so my idea was, to not leaky this weird behavior into the official API... So I'd say, custom_attribute=false should stay custom_attribute=false... I find this convention very irritating, and just want to support it, since you can't do without. I'd say, if we really want an API for leaving out attributes that you set(!?), we should use something like missing.

@yurivish
Copy link
Collaborator

Hm.

Here's another thought:

julia> f(; kw...) = kw
f (generic function with 1 method)

julia> skip(k, v::Bool) = v ? (k => v,) : ()
skip (generic function with 1 method)

julia> f(; skip(:a, true)..., skip(:b, false)...)
pairs(::NamedTuple) with 1 entry:
  :a => true

Using this is more verbose and requires the user to know that an attribute is valueless, but it has the benefit of being an explicit and consistent behavior that is orthogonal from the rest of the system. Maybe there's a more concise way to do something like this?

@SimonDanisch
Copy link
Collaborator

hm... So I think I kind of consider the html behavior to be a bug :D That's of course also because I haven't really used HTML before using Hyperscript^^ But I would like to push this behavior as far away from the user as possible. Of course this is somewhat dangerous, since that will confuse any user that tries to dig deeper... Or users that have worked a lot with html before ;)
But still, I want consistent behavior with how things work when setting them from js!
So that jsdiv.attribute = false and div(attribute=false) does the same in Julia & JS

@piever
Copy link
Author

piever commented Feb 23, 2020

It's true that this is overlapping but distinct from the issue of deciding whether to put some attribute programmatically, as that can be done with the keyword argument splatting trick proposed above. Thinking more about it, I feel that the issue is more about having a consistent syntax to initialize and update an element, so that it becomes feasible to make a nice library to create dynamic pages based on Hyperscript. In javascript hyperscript (for example mithril) they try to conflate HTML attributes with DOM properties and stringify true to "", whereas false removes the attribute.

I would limit this behavior to boolean attributes, as in other attributes it would be surprising (with value=false it would be odd to have no value attribute for example). OTOH, in HTML5 boolean attributes it is discouraged / not supported to use boolean values, so I feel this leaves us more freedom to define how they'd behave.

@yurivish
Copy link
Collaborator

It sounds like using boolean values the same way Mithril does (true => "" , false => do not emit the attribute) makes sense here, and so does limiting the behavior to boolean attributes to minimize surprise (though it'll be surprising either way). Thanks!

@SimonDanisch I think you've correctly identified the place to change this — would you mind making a pull request for special-casing true and false for boolean attributes?

@schneiderfelipe schneiderfelipe linked a pull request May 21, 2021 that will close this issue
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

Successfully merging a pull request may close this issue.

3 participants