Conversation
| tweetContent.apply(this[2]); | ||
| } | ||
|
|
||
| var updateDOM = function() { |
There was a problem hiding this comment.
This is just the profile info and not the whole DOM manipulation. Use more specific function names
| <div> | ||
| <div class="banner"></div> | ||
| <div class="profilepic"> | ||
| <div><img src="img/profile.png"></div> |
There was a problem hiding this comment.
unnecessary div tags in many places
| var div = ''; | ||
| var rightbottom = document.querySelector(".rightbottom"); | ||
| for(var i in data) { | ||
| var div = div + `<div class="indposts"> |
There was a problem hiding this comment.
The JSON response does not have an image in it, otherwise I would implement it.
| "https://fsd1.herokuapp.com/users/1/followers/suggestions", | ||
| "https://fsd1.herokuapp.com/users/1/following/tweets"]; | ||
|
|
||
| var consolidate = urls.map(url => fetch(url).then(res => res.json())); |
There was a problem hiding this comment.
Add a .catch statement to handle promise rejection errors.
| "https://fsd1.herokuapp.com/users/1/following"]; | ||
|
|
||
| var consolidate = urls.map(url => fetch(url).then(res => res.json())); | ||
| Promise.all(consolidate).then(data => acceptURLs.call(data));//.then(res => console.log(res)); |
There was a problem hiding this comment.
Always make it a point to add .catch so that you handle the errors that might arise.
| var photosAndVideos = document.querySelector(".photosAndVideos"); | ||
| div = ''; | ||
| for(var i = 0; i < 9; i++) { | ||
| var div = div + `<div class="mediaSquare"></div>`; |
There was a problem hiding this comment.
This is where template strings come in. In most cases when you are joining two strings as above, you can replace it with template strings.
div = ${div} <div class="mediaSquare"></div>;
There was a problem hiding this comment.
Try identifying other places where you could use template strings.
| var month = months[date.getMonth() + 1]; | ||
| var day = date.getDate(); | ||
| var year = date.getFullYear(); | ||
| return day + " " + month + " " + year; |
There was a problem hiding this comment.
This would be a good place to use template strings.
| document.getElementById("login").style.display="block"; | ||
| } | ||
|
|
||
| function closeDialog() { |
There was a problem hiding this comment.
While closing the dialog also remove any dom messages you would have added there.
If there is a message saying "incorrect credentials" just before closing the dialog, the same message should not reappear when the user reopens it.
|
|
||
| function closeDialog() { | ||
| document.getElementById("signup").style.display="none"; | ||
| document.getElementById("login").style.display="none"; |
There was a problem hiding this comment.
Instead of closing both dialogs, prefer having separate methods for closing each or make a utility function for closing the dialogs.
| //isItTrue.someval1.call(smg); | ||
| isItTrue.someval1(submitButtonEnabled); | ||
| document.querySelector("#signupform > button").disabled = false; | ||
| document.querySelector("#signupform span:nth-child(" + nthChild + ")").innerHTML = ''; |
There was a problem hiding this comment.
Don't use nth-child unless actually necessary. This can be better handled using id selectors.
The reason nth-child is not recommended is that if you add another element in between, you would have to find and fix the parameters of all the elements after it.
| transform: translate(-50%,-200%); | ||
| } | ||
|
|
||
| #loginButton { |
There was a problem hiding this comment.
Prefer sticking to one format of naming CSS selectors.
Either use snake_case_like_this or camelCase.
| method: "POST", | ||
| body: JSON.stringify(user) | ||
| } | ||
| var resp = fetch('https://fsd1.herokuapp.com/users/login', |
There was a problem hiding this comment.
Add a .catch error handler in case the API call itself fails. This applies to all fetch requests(promises) that you create.
| } | ||
|
|
||
| let isItTrue = function() { | ||
| value1 = false; |
There was a problem hiding this comment.
Use more descriptive variable names instead of just value1, value2.
| cursor: pointer; | ||
| } | ||
|
|
||
| .modalContent div:nth-child(3) button:nth-child(1):hover { |
There was a problem hiding this comment.
Nesting CSS nth-child selectors is neither preferable nor recommended.
| <input type="email" name="email" placeholder="E-mail" required> | ||
| <input type="password" name="password" placeholder="Password" required minlength="8"> | ||
| <input type="password" name="confirmpassword" placeholder="confirm Password" required> | ||
| <form name="signupform" id="signupform" method="POST" onsubmit="signupUser()"> |
There was a problem hiding this comment.
Sign up is not working. This function isn't defined.
| </form> | ||
| <div style="position: absolute; bottom: 10px; padding: 10px;"> | ||
| <button onclick="closeEditProfile()">Cancel</button> | ||
| <button onclick="closeEditProfile()">Save</button> |
There was a problem hiding this comment.
You have to perform the same validations and make an API call to save these updates.
3 participants
Refactored the /homepage and /profilepage code