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 length validation to text input #3193

Merged
merged 1 commit into from Mar 1, 2018

Conversation

bpedersen2
Copy link
Contributor

This change allows to enforce minimum and maximum length
limit and/or pattern restrictions for regform text input fields.

@ThiefMaster ThiefMaster changed the base branch from v2.1-dev to master January 12, 2018 14:58
@bpedersen2 bpedersen2 force-pushed the field_limits branch 3 times, most recently from 2e4bca3 to fcd1b69 Compare January 29, 2018 12:57
</label>
<input type="text" name="validpattern"
ng-model="formData.validPattern"
>
Copy link
Member

Choose a reason for hiding this comment

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

should go in the previous line (also two more times above)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

<input type="text" name="validpattern"
ng-model="formData.validPattern"
>
<i class="info-helper" title="{{ 'Validation pattern for input , see https://www.w3schools.com/jsref/jsref_obj_regexp.asp }}' | i18n }}"></i>
Copy link
Member

Choose a reason for hiding this comment

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

Even though w3schools isn't as awful as they used to be (but the still sell those phony "html certificates"), I don't want a link to them in Indico ;) (also, there shouldn't be a space before the ,).

Maybe letting users specify regex patterns here is a bit overkill anyway...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed the link.

Using pattern validation can e.g. be used if you need special input from the registrants (e.g only uppercaser, no spaces, etc.)

<input type="number" name="maxlength"
ng-model="formData.maxLength"
>
<i class="info-helper" title="{{ 'Maximimum allowed length' | i18n }}"></i>
Copy link
Member

Choose a reason for hiding this comment

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

to many spaces

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

{{ 'Validation pattern' | i18n }}
</label>
<input type="text" name="validpattern" ng-model="formData.validPattern">
<i class="info-helper" title="{{ 'Validation pattern(a javascript regualr expression) }}' | i18n }}"></i>
Copy link
Member

Choose a reason for hiding this comment

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

missing space before ( and a typo (regualr).

Anyway, I still don't like the idea of having the pattern option. Yes, it is powerful, but eventually we want server-side validation for this stuff and except when restricting yourself to a common subset of the regex features of HTML5/JS and Python, that won't be guaranteed to work on both sides...

Also, having user-provided regexps run on the server may be risky - usually there are some ways to write regular expressions that result in horrible performance.

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 splitted this part out, as regexp validation is not too important for now.

ng-class="{
hasError: validationStarted && nestedForm.$invalid
}"/>

<span ng-show="validationStarted && nestedForm.$invalid && nestedForm.$error.minlength" class="error-message-tag">
Copy link
Member

Choose a reason for hiding this comment

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

double space before class (same below)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@ThiefMaster ThiefMaster changed the title Add length and pattern validation to text input Add length validation to text input Feb 12, 2018
@bpedersen2 bpedersen2 force-pushed the field_limits branch 3 times, most recently from 6e5cc39 to c213091 Compare February 19, 2018 17:02
@bpedersen2
Copy link
Contributor Author

Ping...

This change allows to enforce minimum and maximum length
limit for registration form text input fields.
@ThiefMaster ThiefMaster merged commit 972375e into indico:master Mar 1, 2018
@ThiefMaster
Copy link
Member

ThiefMaster commented Mar 1, 2018

Don't worry, we didn't forget about it - just busy with other things ;)

It's cleaned up (typos etc.) and merged now. Thanks for the PR!

@bpedersen2 bpedersen2 deleted the field_limits branch March 2, 2018 07:07
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

2 participants