Solution by Volha Afanasenka & Juan Carlos Cabello#48
Solution by Volha Afanasenka & Juan Carlos Cabello#48JcarlosCabello1991 wants to merge 2 commits intoassembler-institute:mainfrom
Conversation
| return (a.id.toString() - b.id.toString()); | ||
| }) | ||
| data.forEach(post =>{ | ||
| const postElement = document.createElement("section"); |
There was a problem hiding this comment.
Perhaps having a template would be better then creating each element
| @@ -0,0 +1,100 @@ | |||
| import {modifyingPost} from "./modifyPost.js"; | |||
| import { deletingPost } from "./deletePost.js"; | |||
There was a problem hiding this comment.
Only one function per javaScript? Is this optimal? Or could you have gathered more functions under one js, i.e postFunctions.js userFunctions.js commentFunctions.js . Calling many files may not be a great thing.
| @@ -0,0 +1,19 @@ | |||
| import { getPosts } from "./main.js"; | |||
| export function modifyingPost(id, user){ | |||
| const modalModify = document.getElementById("staticBackdrop-form"); | |||
There was a problem hiding this comment.
const modalModify = document.getElementById("staticBackdrop-form");
const modalModifyButton = document.getElementById("modifyPostButton");
If variables are not used they should be removed, so that your code is easier to read
| }) | ||
| postsList.append(postElement); | ||
| }) | ||
| }else{//if the user deleted some posts or modify some post, we delete all li and create all of them from the beggining |
There was a problem hiding this comment.
it should be the other way, when you delete something you refresh the posts
| const postTitle = post.title; | ||
| postElement.append(postTitle); | ||
| postElement.setAttribute("id", post.id); | ||
| postElement.setAttribute("userId", post.userId); |
There was a problem hiding this comment.
Suggestion: in order to use less code when setting atribute, perhaps you could create a function that assign values to keys and reuse it when needed.
Example:
const assignAttributes = (element, attributes) => { for (const key in attributes) { element.setAttribute(key, attributes[key]) } }
| .then(response=>response.json()) | ||
| .then(data=>{ | ||
| const postsList = document.getElementById("listGroup"); | ||
| Array.from(document.getElementById("listGroup").children).forEach(son=>{ |
There was a problem hiding this comment.
We assume you guys creating an array because you want to iterate over a html collection?
it can be done with a for loop, or for in loop.
|
Guys, thanks for the review, your comments are very useful. |
4 participants
Solution by Volha Afanasenka & Juan Carlos Cabello