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

Breaking changes to form validation #405

Merged
merged 5 commits into from
Mar 16, 2017
Merged

Conversation

gemmaleigh
Copy link
Contributor

@gemmaleigh gemmaleigh commented Feb 15, 2017

What problem does the pull request solve?

The styling for form validation is pretty fragile and relies on a parent .error selector to apply a red border to the left of a .form-group div. This class also sets a red border on any .form-control inputs that are direct descendents.

Rather than relying on a child div sitting inside a parent for this to work - add two new classes .form-group-error and .form-control-error. One will set the border for form-groups and the other for inputs/textareas with a class of form-control.

This also solves the problem where there may be multiple .form-control elements within a parent .error - and only one of them needs to have a red border - an example of this is the date of birth pattern.

This would be a breaking change, but at some point we'll need to make breaking changes, so let's have a discussion about it here, rather than on Slack where it'll get lost.

Screenshots (if appropriate):

I have added the error examples to the date of birth page:

d2e321d8-1ef7-42b3-5198-6b3114ddef96

What type of change is it?

  • Breaking change (fix or feature that would cause existing functionality to change)

Has the documentation been updated?

  • My change requires a change to the documentation.
  • I have read the CONTRIBUTING document.

.form-group-error - to apply a red border to a .form-group
.fom-control-error - to apply a red border to a .form-control

This removes the need to set a parent ‘.error’ class - at present this
is setting both a border to the left of a form-group div and the border
on a form control.
This will apply a red border to the left side of a form-group div
Rather than setting a border on any .form-control that is a direct
child of the .error class, require that a new class is explicitly set
on any fields with an error - by using .form-control-error
Use this to show how the new form-control-error class can be applied to
individual form-control fields.
@NickColley
Copy link
Contributor

Could we remove the nesting if we were to move towards this approach?

@ztolley
Copy link
Contributor

ztolley commented Feb 15, 2017

I've had to code around this issue for things previously but using an 'incomplete' class so happy for this change to happen, even if it means I have to revisit some code. Agreed that it should be a major version change to avoid breaking code

@gemmaleigh gemmaleigh changed the title [For discussion] Breaking changes to form validation Breaking changes to form validation Mar 2, 2017
@robinwhittleton robinwhittleton added this to the 3.0.0 milestone Mar 16, 2017
@gemmaleigh
Copy link
Contributor Author

@NickColley's comments about nesting have been addressed, this is ready to merge @robinwhittleton.

@robinwhittleton robinwhittleton mentioned this pull request Mar 16, 2017
@robinwhittleton robinwhittleton merged commit 5a05f9c into master Mar 16, 2017
@robinwhittleton robinwhittleton deleted the form-validation branch March 16, 2017 12:30
quis added a commit to alphagov/notifications-admin that referenced this pull request Apr 10, 2017
aliuk2012 added a commit to ministryofjustice/gov_uk_date_fields that referenced this pull request May 3, 2018
This commit updates the markup that this gem produces so that its closer
inline with the markup that GOVUK Elements uses

https://govuk-elements.herokuapp.com/form-elements/example-date/

It also fixes an issue with error styling which was broken as a result
of GDS changing the css class name used to style error messages. The
class name changes from `error` to `form-group-error` also each input
field also receives a `form-control-error`

https://github.com/alphagov/govuk_elements/releases/tag/v3.0.0
alphagov/govuk_elements#405

As this commit introduces new markup and potentially breaking changes
for any services relying on the markup for styling or js interaction
may want to upgrade with caution.

In addition to this update if your project is using GOVUK Elements < 3.0
then you will also need to update your GOVUK Elements version
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 this pull request may close these issues.

4 participants