Skip to content
This repository was archived by the owner on Jan 3, 2023. It is now read-only.

Conversation

@anthonythuo2320
Copy link

No description provided.

Copy link

@Happy0 Happy0 left a comment

Choose a reason for hiding this comment

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

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;
Copy link

Choose a reason for hiding this comment

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

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() {
Copy link

Choose a reason for hiding this comment

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

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
Copy link

Choose a reason for hiding this comment

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

Nice use of comments :)

var xhr = new XMLHttpRequest();
xhr.onreadystatechange = function () {
if (xhr.readyState === 4) {
if (xhr.status === 200) {
Copy link

Choose a reason for hiding this comment

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

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 {
Copy link

Choose a reason for hiding this comment

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

Could this 'else' be removed? or do you plan to add things to it?

@Happy0
Copy link

Happy0 commented Jul 5, 2017

Let me know if you need any help with the validation (making sure the user inputs a phone number that looks valid, etc.)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants