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

Fabiola#25

Open
DorsD wants to merge 41 commits into
Laboratoria:masterfrom
DorsD:fabiola
Open

Fabiola#25
DorsD wants to merge 41 commits into
Laboratoria:masterfrom
DorsD:fabiola

Conversation

@DorsD

@DorsD DorsD commented Mar 13, 2020

Copy link
Copy Markdown

Project name: Bars & Dots
Developers: Eliza Olmedo y Fabiola González

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

Muy bien el repo, de los mejores, eviten el español a toda costa, eviten camelCase en CSS y sean más descriptivas con los commits, también en ingles. Felicidades por el gran trabajo, se nota el trabajo progresivo y en equipo.

Comment thread src/indicators.html Outdated
<script type="module" src="indicators.js"></script>
</body>
</html>

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 por borrar comentarios y código sobrante

Comment thread src/indicators.js
});
}

const removeGraphic = () => {

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 los nombres en ingles y el camelCase

Comment thread src/indicators.js
}

const showGraphic = (indicator) => {
let toggleButtonFlag = false;

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 variable nunca se usó

Comment thread src/indicators.js
}

// When the user clicks (x) of the modal, close it
span.onclick = function() {

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.

De preferencia usar addEventListener

Comment thread src/styleIndicators.css
width: 60%;

box-shadow: 0 4px 8px 0 rgba(0,0,0,0.2),0 6px 20px 0 rgba(0,0,0,0.19);
box-shadow: 0 8px 16px 0 rgba(0,0,0,0.4),0 10px 22px 0 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.

bien

Comment thread test/data.spec.js
Comment on lines 3 to +10

describe('example', () => {
describe('bankData', () => {

it('debería ser una función', () => {
expect(typeof example).toBe('function');
expect(typeof bankData).toBe('function');
});

describe('example', () => {
//describe('example', () => {

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 por agregr tests

Comment thread src/indicators.js
@@ -56,18 +49,14 @@ for (let i=0; i< indicatorName.length; i ++){
aTwitter.setAttribute("href","https://twitter.com/share?url=<URL>&text=<TEXT>via=<USERNAME>");
aFacebook.setAttribute("href","https://www.facebook.com/sharer/sharer.php?u=<URL>");
aMail.setAttribute("href","mailto:?subject=<SUBJECT>&body=<BODY>");

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.

podrian intentar usar más `` para dibujar el html y mejorar la lectura

Comment thread src/indicators.js
let graphicModal = document.getElementById("graphicModal");

let graphic = document.createElement("canvas");
graphic.setAttribute("id","myChart");

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

Comment thread src/indicators.js
data:{
labels:Object.keys(indicatorData),
datasets:[{
label:'PER',

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 graficas

Comment thread src/indicators.js
}

// When the user clicks anywhere outside of the modal, close it
window.onclick = function(event) {

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.

Esto no es necesario si el script está al final de la etiueta body

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.

3 participants