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

Version 2.0.0 #175

Merged
merged 52 commits into from
Sep 27, 2020
Merged

Version 2.0.0 #175

merged 52 commits into from
Sep 27, 2020

Conversation

peteryates
Copy link
Member

@peteryates peteryates commented Aug 13, 2020

This is the first big revision to the form builder that contains some breaking changes. The plan is to release it when the next version (3.9.0) of govuk-frontend lands.

The first beta is pencilled in for late August so there will be plenty of time for the changes to be tested.

Rationale

Some decisions were made early on that didn't allow for much flexibility. These were added to more recently and there was a gulf between customisable and non-customisable parts of the component. For example, label can be a custom size, weight and have a custom tag, or be totally overwritten with content provided via a proc but hint can't be customised at all.

With these changes hint and form_group will act more like label, legend and caption. Any 'extra' arguments (**kwargs) will be added as HTML attributes and text will be set using the hint: { text: 'something' } format rather than hint_text: 'something'.

In the case of hints, when a proc is passed in the hint element will be a div instead of a span to ensure the generated HTML is valid.

Change log

Remaining tasks

  • Release some betas so the changes can be properly evaluated before the proper release
  • Create an upgrade guide page. Not sure if it warrants a page in the actual guide or a Github ticket
  • Investigate creating a quick sed script to make the changes. It's probably going to be a pain because Ruby has about 50 ways to declare a string 😓
  • Do some testing with real projects, DFE-Digital/apply-for-teacher-training is a good place to start

Upgrade Guide

This is draft content for an actual upgrade guide which will be part of the v2.0.0 release notes.

This is a manual process. Due to Ruby's flexibility when it comes to declaring strings automating this process will probably take longer than just doing it.

These changes apply to the keyword arguments for all of the form helpers.

hint_texthint

Old syntax New syntax
hint_text: "your hint goes here" hint: { text: "your hint goes here" }

Hints that stretch across multiple lines must be tackled differently. Here's an example of replacing a multiline hint using a partial.

-          <%= f.govuk_radio_button :service, :apply, label: { text: 'Yes, I want to apply using the new service', size: 's' }, link_errors: true, hint_text:
-            "<p class=\"govuk-body govuk-!-margin-top-2\">Choose this option if:</p>
-            <ul class=\"govuk-list govuk-list--bullet\">
-              <li>all your chosen providers are available on the new service</li>
-              <li>you want to try a streamlined service with personalised support</li>
-            </ul>".html_safe %>
+          <%= f.govuk_radio_button :service,
+            :apply,
+            label: { text: 'Yes, I want to apply using the new service', size: 's' },
+            hint: -> { render(partial: 'apply_using_new_service_hint') },
+            link_errors: true
+          %>

Another approach, this time inline, should work too:

<%= f.govuk_radio_button :service,
  :apply,
  label: { text: 'Yes, I want to apply using the new service', size: 's' },
  link_errors: true,
  hint: -> do %>
   <ul class="govuk-list govuk-list--bullet">
      <li>all your chosen providers are available on the new service</li>
      <li>you want to try a streamlined service with personalised support</li>
   </ul>
<% end %>

form_group_classesform_group

form_group_classes: %w(polka-dots) becomes form_group: { classes: %w(polka-dots) }

Finding offending code

The following command finds all instances of hint_text in app/views, app/components and app/helpers with ag. The same approach can be used for form_group_classes.

ag "hint_text\:" app/{views,components,helpers}

Legends are now set by default

