x-govuk/govuk-form-builder

Setting a JSON object as an HTML attribute on form elements doesn't work

Closed this issue · 1 comments

Creating a radio button using something like:

<% object = {"some_key"=>["Yes - a regular", "Yes - a reserve"], "another_key"=>["Yes - a regular", "Yes - a reserve"]} %>

<%= f.govuk_radio_button question_key, val, label: { text: val },
            "data-controller": "stimulus-controller",
            "data-action": "click->stimulus-controller#method",
            "data-info": object.to_json
        %>

Means that here https://github.com/DFE-Digital/govuk-formbuilder/blob/master/lib/govuk_design_system_formbuilder/traits/html_attributes.rb#L27 Form Builder treats the json value as a string to split and duplicate words are removed. This means that instead of correctly outputting the following HTML:

data-info="{&quot;some_key&quot;:[&quot;Yes - a regular&quot;,&quot;Yes - a reserve&quot;],&quot;another_key&quot;:[&quot;Yes - a regular&quot;,&quot;Yes - a reserve&quot;]}

You get

data-info="{&quot;some_key&quot;:[&quot;Yes - a regular&quot;,&quot;Yes reserve&quot;],&quot;another_key&quot;:[&quot;Yes reserve&quot;]}

(Note. "Yes - a reserve" has been turned into "Yes reserve").

You can fix this by modifying Attributes#deep_split_values to something like:

when String
                            begin
                              JSON.parse(value)
                              value
                            rescue JSON::ParserError => e
                              split_mergeable(key, value, parent)
                            end

But that seems likely to break other use cases? Is there another supported way to do this currently or would a change to enable it be viable?

This is a bit of a tricky one.

Currently there's this list of special HTML attributes that we shouldn't merge.

https://github.com/DFE-Digital/govuk-formbuilder/blob/9b0a456b4191b00a6da7689dd9918d3a282662bf/lib/govuk_design_system_formbuilder/traits/html_attributes.rb#L13

We might be better off switching to just listing the attributes we do, that would be all those that contain a space-separated list of values.

I suspect it'd be something along the lines of:

 MERGEABLE = [%i(class) %i(aria describedby), %i(aria labelledby), %i(some more)].freeze

I think this would avoid the problem and probably end up cleaner. I don't recall why I did it this way round originally so will have an experiment and see how feasible switching around is.