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

Handle boolean attributes better #80

Open
dag opened this issue Aug 11, 2013 · 13 comments
Open

Handle boolean attributes better #80

dag opened this issue Aug 11, 2013 · 13 comments

Comments

@dag
Copy link

dag commented Aug 11, 2013

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
Copy link
Author

dag commented Aug 11, 2013

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
Copy link
Author

dag commented Aug 11, 2013

@jaspervdj
Copy link
Owner

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?

@dag
Copy link
Author

dag commented Aug 13, 2013

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

@dag
Copy link
Author

dag commented Aug 13, 2013

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

@meiersi
Copy link
Collaborator

meiersi commented Aug 13, 2013

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

@dmbarbour
Copy link

2015 bump

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

@moll
Copy link

moll commented Feb 17, 2020

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

@avanov
Copy link

avanov commented Sep 18, 2023

no progress for 10 years, time to celebrate!

@jaspervdj
Copy link
Owner

no progress for 10 years, time to celebrate!

🥳

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...

@avanov
Copy link

avanov commented Sep 19, 2023

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
Copy link

avanov commented Sep 19, 2023

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.

@moll
Copy link

moll commented Sep 20, 2023

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 #80 (comment). 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.

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

6 participants