-
-
Notifications
You must be signed in to change notification settings - Fork 12
Hervin's Web Dev Test #9
base: master
Are you sure you want to change the base?
Conversation
Happy0
left a comment
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.
Looks good overall :).
You could improve your validation a little bit if you're up for a challenge.
Let me know if you need help with anything. Also, maybe Suzanne could help you with any comments you don't understand :).
| <li>Technology</li> | ||
| </ul> | ||
|
|
||
| <a class="yellow" href="#">click here to dowload a PDF of our recent work</a> |
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.
Good reuse of the 'yellow' class :)
| var buttons = document.querySelector("#submi"); | ||
| buttons.addEventListener('click', changesome); | ||
|
|
||
| function changesome() { |
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.
You could give your function a less general name.
A good function name describes what it is that your function does. You could summarise the changes that happen to your page when this function is called by your event listener.
| var button1value = button1.value; | ||
| if (button1value.length == 0) { | ||
| alert("please enter a Name"); | ||
| return |
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.
It's good style to add a ; at the end of a 'return' statement.
| return | ||
| } | ||
| var emailaddres = document.querySelector('#inputEmail'); | ||
| if (emailaddres.value.indexOf('@') === -1) { |
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.
You could improve your validation a bit further by validating that the E-mail address does not begin or end with '@' and only contains one '@' symbol. For example:
@gordon@.blah.com would not be a valid E-mail address.
You could also make sure that the E-mail address does not contain any symbols (like ^%&*!"().) hi£$"%@things.com would not be valid, for example.
And you could make sure that there is a '.' somewhere after the '@'.
Note: it's difficult to validate E-mail addresses properly, so we would probably use a library to do it for us. But it would be good practise for you to try those things above :).
| @@ -0,0 +1,34 @@ | |||
| body{ | |||
| background-image: url("assets/background.png"); | |||
| background-size: 100% 100%; | |||
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.
This looks as though it might not be valid CSS :P.
Maybe background-size: 100%; instead?
No description provided.