Allow values for the default date input

[joelanman]

There are two ways to use govukDateInput, by passing items or leaving that blank and getting the default Day, Month and Year that most services need.

However you nearly always need to set the values, at which point you can't use the default mode, you have to set all the items individually.

This PR adds a values option similar to the checkboxes component, so you can keep using the default mode. This is particularly useful in prototyping, where the current approach is much more complex for non-developers.

In usage this looks like:

{{ govukDateInput({
  id: "passport-issued",
  namePrefix: "passport-issued",
  fieldset: {
    legend: {
      text: "When was your passport issued?",
      isPageHeading: true,
      classes: "govuk-fieldset__legend--l"
    }
  },
  hint: {
    text: "For example, 27 3 2007"
  },
  values: [27, 3, 2007]
}) }}

compared to the existing approach:

{{ govukDateInput({
  id: "passport-issued",
  namePrefix: "passport-issued",
  fieldset: {
    legend: {
      text: "When was your passport issued?",
      isPageHeading: true,
      classes: "govuk-fieldset__legend--l"
    }
  },
  hint: {
    text: "For example, 27 3 2007"
  },
  items: [
    {
      classes: "govuk-input--width-2",
      name: "day",
      value: "27"
    },
    {
      classes: "govuk-input--width-2",
      name: "month",
      value: "3"
    },
    {
      classes: "govuk-input--width-4",
      name: "year",
      value: "2007"
    }
  ]
}) }}

It also supports errors:

{{ govukDateInput({
  id: "passport-issued",
  namePrefix: "passport-issued",
  fieldset: {
    legend: {
      text: "When was your passport issued?",
      isPageHeading: true,
      classes: "govuk-fieldset__legend--l"
    }
  },
  hint: {
    text: "For example, 27 3 2007"
  },
  errorMessage: {
    text: "The date your passport was issued must be in the past"
  },
  values: [27, 3, 2047],
  errors: [true, true, true]
}) }}

compared to

{{ govukDateInput({
  id: "passport-issued",
  namePrefix: "passport-issued",
  fieldset: {
    legend: {
      text: "When was your passport issued?",
      isPageHeading: true,
      classes: "govuk-fieldset__legend--l"
    }
  },
  hint: {
    text: "For example, 27 3 2007"
  },
  errorMessage: {
    text: "The date your passport was issued must be in the past"
  },
  items: [
    {
      classes: "govuk-input--width-2 govuk-input--error",
      name: "day",
      value: "27"
    },
    {
      classes: "govuk-input--width-2 govuk-input--error",
      name: "month",
      value: "3"
    },
    {
      classes: "govuk-input--width-4 govuk-input--error",
      name: "year",
      value: "2076"
    }
  ]
}) }}

[joelanman]

thinking some more, if the main value here is for prototyping, it might be better to look into an option to automatically set all values based on names in the kit (no need to manually set values at all), I think @36degrees did an investigation into that a while back

Update:

Here is the investigation:

alphagov/govuk-prototype-kit#1316

Update 2:

This approach is no longer possible in v13 due to the way macros are imported

[joelanman]

you could add another property to handle errors for the default case:

{{ govukDateInput({
    values: [data['passport-issued-day'],
             data['passport-issued-month'],
             data['passport-issued-year']],
    errors: [false, false, true]
...
}) }}

[frankieroberto]

This looks good, and would resolve #1154.

I wonder if the values key would be slightly more intuitive as an object, eg:

