bike for refugee #4
Conversation
IsmaelMartinez
left a comment
IsmaelMartinez
left a comment
There was a problem hiding this comment.
Good job. Some comments added. I hope it helps.
js/homework.js
Outdated
| // create an Event and call the function | ||
| function changeBlueColor() { | ||
| var myJumbo = document.querySelector('.jumbotron'); | ||
| var myDonate = document.querySelector('.btn-primary') |
There was a problem hiding this comment.
Try not to use classes as a selector if you are targetting an element that got an id.
The reason is that a class can be repeated across the dom, while an id can only be once present.
Same applies with the other query selectors.
js/homework.js
Outdated
| // create an Event and call the function | ||
| function changeOrangeColor() { | ||
| //change the color of selected query | ||
| var myJumbo = document.querySelector('.jumbotron'); |
There was a problem hiding this comment.
If you are defining the same selectors as variables in your js file, put them once at the top of the file.
As long as they got the var at the front they will have a local scope to that "file". That way you can:
- Reduce the complexity of your code by reducing the number of lines
- Avoid needing to change multiple lines when a selector changes
That also produces other benefits like ease to read what, as a side effect, make for bugs to be easier to be found and you to be happier ;)
| var emailAddress = document.querySelector('#exampleInputEmail1'); | ||
| var textArea = document.querySelector('#exampleTextarea'); | ||
| var isEmail = false; | ||
| for (i = 0; i < emailAddress.value.length; i++) { |
There was a problem hiding this comment.
You can use emailAddress.value.indexOf('@') instead of the for loop. It basically returns the 1st position of @ in the String. If not present it returns -1.
You will need to check that emailAddress is neither null or empty. as simple as:
if (emailAddress && emailAddress.value.indexOf('@') !== -1)
Can replace the for and if lines.
Check the following link for more info: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/indexOf
| } | ||
| } | ||
|
|
||
| if (!isEmail) { |
There was a problem hiding this comment.
I will prefer to split the different validations in small functions that return true (when valid) or false (when invalid)
| } | ||
|
|
||
|
|
||
| var addArticle = document.querySelector('#addArticleBtn'); |
There was a problem hiding this comment.
I assume all of this is an extra. I am not sure what is the full intention so will no code review further but do follow the same principles as before.
3 participants
I Finished the two part of the homework; i create a JS for clicking the button and the form validation