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

Fixes date field validation message presents in reverse order #72

Merged
merged 8 commits into from
Apr 23, 2024

Conversation

MattOz-CDS
Copy link
Contributor

Overrides the validateDate method in the Datebase class so that date validation error messages display a UK style date format e.g dd-mm-yyyy.
https://github.com/localgovdrupal/localgov_forms/issues/67

@MattOz-CDS MattOz-CDS marked this pull request as ready for review April 11, 2024 16:08
@finnlewis
Copy link
Member

Just discussing in Merge Tuesday with @MattOz-CDS @Adnan-cds @ekes

Thinking it would be safer to namespace the date format to something like:

localgov_forms_uk_html_datetime

This would keep it namespaced and reduce the chances of conflicts with existing date formats.

@andybroomfield
Copy link
Contributor

Is html_date an actual specified date format?

So maybe not calling it html date as thats for the spec.
It would be nice if this was configurable for form builder (not essential for this PR though).

@MattOz-CDS
Copy link
Contributor Author

Is html_date an actual specified date format?

So maybe not calling it html date as thats for the spec. It would be nice if this was configurable for form builder (not essential for this PR though).

Alright Andy, thanks for sharing your thoughts on this. The name choice was inspired by the names of other date formats that are present in our Drupal install. I assumed that this was an established naming convention, so that's why I went with it. I'm happy to change it, that's no problem. What would you recommend? I also agree that it should be configurable for site builders. I'm happy to do that as well, its just that I was looking for a quick win because this change is urgently needed.

@andybroomfield
Copy link
Contributor

I don't know, it was a question! :-)
localgov_forms_date_datetime is good.

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