-
Notifications
You must be signed in to change notification settings - Fork 6
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
disables html5 validation #267
base: 2.x
Are you sure you want to change the base?
Conversation
Immediate thought: To install most localgov_ modules you have to have this module. This disabled HTML5 validation on all forms. Plenty of sites have been extend to have non-localgov_ modules installed, could any of these modules, or things otherwise built into the site, be expecting HTML5 validation to be enabled? |
Suggest we add a setting $settings['localgov_enable_html5_validation'] = TRUE; and detect it using if (Settings::get('localgov_enable_html5_validation') !== TRUE) {
... remove html5 validation. This is because as above there may be cases where modules would expect html5 validation to be present, or may not have any other validation. So whilst we should encourage the GDS recommendation of using validation errors that allow for more descriptive text, if there are cases where this isn't set up correct there is a chance to opt out of this change. |
Or we could just use the module which allows people to turn it off easily. https://www.drupal.org/project/disable_html5_validation Then we enable it on a default install, but it is optional for existing installs. Thoughts @andybroomfield @ekes @markconroy ? |
Discussing in Merge Tuesday, general agreement that it would be good to make it optional. @andybroomfield mentioned that in general it would be good to reduce dependencies. I'm in favour of using the module! hmmm.... to be discussed further.... |
I have no preference for doing this via the contrib module or via a custom hook. It's surprising that Drupal core has such good form validation, but doesn't disable it by default. So maybe we should go with the module approach to follow that practice. |
I'm generally not in favour of adding modules for one line that you could include elsewhere. OK maybe more than generally. I can't think of another occasion I think it's a good idea. But in this case. It feels like it makes sense. It is explicit. It's easy for end UI admins to understand. |
Discussed on MT on 28/1/2025. We thought that switching off the module is better as it allows site administrators to turn it off if they encountered a problem. We will install the module in the profile and then switch it on by default. |
Copy of to use here instead of there.