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

check for new participant number between 1 and 100 on tracking page #84

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

lucyq
Copy link
Member

@lucyq lucyq commented Oct 26, 2017

resolves #76

@lucyq lucyq requested a review from frederickjansen October 26, 2017 17:48
@lucyq lucyq self-assigned this Oct 26, 2017
@@ -111,8 +118,8 @@ <h2 class="text-center">Add participants</h2>
<form>
<div class="form-group">
<label class="control-label" for="participants-count">New participants</label>
<input type="number" id="participants-count" class="form-control" placeholder="0" pattern="^[1-9]\d*{1,5}$"
autocomplete="off" required/>
<input type="number" id="participants-count" class="form-control" min="1" placeholder="0" pattern="^[1-9]\d*{1,5}$"
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't match the JS check

// Generate new participation links
$('#participants-submit').on('click', function (e) {
e.preventDefault();

var pc = parseInt($('#participants-count').val());

if (pc < 1.0 || pc > 100.00) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just use the number type with min and max? https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/number

Copy link
Member Author

Choose a reason for hiding this comment

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

Using the min/max only limits using the up/down arrows within the input box.
A user could still type in something below 1 or above 100.
the parseInt is to ensure that the participant count is an integer since users can input a fraction. should I floor/ceil instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can't believe I'm linking to W3Schools, but ... https://www.w3schools.com/html/tryit.asp?filename=tryhtml_input_number

The user can input a higher number, but when trying to submit the browser should stop them. Of course this is only a client-side check, and browser-dependent, so if we care enough we should implement the same check on the server.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we have already discussed this but the code above is doing the client-side check.
Otherwise just setting max/min doesn't execute a check.

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.

New participants input box on tracking page should not allow negative numbers.
2 participants