Skip to content
This repository was archived by the owner on Jul 11, 2026. It is now read-only.

project - Social Network "estuDÍA"#18

Open
danleonca wants to merge 13 commits into
Laboratoria:masterfrom
danleonca:shari
Open

project - Social Network "estuDÍA"#18
danleonca wants to merge 13 commits into
Laboratoria:masterfrom
danleonca:shari

Conversation

@danleonca

Copy link
Copy Markdown

No description provided.

@YeraldinCasas

Copy link
Copy Markdown

Comment thread database.rules.json
@@ -0,0 +1,7 @@
{
/* Visit https://firebase.google.com/docs/database/security to learn more about security rules. */

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hay que eliminar los comentarios innecesarios

Comment thread functions/.eslintrc.json
Comment thread functions/index.js
//
// exports.helloWorld = functions.https.onRequest((request, response) => {
// response.send("Hello from Firebase!");
// });

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Lo mismo aquí, a menos que sea un comentario que nos explique un bloque de código difícil, entonces se dejan, de otro modo no nos aportan nada.

Comment thread public/index.html
<title>Welcome to Firebase Hosting</title>

<!-- update the version number as needed -->
<script defer src="/__/firebase/7.13.2/firebase-app.js"></script>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Lo mismo aquí, es algo que ya se ha comentado desde el primer proyecto.

Comment thread public/index.html
<script defer src="/__/firebase/7.13.2/firebase-messaging.js"></script>
<script defer src="/__/firebase/7.13.2/firebase-storage.js"></script>
<!-- initialize the SDK after all desired features are loaded -->
<script defer src="/__/firebase/init.js"></script>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Lo mismo aquí! al ver la ruta sabemos que se trata de firebase init, el comentario sale sobrando.

Comment thread public/index.html
body, #message { margin-top: 0; background: white; box-shadow: none; }
body { border-top: 16px solid #ffa100; }
}
</style>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Los estilos deberían ir en un archivo CSS, si se van a dejar aquí hay que cuidar la indentación del código.

Comment thread public/index.html
<a target="_blank" href="https://firebase.google.com/docs/hosting/">Open Hosting Documentation</a>
</div>
<p id="load">Firebase SDK Loading&hellip;</p>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Espacio innecesario.

Comment thread public/index.html
// firebase.storage().ref('/path/to/ref').getDownloadURL().then(() => { });
//
// // 🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Estos comentarios deben eliminarse

Comment thread src/img/gmail.svg
<g>
</g>
<g>
</g>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Las imágenes deben optimizarse para que no sean tan pesadas, pueden utilizar un optimizador en línea para svg

Comment thread src/index.html Outdated

<div id= "firstElement">
<p><img class="imgIPN" src="img/logo-ipn-blanco.png" style="width:200px; height:auto;"></p>
<h1>e s t u D Í A</h1>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Todos los valores en los atributos deberían guardar consistencias, el nombre de las imágenes también debería ser en inglés.

Comment thread src/index.html
@@ -15,20 +16,27 @@

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Espacios innecesarios

Comment thread src/index.html Outdated
<div id="createUser">
<p><input class="boxInput" id="namelNw" type="text" placeholder="Ingrese nombre de usuario"> </p> <br>
<p><input class="boxInput" id="emailNw" type="email" placeholder="Ingrese correo electrónico"> </p> <br>
<p><input class="boxInput" id="passwordNw" type="password" placeholder="Nueva contraseña"></p><br>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Los id deberían ser más descriptivos y están mal escritos, en inglés deberá ir primero el adjetivo: newPassword, newEmail, newName

Comment thread src/index.js Outdated
//example();
//import {firebase} from '@firebase/app';
//require('firebase/auth');
let allApp;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comentarios innecesarios.

Comment thread src/index.js Outdated
</div>
</div>
<main>
<!-- Text Box-->

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Borrar comentario.

Comment thread src/index.js Outdated
<div class="width-data">
<form>
<input type="text" id="text-box">
<input type="submit" value="Compartir">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Los valores deben tener consistencia:
value: share

Comment thread src/index.js Outdated
</div>
</section>
</main>
`

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Por qué tienen un template tan largo?

Comment thread src/style.css Outdated
box-sizing: border-box;
}
/*general*/
body {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comentario innecesario

Comment thread src/style.css Outdated



/*Media querys*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

También es un comentario innecesario porque abajo podemos ver que son media queries

Comment thread src/index.js Outdated
});
};
//* ***********************Print cards posts********************************
function addNewCard(printing1, doc) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comentario innecesario

Comment thread src/index.js Outdated
@@ -164,7 +164,7 @@ function addNewCardNoComents(printing2, doc) {
}
//* ***********************Print coments in real time***********************

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same here

Comment thread src/index.js
db.collection('publications')
.orderBy('Date', 'desc')
.orderBy('date', 'desc')
.onSnapshot((querySnapshot) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Muy bien aquí, que todo sea lowercase

Comment thread src/index.html Outdated
<div class="user">
<img src="img/info.svg" id="info" class="btnStyles" style="width:40px; height:auto;"> <br>
<img src="img/Group.svg" id="logOut" class="btnStyles" style="width:40px; height:auto;">
<img src="img/info.svg" id="info" style="width:40px; height:auto;"> <br>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Por qué quitaron la clase y movieron el estilo para acá?

Comment thread src/index.js Outdated
//* ****************************register users */
export const createNewUser = document.getElementById('createUserNw');
createNewUser.addEventListener('click', newUser);
export const userNew = document.getElementById('newUser');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

En general muy buena indentación y orden del código, por otro lado tienen muchos comentarios que son innecesarios.

Comment thread test/example.spec.js Outdated
// import { openModalEdit } from './utils/modalEdit.js';
// import { signOut } from './utils/exit.js';
// import { getNameProfile, theWatcher } from './index.js';
import * as newUser from '../src/utils/createUsers.js';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hay que borrar todos estos imports

Comment thread src/utils/createUsers.js
break;
default:
alert('ingreso lisiado');
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Las alertas no son le mejor recurso, deben hacerle saber de otra manera a su usuario los errores o mensajes importantes

Comment thread src/utils/loginEmail.js
break;
case 'auth/wrong-password':
alert(`Verifica tu contraseña ${errorMessage}`);
console.log(errorCode);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

los console deben eliminarse.

Comment thread src/index.js
});
});
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

muy bien aquí! me parece que está muy ordenado y cumple con lo que debe hacer.

@AdrianaHY AdrianaHY 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.

Muy bien trabajo, me parece que han tenido un gran avance. Seguimos teniendo problemas de buenas prácticas, de comentarios innecesarios, de naming, de espacios innecesarios, hay que cuidar mucho eso porque le puede restar bastante al código y al trabajo que estamos entregando.

Sigan así! Muy buen avance

Comment thread src/style.css
display: none;
}
}*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hay que eliminar código que ya no necesitamos.

Comment thread src/index.js
export let getName;
let getImg;
let url;
export let userSigned;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sólo por consistencia dejemos todos los exporto al final.

expect(result.title).toBe('Hola, encontre esto que puede ayudarnos');
done();
},
)));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Muy bien chicas con sus pruebas!!!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants