Add support for HTML5 boolean attributes

[colinrotherham]

Afternoon 👋

I noticed the attributes params don't support HTML5 boolean attributes:
https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#boolean-attributes

For example to support attributes here:

<element data-*>
<element checked>
<element disabled>
<element open>
<element hidden>
<element readonly>
<element inert>

With the current macros they'd render as:

<element data-*="">
<element checked="">
<element disabled="">
<element open="">
<element hidden="">
<element readonly="">
<element inert="">

This could cause problems in CSS or JS query selectors where element[open] might wrongly match for open="false" etc. Worth a fix?

I've added a truthy check around the attribute value:

Before

{% for attribute, value in params.attributes %} {{ attribute }}="{{ value }}"{% endfor %}

After

{% for attribute, value in params.attributes %} {{ attribute }}{% if value %}="{{ value }}"{% endif %}{% endfor %}

With a Jest snapshot matcher to ensure the attribute has no empty ="" value appended:

expect(componentHtml).toMatchSnapshot()

[NickColley]

Hey Colin,

Thanks for putting this together it seems logical to me.

Maybe you've explored this but is it possible to make this change by updating the existing attribute tests? That would be preferable to the extra complexity with helpers and snapshots.

Side note: I wonder if we should consider a 'utilities' template with macros for common things in all templates, so we could have this sort of logic tested once and make newer components or attributes functionality easier to add.

[colinrotherham]

@NickColley Unfortunately I tried this, but cheerio parses empty ="" attributes into HTML5 boolean attributes automatically (the empty attribute value gets removed).

So my tests were already passing, without making any Nunjucks changes.

That's why I had to test the Nunjucks HTML directly.

[colinrotherham]

We could still combine the new data-attribute: null HTML snapshots with the existing tests though? Will need to swap render() (parsed by cheerio) with the Nunjucks HTML output.

[colinrotherham]

@NickColley Here's where cheerio chose to remove empty ="" which means the snapshot differs to the output from Nunjucks where the empty value is preserved: cheeriojs/dom-serializer#44

[NickColley]

Thanks for the clarification on Cheerio stuff, makes sense.

We're gonna chat about this idea as a team tomorrow and let you know the next steps for your proposal, thanks again.

[NickColley]

We've chatted about this as a team, we like the change however since we don't have evidence this is an issue for people we think the added complexity to our testing suite and templates is not worth it right now.

If this comes up again this'll be super useful thanks a lot, sorry we have to close this.