Skip to content

Conversation

@Matiassimone
Copy link
Owner

Exercise N1, Complete!

let newElement = document.createElement('p');
let separatingElement = document.createElement('hr');
newElement.textContent = "ERROR:" + error["status"];
newElement.style.color = "#C00";
Copy link

Choose a reason for hiding this comment

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

Sería mejor si le agregaras una clase, y que desde el CSS elijas el color

Copy link
Owner Author

Choose a reason for hiding this comment

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

Check my correction 👍 da46a34

Copy link
Owner Author

Choose a reason for hiding this comment

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

All styles now, are added by classes 👍
cb43949

let jsonResponse = JSON.parse(response);
this.handleResponse(jsonResponse, within);
})
rtn.catch((error) => this.handleError(error, within))
Copy link

Choose a reason for hiding this comment

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

Un temita que tenemos que corregir:
El ejercicio dice que la función debe retornar una promise. Tu función no retorna nada.
La idea es que tu función pueda ser usada de forma

myRequest({method: 'GET', url: 'url'})
.then()
.catch()

Copy link
Owner Author

Choose a reason for hiding this comment

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

Perfect, i'll change this function to return a promise. And the promise returned it will be used in the function (handleResponseRepositories).

Perfecto, cambiaré esta función para devolver una promesa. Y la promesa devuelta se usará en la función (handleResponseRepositories).

Copy link
Owner Author

Choose a reason for hiding this comment

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

let id = document.getElementById("repositoriesList");
id.appendChild(li);

let separatingElement = document.createElement('hr');
Copy link

Choose a reason for hiding this comment

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

Esto no es necesario arreglarlo, pero para el futuro. No uses hrs. Si querés que haya espacio entre un elemento y otro, usá márgenes, y siempre desde CSS

Copy link
Owner Author

Choose a reason for hiding this comment

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

Please check my correction 👍
25752df

}

function CloseModal() {
document.getElementById('openModal').style.display = 'none';
Copy link

Choose a reason for hiding this comment

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

Tanto el color que le asignás a los elemetos, como esta regla display que agregás, deberían en realidad ser clases, y todos los estilos en un archivo de CSS

Copy link
Owner Author

Choose a reason for hiding this comment

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

All styles now, are added by classes, please cheack my correction 👍
cb43949

if (document.getElementById("repositoriesList") == undefined) {
alert("Don't exists list of repositories to search.");

} else if (toSeek == "" || toSeek === " ") {
Copy link

Choose a reason for hiding this comment

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

Fijate que a veces usás == y a ves ===. Sabés la diferencia? Deberías usar siempre ===

Copy link
Owner Author

Choose a reason for hiding this comment

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

La diferencia es "==" compara los datos, y "===" compara los datos y el tipo de dato.
Por lo que "===" es mas estricto y en cambio el "==", operaciones como (5=="5") dan como resultado TRUE.

En resumen sabia la diferencia pero sin querer me saltee un "=" de menos ahí, ya lo corrijo !..

Copy link
Owner Author

Choose a reason for hiding this comment

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

tr.appendChild(td);
}
}
table.style.border = "1px solid black";
Copy link

Choose a reason for hiding this comment

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

Esto mandalo a un archivo CSS

Copy link
Owner Author

Choose a reason for hiding this comment

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

Please check my correction 👍
ffee632

document.getElementById('openModal').style.display = 'none';
}

function matrixDraw(matrix, within) {
Copy link

Choose a reason for hiding this comment

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

Para qué están los parámetros? matrix está siendo pisada y within no se está usando

Copy link
Owner Author

Choose a reason for hiding this comment

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

Please check my correction 👍
57b50c0

Copy link

@intii intii left a comment

Choose a reason for hiding this comment

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

Me interesa que arreglemos dos cosas:
1- La función que hace un request, para que retorna la promesa
2- Sacar todas las referencias a estilos de los archivos de JavaScript

…ted, now to add separation uses padding in style.css)
…larized in style.css and the function (handleSeek) now used the param colorText to change the color of your results)
…, return a promise, and the function handle use this promise)
…s been created to draw a matrix example and use the function matrixDraw)
Copy link
Owner Author

@Matiassimone Matiassimone left a comment

Choose a reason for hiding this comment

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

All troubleshooting are fixed now !

let newElement = document.createElement('p');
let separatingElement = document.createElement('hr');
newElement.textContent = "ERROR:" + error["status"];
newElement.style.color = "#C00";
Copy link
Owner Author

Choose a reason for hiding this comment

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

All styles now, are added by classes 👍
cb43949

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants