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

Added voting feature and strikethrough on invalid answers, next question button #7

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

Conversation

SpencerHC
Copy link
Collaborator

@SpencerHC SpencerHC commented Sep 12, 2024

Strike through only on a users own repeat answers, not when opponent has same answer

@@ -24,11 +24,17 @@ const NUM_CATEGORIES = CATEGORIES.length;

export type Phase = "write" | "review";

export type answer = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export type answer = {
export type Answer = {

@@ -47,6 +53,11 @@ type WritePayload = {
answer: string;
};

type VotePayload = {
userId: string;
answer: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
answer: string;
categoryIndex: number;
points: number;

altough it's fine to keep the points for a later PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is categoryIndex the index of the answer in the category?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Whoops disregard, other comment referring to this was hidden for some reason

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right to be confused I think we also do need the userId in the payload, but it's the user id of the "voteD" player, not the voter! This is confusing 😅

@@ -69,14 +80,58 @@ const write: PlayerMove<GS, WritePayload> = {
}

board.answers = {};

board.playerVotes = {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's swap the if at line 76:

// We are done with this phase when any player has entered 10 answers.
const weAreDone = answerRound.every((a) => a.length > 0)

if (!weAreDone) {
  return;
}

// When we are done we go to the Write phase.
board.phase = 'review' as const;

board.answer = {}
board.playerVotes = {}
...

};

const vote: PlayerMove<GS, VotePayload> = {
executeNow({ board, playerboard, payload }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
executeNow({ board, playerboard, payload }) {
executeNow({ board, playerboard, userId, payload }) {

@@ -47,6 +53,11 @@ type WritePayload = {
answer: string;
};

type VotePayload = {
userId: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to pass the userId as a parameter: we have access to the userId that is making the move, always.

Suggested change
userId: string;

Copy link
Contributor

Choose a reason for hiding this comment

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

Scratch that we need a userId, the userId of the player who wrote the answer we are voting against.

@@ -104,6 +108,7 @@ function ReviewContent() {
<ReviewPlayer key={userId} userId={userId} />
))}
</div>
<button onClick={() => makeMove("nextStep")}>Next Question</button>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<button onClick={() => makeMove("nextStep")}>Next Question</button>
<button onClick={() => makeMove("nextStep")}>Next Category</button>

Comment on lines +5 to +6
const useMakeMove = makeUseMakeMove<G>();
const useSelector = makeUseSelector<GS>();
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably have a separate file, maybe hooks.ts where we put those two lines. All makeUse* functions are doing is adding the typing information to the use* functions (so that type checking is done when we call them).

So this would become a simple import:

import { useMakeMove, useSelector } from './hooks'

(state) => state.board.playerVotes[answer][userId],
);
const makeMove = useMakeMove();
console.log(flagged);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
console.log(flagged);

console.log(flagged);
return (
<button
onClick={() => makeMove("vote", { userId: userId, answer: answer })}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
onClick={() => makeMove("vote", { userId: userId, answer: answer })}
onClick={() => makeMove("vote", { answer })}

import { UserId } from "@lefun/core";
import { makeUseSelector, makeUseMakeMove } from "@lefun/ui";
import { G, GS } from "categorio-game";
export function Flag({ userId, answer }: { userId: UserId; answer: string }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export function Flag({ userId, answer }: { userId: UserId; answer: string }) {
export function Flag({ categoryIndex }: { categoryIndex: number }) {

(answer) => answer.toLowerCase() === a.toLowerCase(),
).length > 1
) {
const valid = checkedAnswers.includes(a) ? false : true;
Copy link
Contributor

Choose a reason for hiding this comment

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

const valid = !checkAnswers.includes(a)

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