-
Notifications
You must be signed in to change notification settings - Fork 70
SOLUTION: Ivan Escribano and Alicia Cembranos #57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| function getDataComments(postId) { | ||
| const containerComments = document.getElementById("commentsContainer"); | ||
| containerComments.textContent = ""; | ||
| const commentsURL = `http://localhost:3000/comments`; |
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.
Create const variable on top --> More reusable code
RogerOliveDelgado
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.
Great Job!
| @@ -0,0 +1,611 @@ | |||
| //!GENERAL VARIABLES | |||
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.
Well structured in functions, will be a good idea to refactor code in more files
| postContainer.scrollTop + postContainer.clientHeight >= | ||
| postContainer.scrollHeight | ||
| ) { | ||
| createSkeleton(4); |
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.
Don't kill me please. Will be fine to declare 4 as a variable, to avoid magic number
| let idPost = post.id; | ||
| let imagePost = getImagesSplash(99); | ||
| removeSkeleton(); | ||
| createHTMLpostSection(imagePost, titlePost, usernamePost, idPost); |
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.
Love function name, super clear :)
| divsCarousel[0].classList.add("active"); | ||
| } | ||
|
|
||
| async function createHTMLsliderSection(image, title, id) { |
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.
Should it work without async? There is no promise inside function.
| carouselContent.append(div); | ||
| } | ||
|
|
||
| async function createHTMLpostSection(image, title, username, id) { |
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.
Same. Should async be written?
| //!GET IMAGE FROM UNSPLASH SOURCE | ||
| function getImagesSplash(index) { | ||
| const randomNumber = randomIndex(index); | ||
| const srcImages = `https://source.unsplash.com/16${randomNumber}x9${randomNumber}/`; |
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.
Should be a good idea to define URL on top, to have a more maintainable code
| } | ||
|
|
||
| async function fetchToServerPosts(id, info = true) { | ||
| const urlPost = `http://localhost:3000/posts/${id}`; |
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.
URL on top. Should this function be async? Not returning a value. I write a question, because we are not sure about it
| // const result = await responseDELETE.text(); | ||
| return false; | ||
| } catch (error) { | ||
| console.log(error); |
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.
We have used console.error(error), but we don't know what should be a better practice
| @@ -0,0 +1,611 @@ | |||
| //!GENERAL VARIABLES | |||
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.
| @@ -0,0 +1,115 @@ | |||
| @import url("https://fonts.googleapis.com/css2?family=Amiri:ital,wght@0,400;0,700;1,400;1,700&family=Libre+Baskerville:wght@700&display=swap"); | |||
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.


No description provided.