Skip to content
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

SOLUTION: JESÚS HERRERA & SOFÍA ROMERA #54

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

Sofianct
Copy link

No description provided.

@@ -0,0 +1,109 @@

const postsURL = fetch("https://jsonplaceholder.typicode.com/posts/");
const contentBody = document.getElementById('modal__body-content');

Choose a reason for hiding this comment

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

Better use camel case notation for ID, and BEM notation for classes

postsURL
.then(response => response.json())
.then(data => {
data.forEach(post => {

Choose a reason for hiding this comment

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

Better use map() method instead of forEach()

commentBox.removeChild(commentBox.lastChild);
}
}

Choose a reason for hiding this comment

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

Don't delete your comments! Use them to organize and give titles to each section of your code :)

Comment on lines +17 to +34
const mainContainer = document.getElementById('cards-container');
const cardContainer = document.createElement('div');
cardContainer.classList.add('card', 'container__card--post');
const cardBody = document.createElement('div');
cardBody.classList.add('card-body');
const cardTitle = document.createElement('h5');
cardTitle.classList.add('card-title');
const titleLink = document.createElement('a');
titleLink.classList.add('card__title--post')
titleLink.setAttribute('href','javaScript:void(0)')
titleLink.setAttribute('data-bs-toggle', 'modal')
titleLink.setAttribute('data-bs-target', '#exampleModal')
titleLink.textContent = post.title.charAt(0).toUpperCase() + post.title.slice(1);
const delIcon = document.createElement('i');
const cardText = document.createElement('p');
const addIcon = document.createElement('i');
cardText.classList.add('card-text');
cardText.textContent = (post.body.charAt(0).toUpperCase() + post.body.slice(1)).slice(0, 80) + '...';

Choose a reason for hiding this comment

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

Try to put all your const "variables" together for better organisation.

});
})
.catch(error => {
console.log(error);

Choose a reason for hiding this comment

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

Remeber to get rid of all the console.log's

Copy link

@jose-cuevas jose-cuevas left a comment

Choose a reason for hiding this comment

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

Clean code, easy to read to it, congrats!

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.

4 participants