{{ govukDateInput({
    values: {
      day: data['passport-issued-day',
      month: data['passport-issued-month'],
      year: data['passport-issued-year']
    },
}) }}  

That would save people having to know which order it's in, and would also allow you to just set one or two of the date part values if needed (although this seems less likely).

[joelanman]

I've update to latest Frontend and added the errors params

[frankieroberto]

@joelanman Thinking about this some more, having the values key accept an object should make it even easier to set all 3 date parts at once using the prototype kit, as you could do this:

{{ govukDateInput({
   namePrefix: "passportIssued",
    values: data.passportIssued,
...    
}) }}

The only other change needed would be to change the default for the name attribute on each of the fields to be {{params.namePrefix}}[day] instead of {{params.namePrefix}}-day.

Feels like that’d be worthwhile?

[joelanman]

It is simpler and I can see how {{params.namePrefix}}[day] would be beneficial for prototyping, but it would be a breaking change, and not follow the naming conventions in Frontend so I don't think it would be accepted

[romaricpascal]

@joelanman Thanks for your patience. These new options look pretty useful, so we'd be keen to add them to the component. Before we can merge this, though, there'd need to be:

• new tests for them in the template.test.js file for the date input, ideally including one confirming what happens when both these new options and items are used (people will hopefully not do that, but that'll document the behaviour to anticipate future breaking changes)
• documentation in [the date-input.yaml file for the component], as well

Is that something you'd be comfortable adding?

@frankieroberto Receiving a {day:..., month:..., year:...} object and updating the field names to use brackets for the name feel better suited for their own piece of work. The latter especially will require a thorough review of the impact of updating the default names, as not all backends will parse the brackets in submitted names (while a common convention, it requires enabling the extended option for express' body-parser, I think). It's not necessarily a breaking change, though, as we could provide the new name formatting through a nameFormat: 'brackets' option to trigger the bracket notation, leaving the default behaviour as it is now.

[frankieroberto]

@frankieroberto Receiving a {day:..., month:..., year:...} object and updating the field names to use brackets for the name feel better suited for their own piece of work. The latter especially will require a thorough review of the impact of updating the default names, as not all backends will parse the brackets in submitted names (while a common convention, it requires enabling the extended option for express' body-parser, I think). It's not necessarily a breaking change, though, as we could provide the new name formatting through a nameFormat: 'brackets' option to trigger the bracket notation, leaving the default behaviour as it is now.

I think the bracket syntax has always worked in the GOV.UK prototype kit, at least for as long as I can remember. The body-parser documentation seems like it defaults to true?

@romaricpascal The change in the default name could come later, and still use the object format for values, which would mean having to do something like this initially, which isn't much more code over an array:

{{ govukDateInput({
    values: {
      day: data['passport-issued-day'],
      month: data['passport-issued-month'],
      year: data['passport-issued-year']
    },
 ...
}) }} 

Alternatively, I guess it could make sense to support both an array and an object? (Although possibly that’s over-complicated?)

One other advantage of accepting an object is that this could then set the values even if you used a custom items array (eg just month and year), which @edwardhorsford has suggested on our version of this PR for NHS frontend.

[romaricpascal]

I think the bracket syntax has always worked in the GOV.UK prototype kit, at least for as long as I can remember. The body-parser documentation seems like it defaults to true?

Oh, good spot, had missed that true was the default for that option of body-parser in that last sentence. Changing the names wholesale would be a breaking change in any case, so using an option for the macro could speed up its release without waiting for the next major release of GOV.UK Frontend.

One other advantage of accepting an object is that this could then set the values even if you used a custom items array (eg just month and year), which @edwardhorsford nhsuk/nhsuk-frontend#994 (comment) on our version of this PR for NHS frontend.

I believe that @edwardhorsford's suggestion could also work for arrays, using loop.index0 to pull the value at the index of the item that's being rendered.

Alternatively, I guess it could make sense to support both an array and an object? (Although possibly that’s over-complicated?)

Untested (and unsure of the syntax), like Ed's suggestion, but I guess it wouldn't be too terrible to look at both array and object access when rendering the item with something like:

item.value or ((values[loop.index0] or values[item.name]) if values)

That'd require some thorough testing around false-y values to ensure that an unexpected 0 (submitted by mistake, for example) gets handled correctly.

[joelanman]

It's still true that "not all backends will parse the brackets in submitted names" - the kit is only one place Frontend is used, other people could be using the components in other backends and frameworks

[romaricpascal]

It's still true that "not all backends will parse the brackets in submitted names" - the kit is only one place Frontend is used, other people could be using the components in other backends and frameworks

Yup, we'd certainly want a review of what Node backends do in any case before changing the default name of the field (Ports being free to adapted the naming of the fields to their language conventions, like what govuk-form-builder does).

[frankieroberto]

I agree it should be considered a breaking change, even if we don't know of any services it’d affect.

Using a option for the macro seems like a nice way of avoiding it being a breaking change, and then it could always become the default in the next breaking change release?