[client] welcome message even if you've done quests #2481
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThe changes in this pull request involve significant modifications to the Changes
Assessment against linked issues
Possibly related PRs
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 (
|
|
You are out of MentatBot reviews. Your usage will refresh December 16 at 08:00 AM. |
|
Failed to generate code suggestions for PR |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
client/src/ui/modules/navigation/QuestMenu.tsx (3)
Line range hint
23-23: Consolidate duplicate world loading state.The component fetches the same state twice from
useWorldStore. Consider using a single variable to improve maintainability.- const worldLoading = useWorldStore((state) => state.isWorldLoading); - const isWorldLoading = useWorldStore((state) => state.isWorldLoading); + const isWorldLoading = useWorldStore((state) => state.isWorldLoading);Also applies to: 37-37
Line range hint
44-57: Enhance error logging in claim handlers.While the error handling is good, consider adding more context to error messages to aid debugging.
- console.error(`Failed to claim resources for quest ${currentQuest?.name}:`, error); + console.error(`Failed to claim resources for quest ${currentQuest?.name}. Quest ID: ${currentQuest?.id}, Prizes: ${currentQuest?.prizes.length}:`, error); - console.error(`Failed to claim resources for quests:`, error); + console.error(`Failed to claim resources for quests. Total unclaimed: ${unclaimedQuests.length}, Total prizes: ${unclaimedQuests.flatMap((quest) => quest.prizes).length}:`, error);Also applies to: 61-79
163-199: Extract skip quest buttons into a separate component.The skip quest UI logic is complex enough to warrant its own component. This would improve readability and maintainability.
Consider extracting this into a
SkipQuestButtonscomponent:interface SkipQuestButtonsProps { skipQuest: boolean; isLoading: boolean; onSkip: () => void; onCancel: () => void; onClaimAll: () => void; onClaim: () => void; } const SkipQuestButtons: React.FC<SkipQuestButtonsProps> = ({ skipQuest, isLoading, onSkip, onCancel, onClaimAll, onClaim, }) => { if (skipQuest) { return ( <div className="flex flex-row gap-4"> <Button className="text-sm font-semibold capitalize" onClick={onClaimAll} variant="red" disabled={isLoading} > {isLoading ? "Loading..." : "Skip All Quests"} </Button> {/* ... other buttons ... */} </div> ); } return ( <Button variant="primary" className="text-sm font-semibold capitalize w-6" onClick={onSkip} disabled={isLoading} > {isLoading ? "..." : "Skip"} </Button> ); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
client/src/ui/modules/navigation/QuestMenu.tsx(4 hunks)client/src/ui/modules/navigation/TopLeftNavigation.tsx(1 hunks)
🔇 Additional comments (1)
client/src/ui/modules/navigation/TopLeftNavigation.tsx (1)
322-322: LGTM! Good error handling improvement.
The changes look good. The unconditional rendering of QuestsMenu aligns with the component's new internal state management, and the added error handling in goToMapView improves robustness.
| > | ||
| {worldLoading ? "Loading..." : "Confirm"} | ||
| {worldLoading ? "Loading..." : "Claim"} |
There was a problem hiding this comment.
worldLoading has to be false now for the component to render, so we should remove these
Fixes #2479
Summary by CodeRabbit
New Features
QuestsMenucomponent now fetches the count of unclaimed quests internally, enhancing its self-sufficiency.QuestsMenuis always displayed in theTopLeftNavigation, regardless of the unclaimed quests count.Bug Fixes
UI Enhancements
QuestsMenuto show buttons based on loading and quest completion states.