Handle matching pairs #7
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mattgrunwald
reviewed
Nov 5, 2024
| @@ -2,36 +2,25 @@ import { useEffect, useMemo, useState } from 'react'; | |||
| import { SignContent } from '../../types'; | |||
| import './PicketSign.css'; | |||
|
|
|||
| export const PicketSign = ({ content }: { content: SignContent }) => { | |||
| const [flipped, setFlipped] = useState(true); | |||
| const [showContent, setShowContent] = useState(false); | |||
There was a problem hiding this comment.
This exists to prevent the card contents from flashing when the page loads. However I think you're on to something. I'll make a suggestion below that can prevent the image flash without extra JS
There was a problem hiding this comment.
The suggestions feature in github won't let me suggest the entire code change, so I made #8 instead.
mattgrunwald
reviewed
Nov 5, 2024
| @@ -2,36 +2,25 @@ import { useEffect, useMemo, useState } from 'react'; | |||
| import { SignContent } from '../../types'; | |||
| import './PicketSign.css'; | |||
|
|
|||
| export const PicketSign = ({ content }: { content: SignContent }) => { | |||
| const [flipped, setFlipped] = useState(true); | |||
| const [showContent, setShowContent] = useState(false); | |||
There was a problem hiding this comment.
The suggestions feature in github won't let me suggest the entire code change, so I made #8 instead.
mattgrunwald
reviewed
Nov 5, 2024
src/App.tsx
Outdated
Comment on lines
17
to
57
|
|
||
| const handleCardClick = (card: Card) => { | ||
| // do nothing if card is already face up or matched: | ||
| if (card.isFaceUp || card.isMatched) { | ||
| return; | ||
| } | ||
|
|
||
| // do nothing if there's already two actively face-up cards: | ||
| const faceUpCards = cards.filter(c => c.isFaceUp && !c.isMatched); // TODO: do we care about O(N) here? probably not | ||
| if (faceUpCards.length >= 2) { | ||
| return; | ||
| } | ||
|
|
||
| // flip the card face-up: | ||
| const newCards = cards.map(c => | ||
| c.id === card.id ? { ...c, isFaceUp: true } : c | ||
| ); | ||
| setCards(newCards); | ||
|
|
||
| // is this the second flip, and if so is it a match for the first? | ||
| const newFaceUpCards = newCards.filter(c => c.isFaceUp && !c.isMatched); | ||
| if (newFaceUpCards.length === 2) { | ||
| const [firstCard, secondCard] = newFaceUpCards; | ||
| if (firstCard.value === secondCard.value) { | ||
| // cards match, mark them as matched and keep them face up: | ||
| setCards(newCards.map(c => | ||
| c.value === firstCard.value ? { ...c, isMatched: true } : c | ||
| )); | ||
| } else { | ||
| // cards don't match, flip them back face-down after a delay: | ||
| setTimeout(() => { | ||
| setCards(cards => cards.map(c => | ||
| c.id === firstCard.id || c.id === secondCard.id | ||
| ? { ...c, isFaceUp: false } | ||
| : c | ||
| )); | ||
| }, 1000); | ||
| } | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
Suggested change
| const handleCardClick = (card: Card) => { | |
| // do nothing if card is already face up or matched: | |
| if (card.isFaceUp || card.isMatched) { | |
| return; | |
| } | |
| // do nothing if there's already two actively face-up cards: | |
| const faceUpCards = cards.filter(c => c.isFaceUp && !c.isMatched); // TODO: do we care about O(N) here? probably not | |
| if (faceUpCards.length >= 2) { | |
| return; | |
| } | |
| // flip the card face-up: | |
| const newCards = cards.map(c => | |
| c.id === card.id ? { ...c, isFaceUp: true } : c | |
| ); | |
| setCards(newCards); | |
| // is this the second flip, and if so is it a match for the first? | |
| const newFaceUpCards = newCards.filter(c => c.isFaceUp && !c.isMatched); | |
| if (newFaceUpCards.length === 2) { | |
| const [firstCard, secondCard] = newFaceUpCards; | |
| if (firstCard.value === secondCard.value) { | |
| // cards match, mark them as matched and keep them face up: | |
| setCards(newCards.map(c => | |
| c.value === firstCard.value ? { ...c, isMatched: true } : c | |
| )); | |
| } else { | |
| // cards don't match, flip them back face-down after a delay: | |
| setTimeout(() => { | |
| setCards(cards => cards.map(c => | |
| c.id === firstCard.id || c.id === secondCard.id | |
| ? { ...c, isFaceUp: false } | |
| : c | |
| )); | |
| }, 1000); | |
| } | |
| } | |
| }; | |
| const faceUpCards = useMemo( | |
| () => cards.filter((c) => c.isFaceUp && !c.isMatched), | |
| [cards] | |
| ); | |
| useEffect(() => { | |
| if (faceUpCards.length !== 2) { | |
| return; | |
| } | |
| const [firstCard, secondCard] = faceUpCards; | |
| if (firstCard.value === secondCard.value) { | |
| // cards match, mark them as matched and keep them face up: | |
| setCards((cards) => | |
| cards.map((c) => | |
| c.value === firstCard.value ? { ...c, isMatched: true } : c | |
| ) | |
| ); | |
| } else { | |
| // cards don't match, flip them back face-down after a delay: | |
| setTimeout(() => { | |
| setCards((cards) => | |
| cards.map((c) => | |
| c.id === firstCard.id || c.id === secondCard.id | |
| ? { ...c, isFaceUp: false } | |
| : c | |
| ) | |
| ); | |
| }, 1000); | |
| } | |
| }, [cards, faceUpCards]); | |
| const handleCardClick = (card: Card) => { | |
| // do nothing if card is already face up or matched | |
| // or if there's already two actively face-up cards: | |
| if (card.isFaceUp || card.isMatched || faceUpCards.length >= 2) { | |
| return; | |
| } | |
| // flip the card face-up: | |
| const newCards = cards.map((c) => | |
| c.id === card.id ? { ...c, isFaceUp: true } : c | |
| ); | |
| setCards(newCards); | |
| }; |
There was a problem hiding this comment.
This suggestion isn't strictly necessary but does move state management out of the click handler and slightly deduplicates code
Remove content flash on page load
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When two cards are flipped, checks if they’re a match, and if so, marks them as such and ignores them for the rest of the game; otherwise, flips them back over after a delay.
Also:
handleCardClickcould probably use a close look from someone who understands state management in React better than me.