Allow attributes on date input items

[malcolmhire]

Currently, there is no way of adding extra attributes to each of the date input fields, this would be a fairly easy update but the macro already sets attributes on Line 67 and as there no built in way to merge objects in Nunjucks (well not that I know of) this would require a filter, but this would need to added application side when initializing Nunjucks.

Any ideas how to achieve this?

[kr8n3r]

Hi @malcolmhire ,
thanks for raising this. As you have correctly identified, there is no built-in way to merge objects in Nunjucks without creating a filter.
Relying on our users to create a filter, as we can't ship one with a component, isn't something we'd want to do.

One possible solution would be to move the pattern attribute from the object and add it directly to the component, but that might not be as flexible if users want to create a custom pattern on data-inputs. I'm not sure how often that would happen though.

I have also created a ticket in our backlog so we can track this.

[joelanman]

I wonder if objects could be merged using a for loop

[malcolmhire]

Hi @malcolmhire ,
thanks for raising this. As you have correctly identified, there is no built-in way to merge objects in Nunjucks without creating a filter.
Relying on our users to create a filter, as we can't ship one with a component, isn't something we'd want to do.

One possible solution would be to move the pattern attribute from the object and add it directly to the component, but that might not be as flexible if users wan't to create a custom pattern on data-input. I'm not sure how often that would happen though.

I have also created a ticket in our backlog so we can track this.

When you say 'directly to the component' do you mean adding it directly to the govukInput? I thought about adding it like that with the pattern as part of the govukInput, this would then leave open attributes object, but this wouldn't be consistent with all the other components so ditched that idea.

@igloosi is this the sort of thing you were meaning?

{{ govukInput({ label: { text: "foo", }, id: "foo", name: "foo" value: "bar", type: "number", pattern: "[0-9]*" })

[kr8n3r]

@malcolmhire something along those lines yes, but you're right, it lacks consistency

wondering if we can get user attributes object and component's internal object, push both to an array an then internally in the component loop over the array?

perhaps we might have to allow an array of objects to be passed as attributes

@malcolmhire
there is a check in Nunjucks one can do check if array is empty

{% if array | length %}

so attributes is passed as an array of objects we could loop over them and merge them with the default attributes
otherwise juts loop over them

Nunjucks array loop

{% for item in items %}

Nunjucks object loop

{% for key, value in item %}

your thoughts?

[malcolmhire]

@malcolmhire something along those lines yes, but you're right, it lacks consistency

wondering if we can get user attributes object and component's internal object, push both to an array an then internally in the component loop over the array?

perhaps we might have to allow an array of objects to be passed as attributes

@malcolmhire
there is a check in Nunjucks one can do check if array is empty

{% if array | length %}

so attributes is passed as an array of objects we could loop over them and merge them with the default attributes
otherwise juts loop over them

Nunjucks array loop

{% for item in items %}

Nunjucks object loop

{% for key, value in item %}

your thoughts?

Hey, sorry for the late reply, been on leave.

I think that idea will work but again is it not consistent with the other components as attributes are a single object on all other components (unless I've missed something). Thoughts?

[kr8n3r]

@malcolmhire yeah they are. I also did some quick testing and there isn't a reliable way to check with something | length as it will return 1 for an object as well.

[NickColley]

I've spoken with Hanna about this, and we think these are the ways forward:

1. Go with current proposal that overrides attributes, and make guidance (YAML) clearer that the pattern attributes need to be added again manually.

Risks:

• people may not know they need to use 'pattern' by default
• if we change these defaults it will not be changed for users that have overridden them
• if we want to improve in the future, could be a breaking change

2. Investigate if it's possible to add a global function for merging without making immediate breaking changes.

Risks:

• might be a breaking change to add global functions

3. Investigate a private attributes API on the input component that is only used when composing the component like in date input.

{{ govukInput({ __additionalAttributes: {} }) }}

We'd need to ensure that the additional attributes overrides existing attributes before they're applied.

Risks:

• we'd have to rely on conventions to make this private

4. Consider adding 'pattern' as a top level API for text input, avoiding the need to merge attributes.

Risks:

• more API surface could mean more complexity for our users to learn

[NickColley]

I've gone over this with @36degrees and had a separate discussion with @hannalaakso.

We think there are risks with option 1. which I've updated in the previous comment, and that we should go with an alternative that avoids this.

So we are proposing to add pattern as a top level option for the text input component macro. (option 4.)

Which will allow the user to set attributes without having to remove the defaults.

The reasoning for this is we think that the attributes option should not be be use internally if possible. If we find attributes that are useful internally, we should consider if they're in the HTML spec to add them as a top level API for the macro.

We would then do a technical spike into option 2. as we think that being able to use global functions has enough evidence of usefulness but we don't want to block this until we've figured that out.

@colinrotherham @malcolmhire how does that sound?

[colinrotherham]

Yeah sounds good to me.

Just like the work to enable autocomplete in #1146 but for pattern as shown below?

Also if absolutely required we can still add attributes too?

{{ govukDateInput({
  items: [
    {
      // If omitted, default is still set
      pattern: "[0-9]*",

      // Additional attributes
      attributes: {
        min: "1",
        max: "31"
      }
    }
  ]
}) }}

[NickColley]

@colinrotherham yes, we'll still keep the attributes for users of the macro to add their additional attributes 👍

[colinrotherham]

Perfect. This is now supported by #1172

[malcolmhire]

This looks good to me 😀

[colinrotherham]

Can this be closed now or shall we keep it open for the further Nunjucks object merging changes?

[36degrees]

As far as I can tell this was addressed by #1172 (tests to show this behaviour here).

I'm going to close this as I think it's been done. If there is anything outstanding that still needs to be addressed please raise it as a new issue.