JuliaWeb / Hyperscript.jl

Hyperscript: A lightweight DOM representation for Julia
Other
101 stars 11 forks source link

support boolean values for boolean attributes #20

Open piever opened 4 years ago

piever commented 4 years ago

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 https://github.com/SimonDanisch/JSServe.jl/pull/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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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?