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

Pine-Marjan, Megan, Roslyn #56

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

Pine-Marjan, Megan, Roslyn #56

wants to merge 4 commits into from

Conversation

roslynm
Copy link

@roslynm roslynm commented Dec 21, 2021

No description provided.

Copy link

@jbieniosek jbieniosek left a comment

Choose a reason for hiding this comment

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

Great work on this project Marjan, Megan and Roslyn! This project is green.

// Wave 2
// You will need to create a method to change the square
// When it is clicked on.
// Then pass it into the squares as a callback
const updateSquares = (id) => {

Choose a reason for hiding this comment

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

There is a small logical bug here. There is no check here to see if there is a winner, so it is possible to continue to play. This can result in unexpected behavior. For example, if the board looks like this:

| | | |
| |o|o|
|x|x|x|

the game will display 'Winner is x'. However, 'o' can still move, so if 'o' clicks in the middle left square:

| | | |
|o|o|o|
|x|x|x|

the game will display "Winner is ". This continues until the entire board is filled, at which point the game will display "Winner is Tie".

Comment on lines +99 to +103
for (let combo of winning){
if (isSuperset(xSet,combo)){
myX.push(combo);
}
}

Choose a reason for hiding this comment

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

Interesting approach!

Comment on lines +130 to +137
const isSuperset = (set, subset) => {
for (let elem of subset) {
if (!set.has(elem)) {
return false;
}
}
return true;
};

Choose a reason for hiding this comment

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

👍

// }else if (square.value==='o'){
// classVal='chris';
// }
oneD.push(<Square key={square.id} id={square.id} value={square.value} onClickCallback={onClickCallback} />);

Choose a reason for hiding this comment

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

Nice!

Comment on lines +14 to +22
background-image: url("https://ca.slack-edge.com/T024N8K3W8N-U029WFRD8HW-807f2572ed86-512");
Background-size: 200px 200px;
}

.chris {
background-image: url("https://ca.slack-edge.com/T024N8K3W8N-U0283SDM62H-f8f25fdaa3da-512");
Background-size: 200px 200px;
}

Choose a reason for hiding this comment

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

😂

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