Send for code review, team5!!#27
Conversation
Co-authored-by: Carmen <49756083+CarmenXP@users.noreply.github.com>
HectorBlisS
left a comment
HectorBlisS
left a comment
There was a problem hiding this comment.
Se nota el trabajo duro en su proyecto, es importante agregar profesionalismo con las variables totalmente en inglés, bien la separación de responsabilidades, pero puede hacerse aún mejor, BD | DOM | FILES, bien por el uso de promesas y los estilos muy bien.
| import { registros } from './pantalla2.js' | ||
|
|
||
| let root = document.querySelector('#root'); | ||
| export const login = () => { |
There was a problem hiding this comment.
Muy bien el uso de arrow functions
| var credential = error.credential; | ||
| // ... | ||
| }); | ||
| } |
There was a problem hiding this comment.
Bien la integración de los listener dentro de la función para render
| import { registros } from './pantalla2.js' | ||
|
|
||
| let root = document.querySelector('#root'); | ||
| export const login = () => { |
There was a problem hiding this comment.
El nombre de las funciones debe ser camelCase mucho más descriptivas, esto parece un nombre de variable.
|
|
||
| let bttn = document.querySelector('.logoG'); | ||
| bttn.addEventListener('click', login); | ||
| function login (){ |
There was a problem hiding this comment.
Nombres de funciones en camelCase
| } | ||
|
|
||
|
|
||
| let bttn0 = document.querySelector('.logoF'); |
There was a problem hiding this comment.
button0 es poco descriptivo, usen nombres más obvios cómo loginButton o homeButtonAction
| import { pantalla1 } from './compenentes/pantalla1.js'; | ||
| import { registrar } from './compenentes/registro.js'; | ||
| login(); | ||
| pantalla1(); |
There was a problem hiding this comment.
PÉSIMO EL USO DE ESPAÑOL DENTRO DEL CÓDIGO
| } | ||
|
|
||
| /*Estilos pantalla posts*/ | ||
| .containtPub{ |
There was a problem hiding this comment.
EVITEMOS el camel case en clases de css, prefiramos contain-pub e intentemos ser más descriptivas
| <button id="delete" value="${doc.id}" class="delete">Eliminar</button> | ||
| <button id="edit" name="${doc.data().texto}" data-id="${doc.id}" class="edit">Editar</button> | ||
| <button class="like" name="${doc.data().likes}" data-id="${doc.id}"> Me gusta </button> <p class="like" >${doc.data().likes}</p> | ||
| <button class="like" name="${doc.data().likes}" data-id="${doc.id}"> <p class="like" >${doc.data().likes}</p> Me gusta</button> |
There was a problem hiding this comment.
Bien por la separación de las funciones para guardar datos en la BD pero siempre separar el DOM de la lógica.
| }else{ | ||
| fileInput.onchange = e => { | ||
| let file = e.target.files[0] | ||
| firebase.storage().ref("images").child(file.name).put(file) |
There was a problem hiding this comment.
Igual, preferible separar el storage del firestore, para poder mantenerlo cundo el proyecto cresca
| @@ -430,10 +430,12 @@ body{ | |||
| font-size: 20px; | |||
| } | |||
| p.like{ | |||
There was a problem hiding this comment.
Bien por el uso de especificidad, siempre evitando llegar a mas de 3 niveles.
4 participants
This is the good