This shouldn't affect most projects who'll already have set their legend text via params or localisation, but legends behaviour has been made more consistent with labels. To omit a legend from a fieldset associated with a field (those generated with #govuk_fieldset remain unaffected) the legend now needs to be set via legend: nil. If the legend keyword argument isn't provided the legend will default to the name of the attribute.

For example, the legend for this field would be Project_start:

Note. the underscore being left is intended behaviour, it's meant to prompt the developer to override it accordingly

= form.govuk_date_field(:project_start)

Automatic addition of hidden fields required for checkbox deselection

Previously it was up to the developer to add a #hidden_field to a #govuk_check_boxes_fieldset to allow all selections to be deselected. This behaviour is now included by default. To prevent one from being added to single fields (ie boolean toggles or confirmations) multiple: false can be passed to #govuk_check_boxes_fieldset to suppress the hidden field.

Screenshot from 2020-09-05 09-51-33

Instead of only allowing a form group's classes to be configured, this
change replaces the form_group_classes argument with form_group which
takes a hash.

Adding a classes key to the hash will append these extra classes to the
required ones (.govuk-form-group). Any other key/value pairs will be
added as attributes to the form group element.
Now it's more in-line with labels, legends and captions in how it works
This makes hints behave more like labels, captions and legends.

However, there is a slight difference based on the kinds of arguments
passed. The following behaviour only applies to hints.

* When a single string of text is supplied, the hint is wrapped in a
  `span` tag.

* When a block of HTML is supplied, the hint is wrapped in a `div`. This
  is because rendering block level elements inside a span results in
  invalid HTML[0][1]

[0] alphagov/govuk-frontend#1855
[1] https://developer.mozilla.org/en-US/docs/Web/HTML/Element/span
@peteryates peteryates added the 2.0.0 Version 2.0.0 label Aug 13, 2020
@aliuk2012
Copy link

As its a breaking change is there any chance we could try align more with GOV.UK Frontend’s public API?
I understand the syntax format/style cannot be matched but at least the functionality/features should be.

@peteryates
Copy link
Member Author

If there are extra arguments that the Nunjucks macros take that the form builder doesn't, that offer more customisation and can be cleanly integrated, I'm all for it. If there's anything you think is missing please raise a ticket (or PR) and I'll definitely have a closer look.

I'm against the idea of passing in complex JSON and raw HTML though; I think the current approach with procs is much cleaner and the API still bears a strong resemblance with Rails' built in form builder.

The intention from the start was to have something that anyone familiar with Rails would be able to grasp in fifteen minutes.

Not that it's entirely related but did you see the proof of concept I made that allows the Nunjucks macros to be called from Ruby? Perhaps V3 could have swappable backends!

@edwardhorsford
Copy link

I think there should be a target that the full set of options available in GOV.UK Frontend should be supported. Full feature parity.

The actual api could possibly differ - but it's 'cleaner' for us if the capability is the same.

Note: this was one of our principles on the BEIS Rails design system port.

@peteryates
Copy link
Member Author

I'm pretty sure that all of the functionality shown in the GOV.UK Design System Components section is implemented.

If there are features that the Nunjucks macros offer that are missing here I'll gladly look into adding them, perhaps someone familiar with both could create a list of discrepancies. Of course, PRs are always welcome.

Make hint arguments follow convention set by label, legend, etc
Due to the fieldset radio button and check box having their own,
separate #hint_element method and it not being updated when hint procs
were implemented they didn't fully support the proc functionality.

This behaviour has been standardised and they now work in the same way
as the other form helpers.
@peteryates
Copy link
Member Author

I did a quick test with Apply's test sutie and having made a few substitutions (with vimnastics 🤸) all the tests pass.

As a bonus some of the previously-invalid HTML is now valid 🎉

This is required to prevent unforeseen things from happening when procs
are called in actual templates

Refs #178, #183
To make the behaviour of fieldset legends more consistent with labels,
they will now be added to the form by default if nothing is specified.

This is intended to act as a prompt for the developer to modify the
content, so no additional formatting (ie removing underscores) is
undertaken.

If the developer wants the legend to be omitted they can pass `nil` to
in legend param.

Fieldsets with no attribute name (ie standalone fieldsets called via
`govuk_fieldset`) will remain unaffected.

Fixes #142
Some of the instance variables set in Label's initializer are required
for both proc and hash style labels. When set using a proc they are
missed and it causes the ids and radio/checkbox-specific classes not to
be set properly.

They have now been moved out, only the variables used to configure the
appearance of the label are now set in the else clause.
…assignment-bug

Move instance variable assigns out of else block
Adding extra attributes by passing additional keyword args was added to
hints in aec13ce - this commit makes the behaviour consistent for
labels, legends and captions.
@codeclimate
Copy link

codeclimate bot commented Sep 4, 2020

Code Climate has analyzed commit b11396b and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 99.8% (0.0% change).

View more on Code Climate.

When forms are submitted, when a field has no check boxes checked it's
omitted from the form's payload[0]. To combat this, Rails adds a hidden
field with no value before the check boxes are rendered. This change
makes the form builder do the same and adds a check to ensure its
presence.

This behaviour is already provided by #govuk_collection_check_boxes
which wraps Rails' functionality more closely.

[0] https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/checkbox#Value
…ns-in-check-boxes-fieldset

Add missing hidden field needed for check box deselection
Increase GOV.UK Design System version to 3.9.0
This change adds support for the new prefix and suffix text support in
GOV.UK frontend. It allows text to be added before and after text inputs
in a grey box to give users more context around the data, for example
stating the currency of a expected monetary value.

Refs #159
According to the design system guidelines:

> Prefixes and suffixes are visual aids and are not available to
  assistive technologies.

Refs #159, #161
Add prefix and suffix text support
This allows us to suppress the generation of a hidden field by the
fieldset helper and allows Rails's check_box helper to generate its own,
providing `multiple: true` is set on both the fieldset and the check
box.

As this is a little unintuitive it has been documented in the guide and
API docs
…heck-boxes-fieldset

Add multiple flag to govuk_collection_check_boxes
@peteryates
Copy link
Member Author

Beta 4 is up. Hopefully it's the final one.

@peteryates peteryates marked this pull request as ready for review September 26, 2020 11:43
@peteryates peteryates merged commit 6d08b77 into master Sep 27, 2020
@peteryates peteryates deleted the version-2.0.0 branch September 27, 2020 15:13
@peteryates peteryates restored the version-2.0.0 branch September 27, 2020 15:25
@peteryates peteryates deleted the version-2.0.0 branch September 27, 2020 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.0.0 Version 2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants