Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add task react-clase-2 #20

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

fedeesti
Copy link

No description provided.

Comment on lines 19 to 35
function useInterval(callback, delay) {
const savedCallback = useRef();

useEffect(() => {
savedCallback.current = callback;
}, [callback]);

useEffect(() => {
function tick() {
savedCallback.current();
}

if (delay !== null) {
let id = setInterval(tick, delay);
return () => clearInterval(id);
}
}, [delay]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

no declares un hook por dentro de otro, declaralo por fuera del useTimer

Copy link
Author

Choose a reason for hiding this comment

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

Este código lo copié y pegué del blog que me mandaste, lo cambio igual? Simplemente lo adapté al memotest

Copy link
Collaborator

Choose a reason for hiding this comment

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

sí, pero en general no tenés que declarar hooks adentro de otros hooks, solamente movelo afuera del otro hook

Comment on lines 4 to 8
const createInitialCards = () => {
let allCards = cardArray.concat(cardArray);
let newCards = allCards.sort(() => 0.5 - Math.random());
return newCards;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

si ya estás guardando las cartas en un state, podés pasar el state de isFlipped acá mismo (así no lo tenés que armar a mano con un tamaño predeterminado). también podés agregar las keys que habíamos dicho para el render (vegeta-0, vegeta-1, etc.)

podés hacer todo eso con un .map() antes de devolverlo

Copy link
Author

Choose a reason for hiding this comment

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

Si las keys puse nombre[i] porque no se me ocurrió deducir el vegeta-0 y vegeta-1. IsFlipped lo paso como array o pensaba asignarlo cómo objeto { isFlipped: false }

Copy link
Collaborator

Choose a reason for hiding this comment

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

claro, asignarlo al objeto es lo que decía

const [previousID, setPreviousID] = useState(null);
const [animating, setAnimating] = useState(false);
const [wonPairs, setWonPairs] = useState(0);
const gameEnded = wonPairs > 7;
Copy link
Collaborator

Choose a reason for hiding this comment

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

esto debería ser wonPairs > (cards.length / 2), así podés agregar cartas nuevas sin tener que modificar nada acá

const [cards, setCards] = useState(createInitialCards);
const [isFlipped, setIsFlipped] = useState([false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false]);
const [currentPairs, setCurrentPairs] = useState([]);
const [previousID, setPreviousID] = useState(null);
Copy link
Collaborator

Choose a reason for hiding this comment

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

El previousID me hace ruido, creo que es innecesario. Entiendo que lo necesitás para dar vuelta las dos cartas después, pero siento que eso mismo lo deberías poder hacer con currentPairs si lo modificás para que en vez de tener solo las strings alt, tengan la id también, onda:

setCurrentPairs([...currentPairs, { alt, id }]) 

Copy link
Author

Choose a reason for hiding this comment

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

El previousID es más para el CSS así aplico las transformaciones al dar vuelta la carta. Ahora corrijo como decís para que me quede más prolijo.


const WinnerScore = ({ show, result, restart = () => {} }) => {
return (
<div className={'winner-score ' + (show ? 'winner-score--hidden': '')}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

creo que esto hace lo opuesto de lo que querés: si show es true, pone la clase winner-score--hidden.
Fijate que cuando ganás no podés apretar el botón porque tiene el pointer-events: none; puesto

Copy link
Author

Choose a reason for hiding this comment

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

Lo del botón no lo pude hacer funcionar 😂 Ahora veo el problema, ojalá ahora si funcione

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.

2 participants