Various fixes for html-validate (cookie banner buttons)
#3116
Conversation
colinrotherham
added
assurance
cookie banner
🐛 bug
✅ You can preview this change here:
To edit notification comments on pull requests, go to your Netlify site configuration. |
colinrotherham
force-pushed
the
cookie-banner-buttons
branch
from
c887f2c to
e05e22e
Compare
colinrotherham
changed the title
html-validate (cookie banner buttons)
colinrotherham
force-pushed
the
html-validate-fixes
branch
from
d540a4e to
a448bd6
Compare
colinrotherham
force-pushed
the
cookie-banner-buttons
branch
from
e05e22e to
b7e8a4f
Compare
| } | ||
| ] | ||
| }) }} | ||
| <form method="post"> |
There was a problem hiding this comment.
Not sure about this change, as it means clicking the accept / reject buttons in the example results in a POST to /styles/page-template/custom/.
On the Netlify preview, that results in a 404 with no page content (displays as a white screen in the iframe, and a Chrome error page if opened in a new tab). Not sure what the behaviour would be on PaaS.
I appreciate technically it's needed, but wonder if we should accept that this is only meant to be an illustration of how to customise parts of the page template and omit the form?
There was a problem hiding this comment.
That's a very good point
How about we wire up preventFormSubmission() like on example pages?
| <form method="post"> | |
| <form action="/form-handler" method="post" novalidate> |
There was a problem hiding this comment.
✅ Pushed. The submit event is prevented like all the other iframes
.htmlvalidate.js
Outdated
| // Allow implicit type="button" without type attribute | ||
| 'no-implicit-button-type': 'off', |
There was a problem hiding this comment.
This is still needed because we haven't shipped the changes in alphagov/govuk-frontend#4113 yet, right?
I think if you don't have that context it's 'surprising' to see this included, given the other changes in this PR mean that we otherwise shouldn't need to disable the rule.
Suggest we either split this out, maybe into #3144, or at least add a note to the commit to explain why it's needed.
There was a problem hiding this comment.
Good thinking. I've removed it from this PR as the rule doesn't exist yet until #3144
These examples currently use name/value pairs for form submission but prevent it with `type="button"` Some necessary features for client-side use are missing too: 1. Message hidden by default users without JavaScript 2. Additional confirmation messages for accept/reject 3. Usage of `role="alert"` for confirmation messages They can be better shown for server-side usage instead
Helps to reduce setup time by sharing form field name/value across pages E.g. The same server-side middleware can respond to form submissions from either the banner or page
colinrotherham
force-pushed
the
cookie-banner-buttons
branch
from
b7e8a4f to
f72a9a8
Compare
Although Design System buttons were all fixed in #3116 we need this line until alphagov/govuk-frontend#4113 is released
Follows on from #3115 to fix a few things spotted by
html-validatetype="button"to Hide cookie message client-side buttonstype="submit"to Hide cookie message server-side buttonshref="#"from Hide cookie message server-side buttonsnameandvaluefrom various client-side examplesTo solve a gotcha I've hit in the past too, the Cookie banner and Cookies page form fields now match
E.g. The same server-side middleware could respond to form submissions from either the banner or page
Customised page template
I've updated the Customised page template cookie banner example to show server-side usage
Some client-side features from the guidance were missing such as:
role="alert"for confirmation messagesBut we can add those missing features if we'd prefer it to show client-side usage instead