feat(gamescreen): add infoBloc, rebrand ui and refactor progressBar #323
Conversation
WalkthroughThe pull request introduces several changes across multiple components in the game interface. The Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
components/gamescreen/ProgressBar.jsx (2)
8-8: LGTM! Consider extracting magic numbers.The padding and margin adjustments align with the UI rebranding objectives. However, the Progress.Bar component has hardcoded values that could be made configurable.
Consider extracting the hardcoded values into constants or props:
const ProgressBar = ({ currentQuestionIndex, questions }) => { + const PROGRESS_WIDTH = 300; + const PROGRESS_HEIGHT = 10; + const PROGRESS_COLOR = '#62C0CC'; return ( <View className="flex justify-center items-center m-4 p-2 rounded-lg" > <Progress.Bar progress={(currentQuestionIndex + 1) / questions.length} - width={300} - height={10} - color="#62C0CC" + width={PROGRESS_WIDTH} + height={PROGRESS_HEIGHT} + color={PROGRESS_COLOR} /> </View> ); };
Line range hint
1-19: Consider adding PropTypes and documentation.The component looks clean and focused, but could benefit from proper prop validation and documentation.
Add PropTypes and JSDoc:
import React from "react"; import { View, Text } from "react-native"; import * as Progress from "react-native-progress"; +import PropTypes from 'prop-types'; +/** + * ProgressBar component that shows the current progress in a quiz/game + * @param {number} currentQuestionIndex - Zero-based index of the current question + * @param {Array} questions - Array of all questions + * @returns {React.Component} A progress bar showing completion status + */ const ProgressBar = ({ currentQuestionIndex, questions }) => { // ... existing code ... }; +ProgressBar.propTypes = { + currentQuestionIndex: PropTypes.number.isRequired, + questions: PropTypes.array.isRequired, +}; export default ProgressBar;screens/Gamescreen.js (3)
42-44: Document the question limit rationaleThe hard-coded limit of 45 questions should be explained with a comment, and consider making it a named constant.
+// Maximum number of questions per game session to maintain optimal gameplay duration +const MAX_QUESTIONS = 45; -const limitedQuestions = shuffledQuestions.slice(0, 45); +const limitedQuestions = shuffledQuestions.slice(0, MAX_QUESTIONS);
49-54: Enhance player score initializationThe score initialization could be more robust by ensuring non-negative values and considering additional player stats.
parsedPlayers = parsedPlayers.map(player => ({ ...player, - score: player.score || 0 + score: Math.max(0, parseInt(player.score) || 0), + gamesPlayed: player.gamesPlayed || 0, }));
Line range hint
113-121: Improve loading state handlingConsider disabling user interactions during loading state and showing a loading indicator in the GameCard component.
<GameCard item={questions[currentQuestionIndex]} currentPlayer={players[currentQuestionIndex % players.length]} isLoading={isLoading} + disabled={isLoading} players={players} questions={questions} currentQuestionIndex={currentQuestionIndex} /> -<NextRoundButton onPress={handleNextRound} isLoading={isLoading} /> +<NextRoundButton + onPress={handleNextRound} + isLoading={isLoading} + disabled={isLoading} +/>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
components/gamescreen/GameCard.jsx(1 hunks)components/gamescreen/InfoBloc.jsx(1 hunks)components/gamescreen/ProgressBar.jsx(1 hunks)package.json(1 hunks)screens/Gamescreen.js(4 hunks)
✅ Files skipped from review due to trivial changes (3)
- components/gamescreen/GameCard.jsx
- components/gamescreen/InfoBloc.jsx
- package.json
🔇 Additional comments (3)
components/gamescreen/ProgressBar.jsx (1)
Line range hint 1-19: Verify error handling for edge cases.
The progress calculation assumes valid inputs but doesn't handle edge cases.
Let's check if there are any error boundaries or error handling in parent components:
Consider adding input validation:
const ProgressBar = ({ currentQuestionIndex, questions }) => {
+ if (!Array.isArray(questions) || questions.length === 0) {
+ return null;
+ }
+
+ const progress = Math.min(
+ Math.max((currentQuestionIndex + 1) / questions.length, 0),
+ 1
+ );
+
return (
<View
className="flex justify-center items-center m-4 p-2 rounded-lg"
>
<Progress.Bar
- progress={(currentQuestionIndex + 1) / questions.length}
+ progress={progress}
width={300}
height={10}
color="#62C0CC"
/>
</View>
);
};screens/Gamescreen.js (2)
16-16: LGTM: Clean import addition
The InfoBloc component import is properly placed with other component imports.
33-34: Verify database migration from V3 to V4
The query has been updated from questionsV3 to questionsV4. This version change might affect other parts of the application.
✅ Verification successful
No migration concerns found for questionsV4 table
The codebase scan shows that questionsV4 is the only version of the questions table being used, with no remaining references to questionsV3. This suggests a clean implementation without version conflicts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to questionsV3
rg "questionsV3" --type js
Length of output: 26
Script:
#!/bin/bash
# Search for any Supabase table queries to understand the database structure
rg "supabase.*from\(" --type js -A 2
Length of output: 263
screens/Gamescreen.js
Outdated
| @@ -86,7 +90,7 @@ export default function Gamescreen() { | |||
| } | |||
|
|
|||
| if (currentQuestionIndex + 1 >= questions.length) { | |||
| // No more questions, navigate to Endscreen or reset for a new game | |||
| <InfoBloc info={isLoading ? "Loading..." : questions[currentQuestionIndex]?.Instructions || "No instructions available."} /> | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consolidate duplicate InfoBloc usage
The InfoBloc component is rendered twice with identical props. Consider consolidating this into a single component or creating a custom hook for the info message.
+const getInfoMessage = (isLoading, currentQuestion) =>
+ isLoading ? "Loading..." : currentQuestion?.Instructions || "No instructions available.";
-<InfoBloc info={isLoading ? "Loading..." : questions[currentQuestionIndex]?.Instructions || "No instructions available."} />
+<InfoBloc info={getInfoMessage(isLoading, questions[currentQuestionIndex])} />Also applies to: 113-113
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
components/settingscreen/SuggestionModal.jsx (2)
29-29: Consider using consistent styling approachThe modal background style mixes Tailwind classes with inline styles. Consider using Tailwind's opacity utilities and custom colors in your theme config for better maintainability.
-<View className=" p-6 rounded-lg w-11/12" style={{ backgroundColor: "rgba(24, 164, 129, 0.9)" }} > +<View className="p-6 rounded-lg w-11/12 bg-emerald-500/90">
Line range hint
1-55: Consider adding form validation and error handlingThe current implementation lacks:
- Input validation
- Error handling for submission failures
- Feedback to users on successful submission
Would you like me to provide a more robust implementation with proper form validation and error handling?
screens/Gamescreen.js (1)
42-44: Extract magic number to a named constant.The hardcoded limit of 45 questions should be defined as a named constant at the top of the file for better maintainability and documentation.
+const MAX_QUESTIONS = 45; // Maximum number of questions per game session + export default function Gamescreen() { // ... existing code ... - const limitedQuestions = shuffledQuestions.slice(0, 45); + const limitedQuestions = shuffledQuestions.slice(0, MAX_QUESTIONS);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
components/gamescreen/ProgressBar.jsx(1 hunks)components/settingscreen/SuggestionModal.jsx(2 hunks)screens/Gamescreen.js(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/gamescreen/ProgressBar.jsx
🔇 Additional comments (3)
screens/Gamescreen.js (3)
16-16: LGTM! Import statement is correctly placed.
The InfoBloc component import follows the established pattern of component imports.
112-112: LGTM! InfoBloc implementation is clean and handles all states.
The component correctly handles:
- Loading state
- Missing instructions
- Optional chaining for safe property access
Note: The previous review comment about duplicate InfoBloc usage is no longer applicable as the implementation has been consolidated.
33-33: Verify the impact of questionsV3 to questionsV4 migration.
The change in table version could impact existing deployments. Please ensure:
- Data migration from V3 to V4 is completed
- Backward compatibility is maintained if needed
- Rollback strategy is in place
| <TouchableOpacity onPress={handleSubmit} className="self-end bg-black p-2 rounded-xl"> | ||
| <Text className="text-white text-lg font-semibold mx-2">Envoyer</Text> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance button accessibility and user experience
The submit button could benefit from:
- A loading state during submission
- Proper accessibility labels
- Visual feedback on press
-<TouchableOpacity onPress={handleSubmit} className="self-end bg-black p-2 rounded-xl">
- <Text className="text-white text-lg font-semibold mx-2">Envoyer</Text>
+<TouchableOpacity
+ onPress={handleSubmit}
+ accessibilityLabel="Submit suggestion"
+ accessibilityRole="button"
+ className="self-end bg-black active:bg-black/80 p-2 rounded-xl"
+>
+ <Text className="text-white text-lg font-semibold mx-2">
+ {isSubmitting ? 'Envoi...' : 'Envoyer'}
+ </Text>
</TouchableOpacity>Committable suggestion skipped: line range outside the PR's diff.
Pull Request Template
Description
Please include a summary of the change and which issue is fixed. Also, include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.
Screenshots
If applicable, add screenshots to help explain your changes or demonstrate the UI/UX impact.
Summary by CodeRabbit
Release Notes
New Features
InfoBloccomponent to display loading messages and instructions for questions.Bug Fixes
Style
GameCardandProgressBarcomponents for a cleaner appearance.ProgressBarby removing the text label.SuggestionModaland its button for improved usability.Chores
expo-dev-clientdependency to the latest version.