Skip to content

SOLUTION: Ivan Escribano and Alicia Cembranos#57

Open
ivan-escribano wants to merge 50 commits intoassembler-institute:mainfrom
ivan-escribano:main
Open

SOLUTION: Ivan Escribano and Alicia Cembranos#57
ivan-escribano wants to merge 50 commits intoassembler-institute:mainfrom
ivan-escribano:main

Conversation

@ivan-escribano
Copy link
Copy Markdown

No description provided.

function getDataComments(postId) {
const containerComments = document.getElementById("commentsContainer");
containerComments.textContent = "";
const commentsURL = `http://localhost:3000/comments`;
Copy link
Copy Markdown

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

Copy link
Copy Markdown

@RogerOliveDelgado RogerOliveDelgado left a 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
Copy link
Copy Markdown

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

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

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

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

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}/`;
Copy link
Copy Markdown

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}`;
Copy link
Copy Markdown

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

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

Choose a reason for hiding this comment

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

GREAT JOB GUYS!!
25

@@ -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");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great Job!!
6

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants