Agree on the macro API for a 'None of the above' checkbox JavaScript enhancement

timpaul commented 3 years ago

What

Agree on the macro API for a proposed JavaScript enhancement to this PR:

https://github.com/alphagov/govuk-frontend/pull/2151

The enhancement would automatically uncheck all of the other checkboxes in the list, when the 'none-of-the-above' box is checked.

Why

A contributor has volunteered to develop the JavaScript for this feature, but would like to agree up front on how it should be triggered in the macro.

Who needs to know about this

Developer

Done when

[x] Chatted to contributor and agreed on the API design

frankieroberto commented 3 years ago

Here's a quick suggestion from me, adding a boolean to the item (key name tbc):

{{ govukCheckboxes({
  idPrefix: "waste",
  name: "waste",
  fieldset: {
    legend: {
      text: "Which types of waste do you transport?",
      isPageHeading: true,
      classes: "govuk-fieldset__legend--l"
    }
  },
  hint: {
    text: "Select all that apply."
  },
  items: [
    {
      value: "carcasses",
      text: "Waste from animal carcasses"
    },
    {
      value: "mines",
      text: "Waste from mines or quarries"
    },
    {
      value: "farm",
      text: "Farm or agricultural waste"
    },
    {
      divider: "Or"
    },
    {
      value: "none",
      text: "None of these",
      selectsNone: true
    }
  ]
}) }}

Pros:

Flexible, doesn't make any assumption about the need for a divider or the position or label of the "None" option.

Cons:

Unclear what would/should happen if you have two items with this boolean set to true...

Out of scope for this ticket, but possibly worth considering for the API design, is how you might also incorporate a "Select all" feature, which one checkbox (usually at the top) selects all the other ones – useful for bulk actions. That one conventionally also un-ticks all of them if they're all ticked and the "Select all" checkbox is ticked again, which is kind-of similar to the "none" option.

frankieroberto commented 3 years ago

Related issue: #1795.

edwardhorsford commented 3 years ago

On my previous project we supported this by allowing for mutually exclusive groups. This is very flexible, allowing for different combinations of groups - but at the cost of complexity of api vs Frankie's suggestion.

Although multiple groups would be nice to support, I reckon 95% of the need is for a single 'none of these' option.

edwardhorsford commented 3 years ago

Riffing on Frankie's

{{ govukCheckboxes({
  idPrefix: "waste",
  name: "waste",
  fieldset: {
    legend: {
      text: "Which types of waste do you transport?",
      isPageHeading: true,
      classes: "govuk-fieldset__legend--l"
    }
  },
  hint: {
    text: "Select all that apply."
  },
  items: [
    {
      value: "carcasses",
      text: "Waste from animal carcasses"
    },
    {
      value: "mines",
      text: "Waste from mines or quarries"
    },
    {
      value: "farm",
      text: "Farm or agricultural waste"
    },
    {
      divider: "Or"
    },
    {
      value: "none",
      text: "None of these",
      mutuallyExclusiveGroup: "none-of-these"
    }
  ]
}) }}

Instead pass a name for a mutually exclusive group - js combines all of these in to a group and will deselect items outside of the group (either in named groups or default). Selecting items in another group will deselect items that aren't in the same or default group.

I think this would answer Frankie's question about two items with the boolean - if they had the same name they're in the same group so can be independently checked. If they had different names, they'd be treated as separate groups.

edwardhorsford commented 3 years ago

Different idea:

deselect: {opts},

Where opts could be all-others which enumerates all other items, or possibly in the future an array of ids or something else. This supports the simple case, but allows flexibility for enhancement in the future api.

Similarly it could expand to:

selects: "all-others"

frankieroberto commented 3 years ago

Another idea:

    {
      value: "none",
      text: "None of these",
      behaviour: "select-none"
    }

The behaviour key might help to hint that this changes the behaviour of the button, and having a keyword value rather than a boolean means this could be extended in the future with other types of behaviour, eg behaviour: "select-all"?

Heh, API design is hard!

joelanman commented 3 years ago

    {
      value: "none",
      text: "None of these",
      behaviour: "select-none"
    }

I think this is a good balance of designing for the case we're sure about (select none) and flexibility for other cases we have some confidence in (select all). I don't think we know much about user needs for more complex selection of groups

We normally try to stick close to HTML naming/api, but the closest thing I can think of to this in HTML is the <button element doing different things in a form, according to its type - so type: "select-none"?

Do we know how any other framework does this?

frankieroberto commented 3 years ago

