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

Solicitando primer pull request#27

Open
EdithOrt wants to merge 48 commits into
Laboratoria:masterfrom
EdithOrt:master
Open

Solicitando primer pull request#27
EdithOrt wants to merge 48 commits into
Laboratoria:masterfrom
EdithOrt:master

Conversation

@EdithOrt

Copy link
Copy Markdown

No description provided.

@fabi-pharoxysme fabi-pharoxysme deleted the master branch March 19, 2020 15:04

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

Los commits podrían ser un poco más descriptivos, el uso de nombres, variables, id, clases etc en español ya no será tolerable en el siguiente proyecto, muy bien el trabajo en equipo y progresivo, se nota que respaldaron seguido, muy bien.

Comment thread src/index.html Outdated
Comment on lines +20 to +34
<option id="grass">grass</option>
<option id= "poison">poison</option>
<option id= "fire">fire</option>
<option id = "fairy">fairy</option>
<option id= "water">water</option>
<option id = "bug">bug</option>
<option id = "normal">normal</option>
<option id = "electric">electric</option>
<option id = "ground">ground</option>
<option id = "fighting">fighting</option>
<option id = "psychic">psychic</option>
<option id = "rock">rock</option>
<option id = "ice">ice</option>
<option id = "ghost">ghost</option>
<option id = "dragon">dragon</option>

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 pudo crearse dinámicamente con la misma data de los pokemons.

Comment thread src/main.js Outdated
label.className+=("a" +item.id)
label.id= ("a"+item.type)
let btn= document.createElement('button');
btn.className+= ("btn-Images")

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.

className no existe en native, eso es de ReactJS, la forma de trabajar con clases en Vanilla es:

btn.classList.add("btn-Images")

Comment thread src/main.js Outdated
label.className+=("a" +item.id)
label.id= ("a"+item.type)
let btn= document.createElement('button');
btn.className+= ("btn-Images")

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.

Eviten usar clases con mayusculas o camelCase, btn-images

Comment thread src/main.js Outdated

let btnImg = document.querySelectorAll('.btn-Images');
let Imgs = btnImg.id
let Imgs = document.querySelector('#a')

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.

Eviten las mayusculas en las variables, a menos que se entienda qué es una clase (JS) y porqué se usan Mayusculas en una declaración de clase, bien por el uso de querySelector

Comment thread src/main.js Outdated
Comment on lines +54 to +57
function onChange(e){
let value = e.target.value
let nuevaListaFiltrada = dataPokemon.filter(p=>p.type.includes(value))
console.log(nuevaListaFiltrada)

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.

Evitar español, y cuidado con la identación de las funciones, lo vuelve difícil de leer

Comment thread src/index.html Outdated
Comment on lines +19 to +22
<a href="#">FUEGO</a>
<a href="#">FUEGO</a>
<a href="#">FUEGO</a>
<a href="#">FUEGO</a>

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.

mismo link?

Comment thread src/data.js Outdated
Comment on lines 5 to 8
export const dataPokemon= () => {
//data.pokemon.filter((item) => item.id === Number);
return data.pokemon;
};

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.

Función inecesaria no crees?

Comment thread src/main.js Outdated
let options= document.querySelector(".dropdown-content");
options.addEventListener('click',onChange)
function onChange(e){
let value = e.target.value

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 del evento.

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