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

Para revision#29

Open
Oralia wants to merge 15 commits into
Laboratoria:masterfrom
Oralia:master
Open

Para revision#29
Oralia wants to merge 15 commits into
Laboratoria:masterfrom
Oralia:master

Conversation

@Oralia

@Oralia Oralia commented May 4, 2020

Copy link
Copy Markdown

No description provided.

@HectorBlisS HectorBlisS left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No tengo muchas notas negativas, en realidad su código es muy consistente, se nota la falta de commits más seguido, pero bien, la integración y el trabajo en equipo, las descripciones de los commits puede mejorar, pero excelente trabajo dividiendo las responsabilidades de cada archivo.

Comment thread src/index.html Outdated
Comment on lines 62 to 67
<div id="LogInNewUser" class="creatAccount">
<p>No tienes Cuenta?</p>
<button id="creatAccount" class="btn btn-link"> Crear Cuenta</button>
</div>
</section>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Es mejor que todo se adinámico, evitar el maquetado estático lo más posible

Comment thread src/index.js Outdated
Comment on lines +76 to +79
document.getElementById('logInUser').style.display="none";
document.getElementById('init').style.display="none";
document.getElementById('logInNetwoork').style.display="none";
document.getElementById('LogInNewUser').style.display="none";
document.getElementById('LogInNewUser').style.display="none"; // let file = e.target.files[0]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Es mejor usar nodos, (guardamos estos selectores en variables) para no repetir la selección

Comment thread src/index.js Outdated
toggleButton.onclick= e=> menu()

let homeButton = document.querySelector('#home');
homeButton.onclick = e=> home()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Esta sintaxis es muy limpia y eficiente, muy bien!

Comment thread src/index.js Outdated
@@ -188,11 +192,6 @@ function createAccount() {

function menu (){

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Intentemos ser mas semanticas con los nombres de las funciones, este nombre menu es mas usada para una variable, y aún así le faltaría semántica, para una función es importante describir lo que hace. drawMenu o toggleMenu

Comment thread src/styles.css Outdated
Comment on lines +1 to +5
/*body{
background: linear-gradient(180deg, rgb(39, 250, 127), rgb(208, 253, 228), rgb(9, 209, 162));
mix-blend-mode: soft-light;
box-shadow: 0px 4px 4px rgba(0, 0, 0, 0.25), 0px 4px 4px rgba(0, 0, 0, 0.25);
}
}*/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Eliminar comentarios que no usamos

Comment thread src/index.js Outdated
const btnToggle = document.querySelector('.toggle-btn');

btnToggle.addEventListener('click', function () {
document.getElementById('sidebar').classList.toggle('active');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Excelente el uso de toggle

Comment thread src/test.spec.js
Comment on lines +24 to +48
/* import { example } from '../src/index.js';

describe('logInFacebook', () => {
it('debería ser una función', () => {
expect(typeof logInFacebook).toBe('function');
});
it('Debería poder iniciar sesion con Facebook', () => {
return logInFacebook()
.then(() => {
expect('').toBe('');
});
});
});

describe('logInFacebook', () => {
it('debería ser una función', () => {
expect(typeof logInFacebook).toBe('function');
});
it('Debería poder registrarse con Facebook', () => {
return logInFacebook()
.then((user) => {
expect('').toBe('');
});
});
}); */ No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Eliminar comentarios que no usamos

Comment thread src/firebase.js Outdated
Comment on lines 98 to 104
let loginView = `
<section>
<p id="post" > Aqui se debe publicar</p>
<textarea name="" id="" cols="40" rows="10" placeholder="Crear publicación"></textarea>
<input type="submit" class="btnPost" id="" value="Publicar">
<button id="like"><i class="fab fa-gratipay"></i></button>
</section>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Muy bien el uso semantico de section

Comment thread src/firebase.js Outdated
});

//Cerrando sesión
function close(){

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

El nombre de las funciones debe ser más descriptiva, regularmente con 2 palabras en camel case como closeSession

Comment thread src/index.js Outdated
@@ -77,3 +77,4 @@ function logIn(){
}

logInButton.onclick = logIn;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bien el uso de listener más directos

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