Skip to content

Solution: Adrià Aparicio & Javier Fernández#55

Open
DogSoulDev wants to merge 22 commits into
assembler-institute:mainfrom
DogSoulDev:develop
Open

Solution: Adrià Aparicio & Javier Fernández#55
DogSoulDev wants to merge 22 commits into
assembler-institute:mainfrom
DogSoulDev:develop

Conversation

@DogSoulDev

Copy link
Copy Markdown

Blog with API. All info on the .pdf

Thank you.

Comment thread index.html
<!-- Modal Post -->


<div class="modal fade" id="modal__wrapp" tabindex="-1" aria-labelledby="modal__wrapp" aria-hidden="true">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tags could be more semantic in the modal sections

Comment thread assets/js/api.js
.then(response => response.json())
.then(data => data)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this{ and }catch{} could be used here

Comment thread assets/js/main.js

function displayPost() {
getPost().then(dataPost => {
for(let i = 0; i < 20; i++) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

map () is better

Comment thread assets/js/main.js
}

// crear el plantilla del post html

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

A refactor with smaller functions might help structure and readability

Comment thread assets/js/modal.js
@@ -0,0 +1,28 @@
/* Event Modal

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

perhaps it would be better to pass the postStructure function in this file

@Gaerste Gaerste left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

files should be better organized and can be refactored

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