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

Add input types to livevalidation #3336

Merged
merged 2 commits into from
Mar 19, 2023

Conversation

williamthome
Copy link
Contributor

Description

Livevalidation it's an old lib and not all input types existed back then.
This PR adds the following types:

  • color
  • date
  • datetime-local
  • month
  • week
  • time
  • range
  • search

Note
The change is from line 285 to 300, the rest are just trimmed whitespaces.

Checklist

  • documentation updated
  • tests added
  • no BC breaks

Comment on lines +285 to +300
if (nodeName == 'INPUT' && this.element.type.toUpperCase() == 'COLOR')
return LiveValidation.TEXT;
if (nodeName == 'INPUT' && this.element.type.toUpperCase() == 'DATE')
return LiveValidation.TEXT;
if (nodeName == 'INPUT' && this.element.type.toUpperCase() == 'DATETIME-LOCAL')
return LiveValidation.TEXT;
if (nodeName == 'INPUT' && this.element.type.toUpperCase() == 'MONTH')
return LiveValidation.TEXT;
if (nodeName == 'INPUT' && this.element.type.toUpperCase() == 'WEEK')
return LiveValidation.TEXT;
if (nodeName == 'INPUT' && this.element.type.toUpperCase() == 'TIME')
return LiveValidation.TEXT;
if (nodeName == 'INPUT' && this.element.type.toUpperCase() == 'RANGE')
return LiveValidation.TEXT;
if (nodeName == 'INPUT' && this.element.type.toUpperCase() == 'SEARCH')
return LiveValidation.TEXT;
Copy link
Member

@vkatsuba vkatsuba Mar 18, 2023

Choose a reason for hiding this comment

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

I suppose that this can be simplify like:
1 - Define a constant eg.:

const x = ['COLOR', 'DATE', 'DATETIME-LOCAL', ''WEEK', ...']

2 - Use includes() method for detect if item are exist: eg:

if (nodeName == 'INPUT' && x.includes(this.element.type))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with you, @vkatsuba. This code can be improved, but IMHO, this lib needs to be updated, reviewed, or changed by another one, like Zod. The majority of the code is still the same as the original one (the last commit date is Feb 13, 2010). I just kept the code semantic.
See this comment from @mworrell.

Copy link
Member

Choose a reason for hiding this comment

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

Yep. It was just a proposal.

@mmzeeman
Copy link
Member

Thanks @williamthome, and indeed, the validation code must be revised to include "modern" input types.

@mmzeeman mmzeeman merged commit ea6b266 into zotonic:master Mar 19, 2023
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.

None yet

3 participants