Anthony's current changes to WebDevelopertest#7
Anthony's current changes to WebDevelopertest#7anthonythuo2320 wants to merge 5 commits intoCodeYourFuture:masterfrom
Conversation
Happy0
left a comment
Happy0
left a comment
There was a problem hiding this comment.
Good work :).
I've added some recommendations. Let me know if you need me to explain any of them / any help.
| @@ -0,0 +1,3 @@ | |||
| body{ | |||
| background; | |||
There was a problem hiding this comment.
This CSS 'background' property has no value ( property: value; ).
Perhaps you were playing around with something and meant to remove it?
| var button = document.querySelector('#newsButton'); | ||
| button.addEventListener('click', doSomething); | ||
|
|
||
| function doSomething() { |
There was a problem hiding this comment.
Now that you've got the hang of how to use 'addEventListener', you should give your function a better name.
'doSomething' doesn't tell me as the reader what changes happen to your HTML page when the button is clicked.
Can you think of a better name which describes what the function does? :)
| button.addEventListener('click', doSomething); | ||
|
|
||
| function doSomething() { | ||
| //if my <p>is showing hide it,else show it |
| var xhr = new XMLHttpRequest(); | ||
| xhr.onreadystatechange = function () { | ||
| if (xhr.readyState === 4) { | ||
| if (xhr.status === 200) { |
There was a problem hiding this comment.
You can combine these two if statements into one by using ' && '
For example:
https://www.w3schools.com/js/tryit.asp?filename=tryjs_comparison_and
|
|
||
| } | ||
|
|
||
| } else { |
There was a problem hiding this comment.
Could this 'else' be removed? or do you plan to add things to it?
|
Let me know if you need any help with the validation (making sure the user inputs a phone number that looks valid, etc.) |
2 participants
No description provided.