Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gem can generate invalid duplicate data attributes on buttons #529

Closed
DavidBiddle opened this issue Apr 5, 2024 · 0 comments · Fixed by #530
Closed

Gem can generate invalid duplicate data attributes on buttons #529

DavidBiddle opened this issue Apr 5, 2024 · 0 comments · Fixed by #530

Comments

@DavidBiddle
Copy link
Contributor

DavidBiddle commented Apr 5, 2024

In some cases, the gem can generate two identical data attributes on a single button.

Steps to reproduce

Create a button with a data-module attribute using the Rails nested hash convention:

<%= govuk_button_to 'A button', '#', data: { module: "special-js-button" } %>

observe that the generated HTML now contains two data-module atttributes:

<button class="govuk-button" data-module="govuk-button" new_tab="false" data-module="special-js-button" type="submit">A button</button>

When this happens, browsers will discard the second data-module value as it is invalid.

Note that if you use the full data-module attribute instead:

<%= govuk_button_to 'A button', '#', 'data-module': "special-js-button" } %>

the generated HTML only contains our custom data-module atttribute:

<button class="govuk-button" data-module="special-js-button" new_tab="false" type="submit">A button</button>

Expected behaviour

I would expect to be able to overwrite the data-module attribute using the nested hash convention.

Suspected cause

I think this is happening because of the way build_data_attributes works - it uses the full data-module attribute rather than the nested hash:

def build_data_attributes(data_module, prevent_double_click: nil)
    {
      "data-module": data_module,
      "data-prevent-double-click": prevent_double_click
    }.compact
  end

The govuk_button_to helper uses deep_merge_html_attributes which comes from the x-govuk/html-attributes-utils gem. Issue 1 in that repo says:

This library, and both the components and form builder assume people are using the latter. Using the former might lead to unintended consequences.

So I think the solution here might be to use the nested hash convention in build_data_attributes:

def build_data_attributes(data_module, prevent_double_click: nil)
    {
        data: {
          module: data_module,
          'prevent-double-click': prevent_double_click
        }.compact
    }
end
github-merge-queue bot pushed a commit that referenced this issue Apr 7, 2024
Uses the nested hash method of declaring data attributes in
build_data_attributes, as this is what `deep_merge_html_attributes`
expects.

Fixes #529
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant