x-govuk / govuk-form-builder

A form builder for Ruby on Rails that’s compatible with the GOV.UK Design System.
https://govuk-form-builder.netlify.app
MIT License
75 stars 21 forks source link

Legends should not default to using an H1 #201

Closed fofr closed 4 years ago

fofr commented 4 years ago

Including an H1 in a legend by default is not semantically correct.

When writing:

legend: { text: "Legend text" }

The following HTML is output, defaulting to using an H1 tag:

<legend class="govuk-fieldset__legend govuk-fieldset__legend--m">
  <h1 class="govuk-fieldset__heading">Legend text</h1>
</legend>

When using the official Nunjucks macro:

fieldset: {
  legend: {
    text: "Legend text"
  }
}

The default HTML does not include an H1:

<legend class="govuk-fieldset__legend">
  Legend text
</legend>

Headings only get included with the isPageHeading flag:

https://github.com/alphagov/govuk-frontend/blob/master/src/govuk/components/fieldset/template.njk#L8-L14

edwardhorsford commented 4 years ago

Strong agree - ideally we'd have the same defaults as the design system.

aliuk2012 commented 4 years ago

I agree, it should be an optional setting called something like "isPageHeading" - link to options

peteryates commented 4 years ago

Yeah, this makes sense. I originally built it based purely off the examples on this page, both of which wrap it in <h1>. As I never really looked into the Nunjucks it didn't occur to me to allow non-header legends.

It's possible to set the tag to another header level by passing in tag: 'h3' or whatever in the legend options, and also possible to set the default tag in the config:

GOVUKDesignSystemFormBuilder.configure do |conf|
  conf.default_legend_tag  = 'h3'
  conf.default_legend_size = 'l'
end

Unfortunately, it's currently not possible to omit the tag altogether, so I think once that's implemented all of the desired functionality will be present. The equivalent of isPageHeading will be achievable by passing in tag: 'h1' and the default behaviour will be no tag (tag: nil) which will just output <legend class="govuk-fieldset__legend">Legend text</legend>.

fofr commented 4 years ago

@peteryates Agree about the examples being misleading, I've raised this issue: https://github.com/alphagov/govuk-design-system/issues/1364

peteryates commented 4 years ago

Fixed by #204, will be included in v2.1.0 shortly.