blaze-html icon indicating copy to clipboard operation
blaze-html copied to clipboard

Handle boolean attributes better

Open dag opened this issue 10 years ago • 13 comments

http://www.whatwg.org/specs/web-apps/current-work/multipage/common-microsyntaxes.html#boolean-attribute

Currently attributes like disabled and checked (there are many more) in blaze-html all take an argument which means you have to give them the empty string or the name of that attribute (which you can misspell), and that the rendering will be cluttered with redundant noise.

I think it would be better to export e.g. disabled :: Attribute and then render that as simply disabled (might require tracking it in the MarkupM type?).

dag avatar Aug 11 '13 15:08 dag

Boolean attributes in HTML 5:

allowfullscreen 
async 
autofocus 
autoplay 
checked 
controls 
default 
defer 
disabled 
formnovalidate 
hidden 
inert 
ismap 
itemscope 
loop 
multiple 
muted 
novalidate 
open 
readonly 
required 
reversed 
scoped 
seamless 
selected 
typemustmatch 

dag avatar Aug 11 '13 16:08 dag

(Source: http://www.whatwg.org/specs/web-apps/current-work/multipage/section-index.html#attributes-1)

dag avatar Aug 11 '13 16:08 dag

I like this idea, but is this compatible with earlier HTML versions? I'd like to keep the API similar for e.g. Text.Blaze.Html4.Strict and Text.Blaze.Html5. What do you think, @meiersi?

jaspervdj avatar Aug 12 '13 23:08 jaspervdj

I'm not sure HTML4 even has any of these attributes, regardless of form?

dag avatar Aug 13 '13 09:08 dag

Scratch that, it has some of them and it supports the same forms: http://www.w3.org/TR/html401/intro/sgmltut.html#h-3.3.4.2

dag avatar Aug 13 '13 09:08 dag

It's a good idea. We should do this change. Thanks for suggesting this @dag.

meiersi avatar Aug 13 '13 13:08 meiersi

2015 bump

I've recently started using blaze-html and this issue stands out.

dmbarbour avatar May 19 '15 22:05 dmbarbour

Just stumbled upon the same question. I take it no progress since 2015. :)

moll avatar Feb 17 '20 23:02 moll

no progress for 10 years, time to celebrate!

avanov avatar Sep 18 '23 22:09 avanov

no progress for 10 years, time to celebrate!

:partying_face:

I'd happily merge this change if someone has time to put this together.

One requirement is that I don't want to break the existing API, so I would do this as an addition of new attributes. We currently have:

async :: AttributeValue -> Attribute

We could either use a ' or B (from Bool) suffix, though I'm also open to other options (like a different module?):

async' :: Attribute
asyncB :: Attribute

As a first step this could reuse the existing async attribute and be rendered as async="async", and a second step would be to remove the redundant noise in the renderer...

jaspervdj avatar Sep 19 '23 11:09 jaspervdj

I can see why this hasn't been fixed though, as almost all of the recent CSS frameworks implement boolean markers via class attribute values, so that for instance the independent required attribute is less needed than class="required".

I've skimmed through the Attribute implementation and it seems that all potential solutions have to eventually either modify how AddAttribute expects ChoiceString (which boolean attributes won't provide) or to provide another variant that extends the MarkupM, and they both do cause API change (because MarkupM is exposed to user calling sites). So, which option would you agree with @jaspervdj ?

avanov avatar Sep 19 '23 13:09 avanov

I found another option that doesn't change API at the cost of an extra pattern match for every rendered attribute.

Let's say attrB is defined as attrB = attribute "attrB" " attrB=\"" "<special-nonempty-symbol>", then the renderer could pattern match on (attrVal == "<special-nonempty-symbol>", attrName `elem` boolAttrs) to decide whether it can skip attribute value rendering.

avanov avatar Sep 19 '23 17:09 avanov

There's a benefit in having the lack of an argument in the type system. It reduces the chance you accidentally pass a value expecting it to have meaning. That's probably an argument for having a breaking change @jaspervdj was hesitant to do in https://github.com/jaspervdj/blaze-html/issues/80#issuecomment-1725356550. Effectively I'd say all uses of A.checked "some-nonempty-value" are possible bugs. Having a new separate function wouldn't force a review of current call sites and would always require tooling to catch in the future.

moll avatar Sep 20 '23 10:09 moll