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

Update validation masthead pattern #439

Merged
merged 1 commit into from
May 29, 2018

Conversation

samuelhwilliams
Copy link
Contributor

@samuelhwilliams samuelhwilliams commented May 27, 2018

Summary

GOV.UK validation masthead patterns use h2 rather than h1 for their
validation mastheads which appear at the top of the page indicating some
errors during form submission. There are also accessibility and general
HTML-compliance issues with having more than one h1 on the page. Best
practice and guidance therefore indicates we should change our template
to using an h2 heading.

Finally, we include the validation.html in the base page layout for
the frontend toolkit, ensuring that all pages will correctly show
validation when errors are passed into the rendering template as
errors. This means that individual pages no longer need to include the
boilerplate code for including validation and it's available out of the
box.

Ticket

https://trello.com/c/RSYEM3FL/448

@samuelhwilliams samuelhwilliams force-pushed the shw-validation-masthead-to-h2 branch 2 times, most recently from ca1f1c9 to d334a90 Compare May 27, 2018 09:08
@samuelhwilliams samuelhwilliams changed the title Use h2 rather than h1 for validation Update validation masthead pattern May 27, 2018
@samuelhwilliams samuelhwilliams force-pushed the shw-validation-masthead-to-h2 branch 9 times, most recently from e71a0f7 to a6ee660 Compare May 27, 2018 10:43
samuelhwilliams pushed a commit to Crown-Commercial-Service/digitalmarketplace-supplier-frontend that referenced this pull request May 29, 2018
 ## Summary
Use validation masthead from the frontend-toolkit consistently.

Also converts most pages over to using the base page from the frontend-toolkit. This should enable the `report-a-problem` feature on all pages, however there appears to be an incompatibility between this feature and the `show-hide-content` feature already in use, so the required javascript for `report-a-problem` has not been included. A tech debt ticket tracking this problem has been logged at https://trello.com/c/PdOmr8NH/469.

 ## Ticket
https://trello.com/c/RSYEM3FL/448

Crown-Commercial-Service/digitalmarketplace-frontend-toolkit#439
Crown-Commercial-Service/digitalmarketplace-utils#398
### What changed
The validation masthead in `toolkit.forms.validation` is now an h2 rather than an h1. This may break associated unit and functional tests. Manual review will be needed here.

The pattern now expects to reference a variable called `errors`, so you will need to pass all form errors into the templating engine as `errors=errors` or `errors=get_errors_from_wtform(form)` when calling `render_template()`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should make it clear that errors is now a nested dict rather than a list of dicts. This will potentially affect the ordering of validation messages (which was an issue for the password change form errors in the User FE). Maybe add a New code sample 3 with an example of all the stuff that's needed for the toolkit template:

ordered_form_errors = OrderedDict([
    (`key1`, {'question': 'First question', 'input_name': 'new_password', 'message': 'Not long enough'}),
    (`key2`, {'question': 'Second question, 'input_name': 'repeat_new_password', 'message': 'Does not match'}),
])
return render_template('blah.html', errors=ordered_form_errors)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah - just read through the utils PR 😹 Maybe an OrderedDict example isn't needed but I think it's still worth being clear what format we expect to be returned from get_errors_from_wtform() (and emphasise here that we should be using it where possible).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea - I'll clarify this documentation a tad. 👍

 ## Summary
GOV.UK validation masthead patterns use `h2` rather than `h1` for their
validation mastheads which appear at the top of the page indicating some
errors during form submission. There are also accessibility and general
HTML-compliance issues with having more than one h1 on the page. Best
practice and guidance therefore indicates we should change our template
to using an h2 heading.

 ## Ticket
https://trello.com/c/RSYEM3FL/448
@samuelhwilliams samuelhwilliams force-pushed the shw-validation-masthead-to-h2 branch from a6ee660 to fee9cab Compare May 29, 2018 12:23
@samuelhwilliams
Copy link
Contributor Author

Docs updated.

Copy link
Contributor

@katstevens katstevens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff 👍

@samuelhwilliams samuelhwilliams merged commit 87b8535 into master May 29, 2018
@samuelhwilliams samuelhwilliams deleted the shw-validation-masthead-to-h2 branch May 29, 2018 13:28
samuelhwilliams pushed a commit to Crown-Commercial-Service/digitalmarketplace-user-frontend that referenced this pull request May 29, 2018
 ## Summary
Use validation masthead from the frontend-toolkit consistently.

Also converts most pages over to using the base page from the frontend-toolkit. This should enable the `report-a-problem` feature on all pages, however there appears to be an incompatibility between this feature and the `show-hide-content` feature already in use, so the required javascript for `report-a-problem` has not been included. A tech debt ticket tracking this problem has been logged at https://trello.com/c/PdOmr8NH/469.

 ## Ticket
https://trello.com/c/RSYEM3FL/448

Crown-Commercial-Service/digitalmarketplace-frontend-toolkit#439
Crown-Commercial-Service/digitalmarketplace-utils#398
samuelhwilliams pushed a commit to Crown-Commercial-Service/digitalmarketplace-user-frontend that referenced this pull request May 29, 2018
 ## Summary
Use validation masthead from the frontend-toolkit consistently.

Also converts most pages over to using the base page from the frontend-toolkit. This should enable the `report-a-problem` feature on all pages, however there appears to be an incompatibility between this feature and the `show-hide-content` feature already in use, so the required javascript for `report-a-problem` has not been included. A tech debt ticket tracking this problem has been logged at https://trello.com/c/PdOmr8NH/469.

 ## Ticket
https://trello.com/c/RSYEM3FL/448

Crown-Commercial-Service/digitalmarketplace-frontend-toolkit#439
Crown-Commercial-Service/digitalmarketplace-utils#398
samuelhwilliams pushed a commit to Crown-Commercial-Service/digitalmarketplace-supplier-frontend that referenced this pull request May 30, 2018
 ## Summary
Use validation masthead from the frontend-toolkit consistently.

Also converts most pages over to using the base page from the frontend-toolkit. This should enable the `report-a-problem` feature on all pages, however there appears to be an incompatibility between this feature and the `show-hide-content` feature already in use, so the required javascript for `report-a-problem` has not been included. A tech debt ticket tracking this problem has been logged at https://trello.com/c/PdOmr8NH/469.

 ## Ticket
https://trello.com/c/RSYEM3FL/448

Crown-Commercial-Service/digitalmarketplace-frontend-toolkit#439
Crown-Commercial-Service/digitalmarketplace-utils#398
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.

2 participants