Ania#31
Conversation
authentication, pending details
Liz Fusion Changes
HectorBlisS
left a comment
HectorBlisS
left a comment
There was a problem hiding this comment.
Su app es muy consistente, me gusto mucho, lamentablemente no se nota el trabajo de más de 2 personas y es lamentable no solo por el trabajo no visible si no porque como equipo también refleja la falta de organización, el código puede ser más semántico, y definitivamente más modularizado, entendiendo modularización como la separación de responsabilidades, no la creación de muchos archivos, pueden ser 2 pero con responsabilidades separadas.
| @@ -6,13 +6,6 @@ import { renderPostView } from "./views/post.js" | |||
| import { renderProfileView } from "./views/profile.js" | |||
There was a problem hiding this comment.
Muy bueno que tengas separadas las funciones de Firebase de las funciones que tocan el DOM, sería mejor si el archivo tuviera un nombre semantico, firebase.js y dejar el nombre index.js para la mezcla de todos los archivo incluso sin poner código, meramente importaciones.
| import { showPosts } from "./views/home.js" | ||
| import { renderProfileView } from "./views/profile.js" | ||
| import { showPosts } from "./views/home.js" | ||
| import { userObserverProfile } from "./views/profile.js" |
There was a problem hiding this comment.
Tienes varias importaciones que vienen del mismo archivo, todas estas pueden quedar en 1 sola linea import { renderProfileView, userObserverProfile } from "./views/profile.js"
| @@ -24,122 +25,114 @@ const exitButton = document.querySelector('#exit'); | |||
| // listeners | |||
There was a problem hiding this comment.
Muy bien agrupando los listeners
| //Boton para mostrar iniciar sesión | ||
| const btnshow = document.querySelector('#showLogin') | ||
| btnshow.addEventListener('click', principalView) | ||
| window.onload = principalView(); |
There was a problem hiding this comment.
Cual es la razón de usar onload, el uso de este depende de la posición del archivo dentro del html, se vuelve innecesario cuando colocas los scripts en la parte inferior del html
| root.innerHTML = principal() | ||
| export function principalView() { | ||
| root.innerHTML = principal(); | ||
| let goLogin = document.querySelector('#goLogin') |
There was a problem hiding this comment.
Semantica en tus nombres, goLogin se espera que lleve a algun lado, si es una vista sería mejor tener el nombre view implicito
| }) | ||
|
|
||
| .then(function () { | ||
| console.log('Los datos se guardaron'); |
There was a problem hiding this comment.
Al final, debemos quitar los console.log
| console.log('Datos correctos, bienvenido!') | ||
| }) | ||
| .catch(function (error) { | ||
| alert('Upsi, datos incorrectos'); |
There was a problem hiding this comment.
Evitemos el uso de alerts para notificar, usemoslas para advertir.
| const btnexit = document.querySelector('#salir') | ||
| btnexit.addEventListener('click', signoutUser) | ||
|
|
||
| function signoutUser() { |
| .imagePop{ | ||
| font-size: 100px; | ||
| } | ||
| .acceptDelete{ |
There was a problem hiding this comment.
las clases css no son camelCase .accept-delete
| service firebase.storage { | ||
| match /b/{bucket}/o { | ||
| match /{allPaths=**} { | ||
| allow read, write: if request.auth!=null; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Esto definitivamente no debe ir en el repo, agregar en el gitIgnore
4 participants
No description provided.