@joelanman thanks Joe. I wonder if type is a bit of an overloaded word, as the radio input is still type="radio"?

It might make sense if the macro API maps to the HTML implementation too, so behaviour: "select-none" would output a data-behaviour="select-none" on the HTML element, which would trigger the javascript?

Other alternatives for behaviour could be:

onCheck: "select-none" (possible downside is that this doesn't describe the additional behaviour where it gets unchecked if anything else is checked)
action: "select-none"
enhancement: "select-none"

There's a separate question about whether select-none should be selectNone or not... (I don't think we have any macro options which have token values yet?)

joelanman commented 3 years ago

sorry of course, we can't use type, as its radio as you say :)

36degrees commented 3 years ago

Thanks all, lots of really useful discussion here. I talked through some of the options with @vanitabarrett.

We'd prefer to avoid a boolean flag for a behaviour like this, as it can lead to strange 'undefined' functionality if we introduced other competing behaviours in the future, for example if we ended up with selectAll and selectNone booleans, users could technically pass true for both.

We think behaviour makes sense as the key name, and agree that data attribute should mirror it. The only possible complication might be teams implementing their own custom behaviours for checkboxes and using the behaviour key as a hook for their own JS – which I think could cause issues, but I'm not sure that should be a blocker.

We're less sure about select-none – it seems like a bit of a misnomer as you're not really selecting no checkboxes, and as you say it seems unclear what happens if multiple checkboxes have it.

We were thinking about behaviour: "exclusive" which seems a slightly better description, with the following behaviours:

Checking an exclusive checkbox unchecks every other checkbox with the same name (including any other exclusive checkboxes)
Checking a non-exclusive checkbox, unchecks any exclusive checkboxes with the same name

This also mirrors the existing behaviour from checkboxes built into GOV.UK Publishing Components – although it doesn't match their API as they've gone for a boolean flag.

Finally, we'd agree groups of exclusive checkboxes are out of scope, but think this could be added in the future if needed without needing to change the existing API (e.g. behaviour: "exclusive", group: "vegetables")

Thoughts?

frankieroberto commented 3 years ago

@36degrees that all sounds good to me.

Just to be super clear, I’ll update the Checkboxes macro so that if you pass in behaviour: "exclusive" on an item, it’ll add a data-behaviour="exclusive" attribute to the <input>, and then I'll update the existing Checkboxes javascript to implement the behaviour described in your 2 bullet points.

For non-javascript contexts, the divider and input will still be rendered, and it'll be up to services to handle a validation error if the "None" checkbox is checked as well as some others.

One other minor thing: I agree it'll need to look at inputs with the same name attribute value, but should this also be scoped to inputs within the same <fieldset> or not? I think the existing javascript does this for conditional reveals (even though the IDs should in theory be unique).

36degrees commented 3 years ago

Just to be super clear, I’ll update the Checkboxes macro so that if you pass in behaviour: "exclusive" on an item, it’ll add a data-behaviour="exclusive" attribute to the <input>, and then I'll update the existing Checkboxes javascript to implement the behaviour described in your 2 bullet points.

For non-javascript contexts, the divider and input will still be rendered, and it'll be up to services to handle a validation error if the "None" checkbox is checked as well as some others.

That all sounds good to me 👍🏻

One other minor thing: I agree it'll need to look at inputs with the same name attribute value, but should this also be scoped to inputs within the same <fieldset> or not? I think the existing javascript does this for conditional reveals (even though the IDs should in theory be unique).

I think it should affect other inputs with the same name and form – similar to what we currently do in radios:

https://github.com/alphagov/govuk-frontend/blob/c4fbe3dc7ff67741f7a07a2fe70ee9cd91b94cfe/src/govuk/components/radios/radios.js#L90-L120

I think we'll also need to adjust the logic used for conditional reveals, as theoretically checking a checkbox could uncheck a checkbox elsewhere in the form. Again, I think it'll need to look similar to the logic we use in radios.

edwardhorsford commented 3 years ago

@36degrees your suggestions sound good to me 👍

frankieroberto commented 3 years ago

@36degrees ok, scoping to the form makes sense. Allows people to do crazy things like having nested fieldsets. 😄

frankieroberto commented 3 years ago

@36degrees now that we've agreed this, shall we close this Issue / move to Done on your board?

We can have any follow-up discussion on the Pull Request: https://github.com/alphagov/govuk-frontend/pull/2151

36degrees commented 3 years ago

I think that sounds sensible – thanks @frankieroberto 🙂

alphagov / govuk-frontend