Added playble tic tac toe game in Board game section #401
Conversation
|
@Sumanbhadra is attempting to deploy a commit to the bunty's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes in this pull request involve updates to the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thank you for submitting your pull request! 🙌 We'll review it as soon as possible. In the meantime, please ensure that your changes align with our CONTRIBUTING.md. If there are any specific instructions or feedback regarding your PR, we'll provide them here. Thanks again for your contribution! 😊 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- README.md (3 hunks)
- frontend/src/components/Pages/Boardgame.jsx (2 hunks)
- frontend/src/components/Pages/Games/TicTacToe.jsx (1 hunks)
- frontend/src/router/index.jsx (2 hunks)
🧰 Additional context used
🪛 Markdownlint
README.md
283-283: Column: 1
Hard tabs(MD010, no-hard-tabs)
284-284: Column: 1
Hard tabs(MD010, no-hard-tabs)
🔇 Additional comments (3)
frontend/src/router/index.jsx (1)
25-25: LGTM!The import statement follows the project's conventions and maintains proper component organization under the Games directory.
frontend/src/components/Pages/Boardgame.jsx (1)
25-25: LGTM! Import statement is correctly placed.The addition of the Link import from react-router-dom is appropriate for implementing navigation to the new TicTacToe game.
README.md (1)
277-280: LGTM: Contributor information updates are consistent.The changes to the contributor information match the AI summary:
- Added Ashwini_ab
- Updated alolika bhowmik's information
- Updated Vinay Anand Lodhi to Nilanchal
The changes maintain the table structure and include all necessary information (avatar, GitHub link, and name).
Also applies to: 286-289, 314-317, 330-333
frontend/src/router/index.jsx
Outdated
| @@ -41,6 +42,7 @@ const router = createBrowserRouter( | |||
| <Route path="/email-verify" element={<EmailVerify />} /> | |||
| <Route path="/membership" element={<Membership />} /> | |||
| <Route path="/help" element={<HelpAndSupport />} /> | |||
| <Route path="/TicTacToe" element={<TicTacToe />} /> | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider route naming consistency and organization.
The current implementation has a few areas for improvement:
- The route path uses PascalCase ("/TicTacToe") while other routes use kebab-case or lowercase. Consider using "/tic-tac-toe" for consistency.
- Since this is part of the Board game section, consider either:
- Grouping it near the "/boardgame" route for better code organization
- Using nested routing under "/boardgame" for better URL hierarchy
Consider this refactoring approach:
<Route path="/boardgame" element={<Boardgame />} />
+ <Route path="/boardgame/tic-tac-toe" element={<TicTacToe />} />
<Route path="/events" element={<Event />} />
...
- <Route path="/TicTacToe" element={<TicTacToe />} />Note: If you implement this change, remember to update the corresponding Link component in Boardgame.jsx to use the new path.
Committable suggestion was skipped due to low confidence.
| import React, { useState } from 'react'; | ||
|
|
||
| function TicTacToe() { | ||
| const [board, setBoard] = useState(Array(9).fill(null)); | ||
| const [isXNext, setIsXNext] = useState(true); | ||
| const winner = calculateWinner(board); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider enhancing type safety and accessibility
While the basic setup is correct, consider these improvements:
- Add PropTypes or TypeScript for better type safety
- Consider extracting game logic into a custom hook for better separation of concerns
Example implementation of a custom hook:
function useGameState() {
const [board, setBoard] = useState(Array(9).fill(null));
const [isXNext, setIsXNext] = useState(true);
const winner = calculateWinner(board);
// ... game logic here
return { board, isXNext, winner, handleClick, resetGame };
}| <div className="mt-5 content-center "> | ||
| {winner ? ( | ||
| <h2 className="text-2xl font-semibold text-green-600 text-center"> | ||
| {winner} Wins! | ||
| </h2> | ||
| ) : board.every((cell) => cell) ? ( | ||
| <h2 className="text-2xl font-semibold text-red-600 text-center">It's a Draw!</h2> | ||
| ) : ( | ||
| <h2 className="text-xl font-semibold text-gray-700 text-center"> | ||
| Next Player: {isXNext ? 'X' : 'O'} | ||
| </h2> | ||
| )} | ||
| <button | ||
| onClick={resetGame} | ||
| className="ml-1 mt-4 px-4 py-2 bg-blue-500 text-white rounded-lg hover:bg-blue-600 hover:text-white transition-all duration-200" | ||
| > | ||
| Reset Game | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance game status with score tracking
Consider adding a score tracking feature to make the game more engaging across multiple rounds.
Add score tracking:
+ const [scores, setScores] = useState({ X: 0, O: 0 });
const resetGame = () => {
setBoard(Array(9).fill(null));
setIsXNext(true);
};
+ const updateScore = (winner) => {
+ if (winner) {
+ setScores(prev => ({
+ ...prev,
+ [winner]: prev[winner] + 1
+ }));
+ }
+ };
+ // Add score display in the UI
+ <div className="mt-3 text-lg">
+ <span className="mr-4">X: {scores.X}</span>
+ <span>O: {scores.O}</span>
+ </div>Committable suggestion was skipped due to low confidence.
| function calculateWinner(board) { | ||
| const lines = [ | ||
| [0, 1, 2], | ||
| [3, 4, 5], | ||
| [6, 7, 8], | ||
| [0, 3, 6], | ||
| [1, 4, 7], | ||
| [2, 5, 8], | ||
| [0, 4, 8], | ||
| [2, 4, 6], | ||
| ]; | ||
|
|
||
| for (let i = 0; i < lines.length; i++) { | ||
| const [a, b, c] = lines[i]; | ||
| if (board[a] && board[a] === board[b] && board[a] === board[c]) { | ||
| return board[a]; | ||
| } | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize winner calculation with memoization
The winner calculation could be optimized to prevent unnecessary recalculations.
Consider using useMemo:
+ import React, { useState, useMemo } from 'react';
function TicTacToe() {
- const winner = calculateWinner(board);
+ const winner = useMemo(() => calculateWinner(board), [board]);Committable suggestion was skipped due to low confidence.
| {board.map((value, index) => ( | ||
| <button | ||
| key={index} | ||
| onClick={() => handleClick(index)} | ||
| className="w-20 h-20 bg-amber-200 flex items-center justify-center text-3xl font-semibold text-black-800 border-2 border-amber-500 hover:bg-amber-300 transition-all duration-200 rounded-md" | ||
| > | ||
| {value} | ||
| </button> | ||
| ))} |
There was a problem hiding this comment.
Improve accessibility and keyboard navigation
The game board buttons need accessibility enhancements:
- Missing aria-labels for screen readers
- No keyboard navigation support
Apply these improvements:
<button
key={index}
onClick={() => handleClick(index)}
+ aria-label={`Cell ${index + 1}, ${value || 'empty'}`}
+ tabIndex={0}
+ disabled={board[index] || winner}
className={`w-20 h-20 bg-amber-200 flex items-center justify-center text-3xl
- font-semibold text-black-800 border-2 border-amber-500 hover:bg-amber-300
+ font-semibold text-black-800 border-2 border-amber-500
+ ${!board[index] && !winner ? 'hover:bg-amber-300' : 'cursor-not-allowed opacity-75'}
transition-all duration-200 rounded-md`}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {board.map((value, index) => ( | |
| <button | |
| key={index} | |
| onClick={() => handleClick(index)} | |
| className="w-20 h-20 bg-amber-200 flex items-center justify-center text-3xl font-semibold text-black-800 border-2 border-amber-500 hover:bg-amber-300 transition-all duration-200 rounded-md" | |
| > | |
| {value} | |
| </button> | |
| ))} | |
| {board.map((value, index) => ( | |
| <button | |
| key={index} | |
| onClick={() => handleClick(index)} | |
| aria-label={`Cell ${index + 1}, ${value || 'empty'}`} | |
| tabIndex={0} | |
| disabled={board[index] || winner} | |
| className={`w-20 h-20 bg-amber-200 flex items-center justify-center text-3xl font-semibold text-black-800 border-2 border-amber-500 ${!board[index] && !winner ? 'hover:bg-amber-300' : 'cursor-not-allowed opacity-75'} transition-all duration-200 rounded-md`} | |
| > | |
| {value} | |
| </button> | |
| ))} |
| @@ -396,7 +397,7 @@ function Boardgame() { | |||
| onClick={() => handleInstantPlay(board)} | |||
| className="px-4 py-2 text-white bg-blue-500 rounded-lg opacity-0 transition-opacity duration-700 delay-300 group-hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-blue-500" | |||
| > | |||
| Instant Play | |||
| <Link to="/TicTacToe">Instant Play</Link> | |||
There was a problem hiding this comment.
Conditionally render the Instant Play button.
Currently, the "Instant Play" button appears for all board games, but based on the PR objectives, only Tic Tac Toe should be playable. This might confuse users who expect other games to be playable as well.
Consider conditionally rendering the button only for Tic Tac Toe:
<div className="flex space-x-2 mt-4">
<button
onClick={() => handleOpenInstructions(board)}
className="px-4 py-2 text-white bg-green-500 rounded-lg opacity-0 transition-opacity duration-700 delay-300 group-hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-blue-500"
>
See Instructions
</button>
+ {board.title === "Tic-Tac-Toe" && (
<Link
to="/TicTacToe"
className="inline-block px-4 py-2 text-white bg-blue-500 rounded-lg opacity-0 transition-opacity duration-700 delay-300 group-hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-blue-500"
>
Instant Play
</Link>
+ )}
</div>Committable suggestion was skipped due to low confidence.
Fix button accessibility and remove unused handler.
The current implementation has several issues:
- Nesting a Link inside a button is an accessibility anti-pattern as it creates nested interactive elements
- There's an unused onClick handler (handleInstantPlay)
- The styling might not properly propagate to the Link
Consider this implementation instead:
-<button
- onClick={() => handleInstantPlay(board)}
- className="px-4 py-2 text-white bg-blue-500 rounded-lg opacity-0 transition-opacity duration-700 delay-300 group-hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-blue-500"
->
- <Link to="/TicTacToe">Instant Play</Link>
-</button>
+<Link
+ to="/TicTacToe"
+ className="inline-block px-4 py-2 text-white bg-blue-500 rounded-lg opacity-0 transition-opacity duration-700 delay-300 group-hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-blue-500"
+>
+ Instant Play
+</Link>Committable suggestion was skipped due to low confidence.
| <a href="https://github.com/Ashwinib26"> | ||
| <img src="https://avatars.githubusercontent.com/u/149402720?v=4" width="100;" alt="Ashwinib26"/> | ||
| <br /> | ||
| <sub><b>alolika bhowmik</b></sub> | ||
| <sub><b>Ashwini_ab</b></sub> |
There was a problem hiding this comment.
Fix indentation using spaces instead of tabs.
The indentation in these sections uses hard tabs. For consistency with the rest of the file and to follow markdown best practices, replace tabs with spaces.
Apply this change to fix the indentation:
- <a href="https://github.com/Ashwinib26">
- <img src="https://avatars.githubusercontent.com/u/149402720?v=4" width="100;" alt="Ashwinib26"/>
- <br />
- <sub><b>Ashwini_ab</b></sub>
+ <a href="https://github.com/Ashwinib26">
+ <img src="https://avatars.githubusercontent.com/u/149402720?v=4" width="100;" alt="Ashwinib26"/>
+ <br />
+ <sub><b>Ashwini_ab</b></sub>Also applies to: 286-289
RamakrushnaBiswal
added
enhancement
There was a problem hiding this comment.
@Sumanbhadra resolve the conflicts and resolve coderabit changes
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
frontend/src/components/Pages/Games/TicTacToe.jsx (2)
3-6: Add cleanup logic for component unmountingConsider adding a cleanup mechanism using useEffect to prevent any potential memory leaks, especially if you plan to add features like timers or multiplayer functionality in the future.
+ import React, { useState, useEffect } from 'react'; function TicTacToe() { const [board, setBoard] = useState(Array(9).fill(null)); const [isXNext, setIsXNext] = useState(true); const winner = calculateWinner(board); + + useEffect(() => { + return () => { + // Cleanup logic for future implementations + }; + }, []);
40-53: Enhance game status accessibilityAdd ARIA live regions to announce game status changes to screen readers.
- <div className="mt-5 content-center "> + <div className="mt-5 content-center" role="status" aria-live="polite">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- frontend/src/components/Pages/Boardgame.jsx (9 hunks)
- frontend/src/components/Pages/Games/TicTacToe.jsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/Pages/Boardgame.jsx
🔇 Additional comments (1)
frontend/src/components/Pages/Games/TicTacToe.jsx (1)
1-86: Implementation doesn't match online gameplay requirementThe current implementation is a local-only game, but issue #385 specifically requests an online multiplayer feature. Consider extending this implementation to support online gameplay:
- Add WebSocket connection for real-time game state sync
- Implement player rooms/matching
- Add online player status indicators
Let's check if there are any WebSocket-related implementations in the codebase:
Would you like assistance in implementing the online multiplayer feature?
| aria-label={`Cell ${index + 1}, ${value || 'empty'}`} | ||
| tabIndex={0} | ||
| disabled={board[index] || winner} | ||
| className="w-20 h-20 bg-amber-200 flex items-center justify-center text-3xl font-semibold text-black-800 border-2 border-amber-500 ${!board[index] && !winner ? 'hover:bg-amber-300' : 'cursor-not-allowed opacity-75'} transition-all duration-200 rounded-md" |
There was a problem hiding this comment.
Fix template string syntax in className
The className contains an improperly formatted template string that won't work as expected.
- className="w-20 h-20 bg-amber-200 flex items-center justify-center text-3xl font-semibold text-black-800 border-2 border-amber-500 ${!board[index] && !winner ? 'hover:bg-amber-300' : 'cursor-not-allowed opacity-75'} transition-all duration-200 rounded-md"
+ className={`w-20 h-20 bg-amber-200 flex items-center justify-center text-3xl font-semibold text-black-800 border-2 border-amber-500 ${!board[index] && !winner ? 'hover:bg-amber-300' : 'cursor-not-allowed opacity-75'} transition-all duration-200 rounded-md`}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| className="w-20 h-20 bg-amber-200 flex items-center justify-center text-3xl font-semibold text-black-800 border-2 border-amber-500 ${!board[index] && !winner ? 'hover:bg-amber-300' : 'cursor-not-allowed opacity-75'} transition-all duration-200 rounded-md" | |
| className={`w-20 h-20 bg-amber-200 flex items-center justify-center text-3xl font-semibold text-black-800 border-2 border-amber-500 ${!board[index] && !winner ? 'hover:bg-amber-300' : 'cursor-not-allowed opacity-75'} transition-all duration-200 rounded-md`} |
| <button | ||
| onClick={resetGame} | ||
| className="ml-1 mt-4 px-4 py-2 bg-blue-500 text-white rounded-lg hover:bg-blue-600 hover:text-white transition-all duration-200" | ||
| > | ||
| Reset Game | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add keyboard shortcut for game reset
Consider adding a keyboard shortcut (e.g., 'R' key) for resetting the game to improve user experience.
+ useEffect(() => {
+ const handleKeyPress = (e) => {
+ if (e.key.toLowerCase() === 'r') {
+ resetGame();
+ }
+ };
+ window.addEventListener('keydown', handleKeyPress);
+ return () => window.removeEventListener('keydown', handleKeyPress);
+ }, []);
<button
onClick={resetGame}
+ title="Press 'R' to reset game"
className="ml-1 mt-4 px-4 py-2 bg-blue-500 text-white rounded-lg hover:bg-blue-600 hover:text-white transition-all duration-200"
>
Reset Game
</button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| onClick={resetGame} | |
| className="ml-1 mt-4 px-4 py-2 bg-blue-500 text-white rounded-lg hover:bg-blue-600 hover:text-white transition-all duration-200" | |
| > | |
| Reset Game | |
| </button> | |
| </div> | |
| useEffect(() => { | |
| const handleKeyPress = (e) => { | |
| if (e.key.toLowerCase() === 'r') { | |
| resetGame(); | |
| } | |
| }; | |
| window.addEventListener('keydown', handleKeyPress); | |
| return () => window.removeEventListener('keydown', handleKeyPress); | |
| }, []); | |
| <button | |
| onClick={resetGame} | |
| title="Press 'R' to reset game" | |
| className="ml-1 mt-4 px-4 py-2 bg-blue-500 text-white rounded-lg hover:bg-blue-600 hover:text-white transition-all duration-200" | |
| > | |
| Reset Game | |
| </button> | |
| </div> |
| const handleClick = (index) => { | ||
| if (board[index] || winner) return; | ||
| const newBoard = board.slice(); | ||
| newBoard[index] = isXNext ? 'X' : 'O'; | ||
| setBoard(newBoard); | ||
| setIsXNext(!isXNext); | ||
| }; |
There was a problem hiding this comment.
Add input validation and state update safety
The handleClick function should validate the input index and use the functional update form of setState to handle potential race conditions.
const handleClick = (index) => {
+ if (typeof index !== 'number' || index < 0 || index > 8) return;
if (board[index] || winner) return;
- const newBoard = board.slice();
- newBoard[index] = isXNext ? 'X' : 'O';
- setBoard(newBoard);
+ setBoard(currentBoard => {
+ if (currentBoard[index]) return currentBoard;
+ const newBoard = currentBoard.slice();
+ newBoard[index] = isXNext ? 'X' : 'O';
+ return newBoard;
+ });
setIsXNext(!isXNext);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleClick = (index) => { | |
| if (board[index] || winner) return; | |
| const newBoard = board.slice(); | |
| newBoard[index] = isXNext ? 'X' : 'O'; | |
| setBoard(newBoard); | |
| setIsXNext(!isXNext); | |
| }; | |
| const handleClick = (index) => { | |
| if (typeof index !== 'number' || index < 0 || index > 8) return; | |
| if (board[index] || winner) return; | |
| setBoard(currentBoard => { | |
| if (currentBoard[index]) return currentBoard; | |
| const newBoard = currentBoard.slice(); | |
| newBoard[index] = isXNext ? 'X' : 'O'; | |
| return newBoard; | |
| }); | |
| setIsXNext(!isXNext); | |
| }; |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@Sumanbhadra error while deployment |
|
This PR has been automatically closed due to inactivity from the owner for 3 days. |
Fixes: #385
Screen.Recording.2024-10-26.003644.mp4
@RamakrushnaBiswal take a look.
Summary by CodeRabbit
Release Notes
New Features
Documentation