Test for empty items in radios and checkboxes

[edwardhorsford]

I'd like to propose testing if items inside the items array for checkboxes and radios are empty. If they are, don't output markup.

My use case

I want to conditionally show a radio based on some data.

In Nunjucks I can alter a value of an item in an array by adding a conditional after it - but it still outputs an empty item and I've not worked out how to not provide it.

I could prepare my data in advance - but for a simple thing like this, doing it directly is more readable IMHO. You can see the logic directly.

Example macro:

{{ govukRadios({
  idPrefix: "where-do-you-live",
  name: "where-do-you-live",
  fieldset: {
    legend: {
      text: "Where do you live?",
      isPageHeading: true,
      classes: "govuk-fieldset__legend--xl"
    }
  },
  items: [
    {
      value: "england",
      text: "England"
    },
    {
      value: "scotland",
      text: "Scotland"
    },
    {
      value: "wales",
      text: "Wales"
    },
    {
      value: "northern-ireland",
      text: "Northern Ireland"
    },
    {
      divider: "or"
    } if data.showFantasy == true,
    {
      value: "narnia",
      text: "Narnia"
    } if data.showFantasy == true
  ]
}) }}

This mostly works - but right now outputs a blank space and random html doing nothing:

Suggested fix

Either strip out empty items from the items array or else test whether they're empty when iterating over them.

This feels like a safer thing to do / more defensive. We don't output markup where there's no input.

I had a quick test of this by editing the radios macro directly in my node_modules, and it appears to work well.

[NickColley]

I've put together a live demonstration to show what Ed is talking about here:

https://govuk-frontend-issue-1494.glitch.me/ - (edit: https://glitch.com/edit/#!/govuk-frontend-issue-1494)

[NickColley]

Should we also consider doing something similar for:

• https://design-system.service.gov.uk/components/accordion/ (items)
• https://design-system.service.gov.uk/components/breadcrumbs/ (items)
• https://design-system.service.gov.uk/components/error-summary/ (error list)
• https://design-system.service.gov.uk/components/tabs/ (items)
• https://design-system.service.gov.uk/components/summary-list/ (rows)

[paulmsmith]

If I'm following along correctly... you want to declaratively use that macro by checking if a flag is true and adding in data based on the condition?

I've ran into this sort of thing before and I ended up using additional filters for manipulating data within the nunjucks templates.

So I expose something like lodash as nunjucks filters which can then be piped together. In this case might create the first part of the array a separate variable using {% set %} and then maybe do the conditional logic for the flag or if the duplication of the values offends you could do an array merge with the additional array items should the flag be true.

Off the top of my head something like:

{{ govukRadios({
  idPrefix: "where-do-you-live",
  name: "where-do-you-live",
  fieldset: {
    legend: {
      text: "Where do you live?",
      isPageHeading: true,
      classes: "govuk-fieldset__legend--xl"
    }
  },
  items: [
    {
      value: "england",
      text: "England"
    },
    {
      value: "scotland",
      text: "Scotland"
    },
    {
      value: "wales",
      text: "Wales"
    },
    {
      value: "northern-ireland",
      text: "Northern Ireland"
    }
  ] | conditionalMerge(data.showFantasy, [
    {
      divider: "or"
    },
    {
      value: "narnia",
      text: "Narnia"
    }
}) }}

With conditionalMerge being a filter I've written for doing the repeatable bit of data manipulation I'd like. I prefer that to logic being in the macro (which I guess in this case it never would be) and it means you can be more declarative in the templates than always have to massage the data prior which keeps prototyping snappy.

[edwardhorsford]

@paulmsmith thanks for that!

For the context of this issue, I feel like the macro should be more defensive and not output anything where the input is blank. As a byproduct that also solves my issue ;)

Separately I'm somewhat sure I've seen a way of doing this in Nunjucks directly - but can't recall where / what it is.

[NickColley]

I've had a go at this and it looks like a sensible suggestion to only output items when they are truthy like Ed has suggested.

Will see what the rest of the team thinks, we'd want to do this for all the components I've mentioned above as well.

[paulmsmith]

I agree, the macro should be only outputting stuff when it has valid data. :)

[NickColley]

Thanks Ed, we've added this to our backlog as part of the improve macros in GOV.UK Frontend epic.

If you wanted to contribute this sooner, feel free to raise a pull request.

[edwardhorsford]

A similar issue exists if you pass empty conditionals:

[edwardhorsford]

A similar thing would be good for tables - testing items inside head and rows if any are empty.

[36degrees]

I believe this was addressed in #1512

[edwardhorsford]

@36degrees Should I raise my above issue with empty conditionals separately? or is that also solved with #1512

[36degrees]

Ah, sorry @edwardhorsford, my bad. If you can raise a separate issue that'd be great – I don't believe it's solved by #1512 (though we do test for {% if item.conditional %}, so I'm not entirely sure what's going on there).