-
-
Notifications
You must be signed in to change notification settings - Fork 12
Finished #11
base: master
Are you sure you want to change the base?
Finished #11
Conversation
|
These are really good commit messages - well done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really good work! Just a couple of comments:
-
The field ‘Contact phone number’ is not required, so no need to force the user to put that in, unlike the email and name fields. An empty phone number field should be accepted when the form is submitted.
-
However there should be a check on the phone number field so that if something was entered into the field, you check whether it contains only digits and is a maximum of 11 characters long.
-
In the AJAX section, you're saving the data returned from the request twice to the DOM: once by using the renderHTML function and then again by attempting ".innerHTML = ourRequest.responseText". This is unnecessary and results in a console error because the renderHTML function doesn't return a DOM element so you can't use innerHTML on it. I recommend referencing/creating a DOM element right there and then using a function only to construct the string (with headers and such) that you want to place in it. Although there might be an even more elegant.
Let me know if you have any questions!
Irina :)
I try to finish the web dev test but I need a comment on it;