-
Notifications
You must be signed in to change notification settings - Fork 51
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
add registration countdown and register claim button #2600
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces updates to the Eternum game's landing page, enhancing leaderboard and reward claiming functionality. Key changes include the addition of a "Claim" menu item in the sidebar, new asynchronous functions for leaderboard interactions, modifications to GraphQL queries, and the creation of a new route and component for claiming leaderboard rewards. These updates improve user navigation and interaction with game features related to leaderboard contributions and reward claims. Changes
Sequence DiagramsequenceDiagram
participant User
participant Sidebar
participant ClaimRoute
participant LeaderboardManager
participant SystemCalls
User->>Sidebar: Click "Claim" menu item
Sidebar->>ClaimRoute: Navigate to /claim
ClaimRoute->>LeaderboardManager: Check reward claim status
LeaderboardManager-->>ClaimRoute: Return claim status
ClaimRoute->>SystemCalls: Check registration
SystemCalls-->>ClaimRoute: Return registration status
ClaimRoute->>User: Display leaderboard info and claim options
Possibly related PRs
Suggested Reviewers
Poem
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 30 at 08:00 AM. |
Failed to generate code suggestions for PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (10)
landing/src/hooks/gql/gql.ts (1)
18-32
: Consider Code Splitting fordocuments
Object
Thedocuments
object at lines 18-32 holds all GraphQL queries together, which can lead to unnecessary bundle bloat. Consider using the recommended plugin-based approach, code splitting, or dynamic imports to reduce bundle size.landing/src/hooks/gql/graphql.ts (3)
7332-7336
: Consider adding pagination to leaderboard queryThe
GetLeaderboard
query fetches all leaderboard data without pagination. While this works for now, consider adding pagination parameters for better scalability.-export type GetLeaderboardQueryVariables = Exact<{ [key: string]: never; }>; +export type GetLeaderboardQueryVariables = Exact<{ + first?: InputMaybe<Scalars['Int']>; + after?: InputMaybe<Scalars['Cursor']>; +}>;
7337-7343
: Add limit parameter to hyperstructure contributions queryThe
GetHyperstructureContributions
query has a hardcoded limit of 1000. Consider making this configurable.-export type GetHyperstructureContributionsQueryVariables = Exact<{ - accountAddress: Scalars['ContractAddress']['input']; -}>; +export type GetHyperstructureContributionsQueryVariables = Exact<{ + accountAddress: Scalars['ContractAddress']['input']; + limit?: InputMaybe<Scalars['Int']>; +}>;
7344-7348
: Consider adding filtering options to epochs queryThe
GetEpochs
query fetches all epochs with a fixed limit. Consider adding filtering options for more targeted queries.-export type GetEpochsQueryVariables = Exact<{ [key: string]: never; }>; +export type GetEpochsQueryVariables = Exact<{ + fromTimestamp?: InputMaybe<Scalars['u64']>; + hyperstructureId?: InputMaybe<Scalars['u32']>; + limit?: InputMaybe<Scalars['Int']>; +}>;landing/src/hooks/query/leaderboard.tsx (1)
15-15
: Typographical adjustment on 'GET_PLAYER_HAS_REGISTRED'“Registered” is misspelled. Consider renaming to
GET_PLAYER_HAS_REGISTERED
to improve clarity and consistency.-export const GET_PLAYER_HAS_REGISTRED = graphql(` +export const GET_PLAYER_HAS_REGISTERED = graphql(`landing/src/components/modules/app-sidebar.tsx (1)
39-39
: Differentiate icons for better UXBoth "Claim" and "Marketplace" routes use the
Scale
icon, which can be confusing for users. Consider assigning distinct icons to make each menu item visually unique.landing/src/hooks/usePrizeClaim.tsx (2)
13-47
: Separate query keys for clarityUsing
["address", playerAddress]
as a query key for bothuseLeaderboardEntry
anduseHasPlayerClaimed
might cause name collisions if the data shapes or revalidation criteria differ. Consider prepending or appending a unique identifier, such as["leaderboard-entry", playerAddress]
.- queryKey: ["address", playerAddress], + queryKey: ["leaderboard-entry", playerAddress],
16-66
: Consolidate shared refetch intervalSeveral queries use
refetchInterval: 10_000
. Consider extracting this into a constant (e.g.,REFRESH_INTERVAL = 10_000;
) or a shared config to ensure consistency and make future updates easier.- refetchInterval: 10_000, + refetchInterval: REFRESH_INTERVAL,landing/src/dojo/modelManager/leaderboard/LeaderboardManager.ts (1)
172-178
: Return value might be undefined if the record is missing.
Currently,claimed?.claimed
can beundefined
if the component record does not exist. Ensure all calling code handles undefined properly. If you want a strict boolean, consider returning!!claimed?.claimed
.- return claimed?.claimed; + return !!claimed?.claimed;landing/src/routes/claim.lazy.tsx (1)
74-83
: Basic error handling and concurrency flow.
TheonRegister
function is guarded by checks foraccount
,hyperstructures
, andepochs
. The finalfinally
block elegantly resets the loading state. Consider adding user feedback if the user is not eligible to register.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
landing/src/components/modules/app-sidebar.tsx
(1 hunks)landing/src/dojo/createSystemCalls.ts
(2 hunks)landing/src/dojo/modelManager/leaderboard/LeaderboardManager.ts
(3 hunks)landing/src/hooks/gql/gql.ts
(2 hunks)landing/src/hooks/gql/graphql.ts
(2 hunks)landing/src/hooks/query/leaderboard.tsx
(1 hunks)landing/src/hooks/usePrizeClaim.tsx
(1 hunks)landing/src/routeTree.gen.ts
(3 hunks)landing/src/routes/claim.lazy.tsx
(1 hunks)
🔇 Additional comments (21)
landing/src/hooks/gql/gql.ts (1)
2-3
: Imports Look Good
The import statement is straightforward and follows conventional patterns. No issues spotted here.
landing/src/hooks/gql/graphql.ts (6)
7306-7324
: New query definition for game status check looks good
The HasGameEnded
query and its associated types are well-structured and properly typed. The query correctly fetches the winner's address from the GameEnded
model.
7311-7317
: Player registration query implementation is correct
The HasPlayerRegistered
query is properly implemented to check registration status by counting owner records for a given account address.
7318-7324
: Reward claim status query is well designed
The HasPlayerClaimed
query correctly checks if a player has claimed rewards by counting records in the LeaderboardRewardClaimed
model.
7325-7331
: Leaderboard entry query implementation is accurate
The GetLeaderboardEntry
query properly fetches a player's leaderboard entry with points and address fields.
7574-7590
: Consider adding error handling for epoch data
The getEpochs
query response should be handled carefully as the owners array could be empty or null.
7505-7515
: Verify game end state handling
The hasGameEnded
query should be used with caution as it may return null if no game has ended. Ensure proper null handling in the consuming code.
✅ Verification successful
Let me try a different approach to find where and how the game end state is handled.
Proper null handling is implemented for game end state
Based on the code analysis, the game end state and winner_address are properly handled with null checks:
- In
usePrizeClaim.tsx
, theuseGameWinner
hook safely handles null with the nullish coalescing operator:
const winnerAddress = data?.s0EternumGameEndedModels?.edges?.[0]?.node?.winner_address ?? null;
- In
Rewards.tsx
, the winner address is safely accessed with optional chaining and proper null handling:
const seasonWinner = useMemo(() => {
if (!gameEnded) return "";
const seasonWinner = getComponentValue(AddressName, getEntityIdFromKeys([gameEnded?.winner_address]));
return shortString.decodeShortString(seasonWinner?.name.toString() ?? "");
}, [gameEnded]);
The code properly handles cases where:
- No game has ended (gameEnded is null)
- The winner address is not set
- The winner's name is not available
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential null handling issues in game end state usage
# Search for direct usage of winner_address without null checks
rg -A 5 'winner_address(?!.*\?)'
Length of output: 274
Script:
#!/bin/bash
# Search for usage of winner_address and HasGameEndedDocument
rg -p "winner_address|HasGameEndedDocument" -A 5 --type ts
Length of output: 5396
Script:
#!/bin/bash
# Search for files containing hasGameEnded related code
fd -e ts -e tsx -x grep -l "hasGameEnded"
Length of output: 209
Script:
#!/bin/bash
# Search for the actual implementation of game end state handling
ast-grep --pattern 'const $_ = useQuery(HasGameEndedDocument, $_)'
Length of output: 68
Script:
#!/bin/bash
# Let's check the implementation in usePrizeClaim.tsx since it uses winner_address
cat landing/src/hooks/usePrizeClaim.tsx
Length of output: 3146
Script:
#!/bin/bash
# Let's also check the rewards component that uses gameEnded.winner_address
cat "client/src/ui/modules/rewards/Rewards.tsx"
Length of output: 7024
landing/src/hooks/query/leaderboard.tsx (1)
62-66
: Validate pagination limit usage
You're using limit: 1000
for fetching contributions, which may be sufficient in many cases, but large queries can lead to performance issues or partial data retrieval if 1000 isn’t large enough. Consider allowing for dynamic pagination to handle bigger data sets.
landing/src/hooks/usePrizeClaim.tsx (1)
88-89
: Validate numeric conversion
Ensure hyperstructure_entity_id
is guaranteed to be a number before casting with Number(...)
. If the field may contain invalid data, handle potential NaN
cases to avoid downstream errors.
landing/src/routeTree.gen.ts (2)
23-50
: Auto-generated file note
This file is auto-generated by TanStack Router. Manual changes may be lost on regeneration. Ensure that your route definitions in the source files (e.g., claim.lazy.tsx
) remain consistent with desired routing behaviors.
83-88
: Successfully registered new '/claim' route
The route definition for /claim
is consistent and follows the established pattern. This integration into FileRoutesByPath
appears correct.
landing/src/dojo/createSystemCalls.ts (4)
146-148
: The method name accurately describes intent and aligns with the existing pattern.
The register_to_leaderboard
function mirrors the established design pattern of directly calling the provider method. No issues are observed here.
150-152
: Ensure consistent usage across the codebase.
The end_game
function is consistent in naming and usage with the rest of this file. Verify that all calls to end_game
align with the new method signature and that any end-of-game logic in upstream or downstream code remains valid.
154-156
: Matches existing error and queue handling pattern.
This claim_leaderboard_rewards
function is properly wrapped by the same queueing and error-handling logic as other calls. Good practice for consistent reliability.
184-186
: All new system calls are properly wrapped.
The newly added calls are correctly included in the systemCalls
object and wrapped with both withQueueing
and withErrorHandling
. This ensures consistent async flow and error feedback.
landing/src/dojo/modelManager/leaderboard/LeaderboardManager.ts (1)
116-117
: Validate game logic for skipping point accumulation.
By returning early if a reward has been claimed, the player no longer accumulates points. This could be correct if once players have claimed, they should not continue earning. However, confirm that this is the intended gameplay mechanic, as it permanently prevents further scoring for the user.
Also applies to: 158-159
landing/src/routes/claim.lazy.tsx (5)
16-18
: The route definition appears correctly configured.
The code uses createLazyFileRoute
to define the /claim
route. This is consistent with typical router usage across the app.
20-32
: Good use of hooks and state for claim logic.
The Claim
component properly retrieves systemCalls
and user account data. The state variables (registerLoading
, claimLoading
, timeLeft
) are neatly organized.
41-66
: Correctly handles registration countdown.
The useEffect
hook and timer logic correctly compute the difference between current time and registrationEnd
. The cleanup call ensures the timer is properly cleared.
85-90
: Consistent approach for claiming rewards.
The onClaim
function mirrors the structure of onRegister
, with synchronous checks and a finally
block. Verify any additional constraints in upstream logic (e.g., claiming allowed only after the registration period).
93-175
: UI layout and fallback states appear user-friendly.
The loading overlay and conditional button states (register vs. claim) nicely guide user interactions. The usage of Suspense
for nested content is appropriate. Overall, the file is well-structured with clear responsibility.
export function graphql(source: "\n query getCapacitySpeedConfig($category: Enum!, $entityType: u32!) {\n s0EternumCapacityConfigModels(where: {category: $category }) {\n edges{\n node {\n weight_gram\n }\n }\n }\n s0EternumSpeedConfigModels(where: {entity_type: $entityType }) {\n edges{\n node {\n sec_per_km\n }\n }\n }\n }\n"): typeof import('./graphql').GetCapacitySpeedConfigDocument; | ||
/** | ||
* The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
*/ | ||
export function graphql(source: "\n query getEternumOwnerRealmIds($accountAddress: ContractAddress!) {\n s0EternumOwnerModels(where: { address: $accountAddress }, limit: 1000) {\n edges {\n node {\n address\n entity_id\n entity {\n models {\n __typename\n ... on s0_eternum_Realm {\n realm_id\n }\n }\n }\n }\n }\n }\n }\n"): typeof import('./graphql').GetEternumOwnerRealmIdsDocument; | ||
/** | ||
* The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
*/ | ||
export function graphql(source: "\n query getEternumEntityOwner($entityOwnerIds: [u32!]!) {\n s0EternumEntityOwnerModels(where: { entity_owner_idIN: $entityOwnerIds}, limit: 200) {\n edges {\n node {\n entity_id\n entity_owner_id\n entity {\n models {\n __typename\n ... on s0_eternum_OwnedResourcesTracker {\n resource_types\n }\n ... on s0_eternum_Position {\n x\n y\n }\n ... on s0_eternum_ArrivalTime {\n arrives_at\n }\n ... on s0_eternum_Weight {\n value\n }\n }\n }\n }\n }\n }\n }\n"): typeof import('./graphql').GetEternumEntityOwnerDocument; | ||
/** | ||
* The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
*/ | ||
export function graphql(source: "\n query getAccountTokens($accountAddress: String!) {\n tokenBalances(accountAddress: $accountAddress, limit: 8000) {\n edges {\n node {\n tokenMetadata {\n __typename\n ... on ERC721__Token {\n tokenId\n metadataDescription\n imagePath\n contractAddress\n metadata\n }\n }\n }\n }\n }\n }\n"): typeof import('./graphql').GetAccountTokensDocument; | ||
/** | ||
* The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
*/ | ||
export function graphql(source: "\n query getERC721Mints {\n tokenTransfers(accountAddress: \"0x0\", limit: 8000) {\n edges {\n node {\n tokenMetadata {\n __typename\n ... on ERC721__Token {\n tokenId\n metadataDescription\n imagePath\n contractAddress\n metadata\n }\n }\n }\n }\n }\n }\n"): typeof import('./graphql').GetErc721MintsDocument; | ||
/** | ||
* The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
*/ | ||
export function graphql(source: "\n query eternumStatistics {\n s0EternumAddressNameModels {\n totalCount\n }\n s0EternumHyperstructureModels {\n totalCount\n }\n s0EternumRealmModels {\n totalCount\n }\n s0EternumFragmentMineDiscoveredModels {\n totalCount\n }\n }\n"): typeof import('./graphql').EternumStatisticsDocument; | ||
/** | ||
* The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
*/ | ||
export function graphql(source: "\n query hasGameEnded {\n s0EternumGameEndedModels {\n edges {\n node {\n winner_address\n }\n }\n }\n }\n"): typeof import('./graphql').HasGameEndedDocument; | ||
/** | ||
* The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
*/ | ||
export function graphql(source: "\n query hasPlayerRegistered($accountAddress: ContractAddress!) {\n s0EternumOwnerModels(where: { address: $accountAddress }) {\n totalCount\n }\n }\n"): typeof import('./graphql').HasPlayerRegisteredDocument; | ||
/** | ||
* The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
*/ | ||
export function graphql( | ||
source: "\n query getEternumOwnerRealmIds($accountAddress: ContractAddress!) {\n s0EternumOwnerModels(where: { address: $accountAddress }, limit: 1000) {\n edges {\n node {\n address\n entity_id\n entity {\n models {\n __typename\n ... on s0_eternum_Realm {\n realm_id\n }\n }\n }\n }\n }\n }\n }\n", | ||
): typeof import("./graphql").GetEternumOwnerRealmIdsDocument; | ||
export function graphql(source: "\n query hasPlayerClaimed($accountAddress: ContractAddress!) {\n s0EternumLeaderboardRewardClaimedModels(where: { address: $accountAddress }) {\n totalCount\n }\n }\n"): typeof import('./graphql').HasPlayerClaimedDocument; | ||
/** | ||
* The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
*/ | ||
export function graphql( | ||
source: "\n query getEternumEntityOwner($entityOwnerIds: [u32!]!) {\n s0EternumEntityOwnerModels(where: { entity_owner_idIN: $entityOwnerIds}, limit: 200) {\n edges {\n node {\n entity_id\n entity_owner_id\n entity {\n models {\n __typename\n ... on s0_eternum_OwnedResourcesTracker {\n resource_types\n }\n ... on s0_eternum_Position {\n x\n y\n }\n ... on s0_eternum_ArrivalTime {\n arrives_at\n }\n ... on s0_eternum_Weight {\n value\n }\n }\n }\n }\n }\n }\n }\n", | ||
): typeof import("./graphql").GetEternumEntityOwnerDocument; | ||
export function graphql(source: "\n query getLeaderboardEntry($accountAddress: ContractAddress!) {\n s0EternumLeaderboardEntryModels(where: { address: $accountAddress }) {\n edges {\n node {\n address\n points\n }\n }\n }\n }\n"): typeof import('./graphql').GetLeaderboardEntryDocument; | ||
/** | ||
* The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
*/ | ||
export function graphql( | ||
source: "\n query getAccountTokens($accountAddress: String!) {\n tokenBalances(accountAddress: $accountAddress, limit: 8000) {\n edges {\n node {\n tokenMetadata {\n __typename\n ... on ERC721__Token {\n tokenId\n metadataDescription\n imagePath\n contractAddress\n metadata\n }\n }\n }\n }\n }\n }\n", | ||
): typeof import("./graphql").GetAccountTokensDocument; | ||
export function graphql(source: "\n query getLeaderboard {\n s0EternumLeaderboardModels {\n edges {\n node {\n total_points\n registration_end_timestamp\n total_price_pool {\n Some\n option\n }\n distribution_started\n }\n }\n }\n }\n"): typeof import('./graphql').GetLeaderboardDocument; | ||
/** | ||
* The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
*/ | ||
export function graphql( | ||
source: '\n query getERC721Mints {\n tokenTransfers(accountAddress: "0x0", limit: 8000) {\n edges {\n node {\n tokenMetadata {\n __typename\n ... on ERC721__Token {\n tokenId\n metadataDescription\n imagePath\n contractAddress\n metadata\n }\n }\n }\n }\n }\n }\n', | ||
): typeof import("./graphql").GetErc721MintsDocument; | ||
export function graphql(source: "\n query getHyperstructureContributions($accountAddress: ContractAddress!) {\n s0EternumContributionModels(where: { player_address: $accountAddress }, limit: 1000) {\n edges {\n node {\n hyperstructure_entity_id\n amount\n }\n }\n }\n }\n"): typeof import('./graphql').GetHyperstructureContributionsDocument; | ||
/** | ||
* The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
*/ | ||
export function graphql( | ||
source: "\n query eternumStatistics {\n s0EternumAddressNameModels {\n totalCount\n }\n s0EternumHyperstructureModels {\n totalCount\n }\n s0EternumRealmModels {\n totalCount\n }\n s0EternumFragmentMineDiscoveredModels {\n totalCount\n }\n }\n", | ||
): typeof import("./graphql").EternumStatisticsDocument; | ||
export function graphql(source: "\n query getEpochs {\n s0EternumEpochModels(limit: 1000) {\n edges {\n node {\n owners {\n _0\n _1\n }\n start_timestamp\n hyperstructure_entity_id\n index\n }\n }\n }\n }\n"): typeof import('./graphql').GetEpochsDocument; | ||
/** | ||
* The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
*/ | ||
export function graphql( | ||
source: "\n query getEntityPosition($entityIds: [u32!]!) {\n s0EternumPositionModels(where: { entity_idIN: $entityIds }) {\n edges {\n node {\n x\n y\n entity_id\n entity {\n __typename\n }\n }\n }\n }\n }\n", | ||
): typeof import("./graphql").GetEntityPositionDocument; | ||
export function graphql(source: "\n query getEntityPosition($entityIds: [u32!]!) {\n s0EternumPositionModels(where: { entity_idIN: $entityIds }) {\n edges {\n node {\n x\n y\n entity_id\n entity {\n __typename\n }\n }\n }\n }\n }\n"): typeof import('./graphql').GetEntityPositionDocument; | ||
/** | ||
* The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
*/ | ||
export function graphql( | ||
source: "\n query getEntitiesResources($entityIds: [u32!]!) {\n s0EternumResourceModels(\n where: { \n entity_idIN: $entityIds\n }\n limit: 100\n ) {\n edges {\n node {\n entity_id\n resource_type\n balance\n entity {\n __typename\n }\n }\n }\n }\n }\n", | ||
): typeof import("./graphql").GetEntitiesResourcesDocument; | ||
export function graphql(source: "\n query getEntitiesResources($entityIds: [u32!]!) {\n s0EternumResourceModels(\n where: { \n entity_idIN: $entityIds\n }\n limit: 100\n ) {\n edges {\n node {\n entity_id\n resource_type\n balance\n entity {\n __typename\n }\n }\n }\n }\n }\n"): typeof import('./graphql').GetEntitiesResourcesDocument; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Inconsistent accountAddress
Type Usage Found - Action Required
Based on the verification results, there is indeed an inconsistency in the accountAddress
parameter type across different GraphQL queries:
- Queries using
String!
type:
getAccountTokens
querytokenBalances
query
- Queries using
ContractAddress!
type:
hasPlayerRegistered
hasPlayerClaimed
getLeaderboardEntry
getHyperstructureContributions
getEternumOwnerRealmIds
This inconsistency could lead to runtime errors as the same address is being passed with different types in different queries. The codebase shows active usage of these queries in various components and hooks:
landing/src/routes/mint.lazy.tsx
landing/src/routes/season-passes.lazy.tsx
landing/src/hooks/usePrizeClaim.tsx
landing/src/hooks/helpers/useEntities.tsx
The review comment's concern about type inconsistency is valid and needs to be addressed.
Recommendations:
- Standardize the
accountAddress
type across all queries to eitherContractAddress!
orString!
- Update the GraphQL schema to use a consistent type for wallet addresses
- Add type conversion utilities if mixing types is unavoidable
🔗 Analysis chain
Unified graphql
Overloads May Cause Type-Checking Ambiguities
The multiple overloads correctly map queries to document types, but returning {}
for unknown queries can mask problems if users pass an invalid source. Consider throwing an error or logging a warning when a matching document is not found to improve debugging. Also, verify references to $accountAddress
to ensure consistent usage in GraphQL schemas (some queries treat it as String!
, others as ContractAddress
).
Below is a script to locate all calls to the new queries and confirm the parameter usage:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify references to the newly added GraphQL queries and ensure
# the $accountAddress parameter usage is consistent across the codebase.
# For each newly added query, search for usage in .ts and .tsx files, retrieving context lines
rg "hasGameEnded" -A 5 --type-add "ts:*.ts" --type-add "tsx:*.tsx" --type ts --type tsx
rg "hasPlayerRegistered" -A 5 --type ts --type tsx
rg "hasPlayerClaimed" -A 5 --type ts --type tsx
rg "getLeaderboardEntry" -A 5 --type ts --type tsx
rg "getLeaderboard" -A 5 --type ts --type tsx
rg "getHyperstructureContributions" -A 5 --type ts --type tsx
rg "getEpochs" -A 5 --type ts --type tsx
Length of output: 3763
Script:
#!/bin/bash
# Let's search for all GraphQL queries that use accountAddress to verify type consistency
rg -A 2 -B 2 'accountAddress:' --type ts --type-add 'ts:*.{ts,tsx}'
# Also search for actual usages of these queries in the codebase
rg -A 2 'useQuery\(' --type ts --type-add 'ts:*.{ts,tsx}'
Length of output: 32327
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (1)
landing/src/routes/claim.lazy.tsx (1)
107-184
: Improve UI accessibility and error handlingThe render logic needs improvements in accessibility and error handling:
- Add ARIA labels and roles for better accessibility
- Add error boundary around Suspense
- Prevent layout shift during loading
+ if (error) { + return ( + <div className="flex items-center justify-center h-full"> + <p className="text-red-400" role="alert">{error}</p> + </div> + ); + } return ( <div className="flex flex-col h-full"> {loading && ( <div className="flex-grow flex items-center justify-center absolute inset-0 bg-background/50 z-50"> - <Loader2 className="w-10 h-10 animate-spin" /> + <Loader2 className="w-10 h-10 animate-spin" role="progressbar" aria-label="Loading..." /> </div> )} <div className="flex-grow overflow-y-auto p-4"> <div className="flex flex-col gap-4"> - <Suspense fallback={<div>Loading...</div>}> + <ErrorBoundary fallback={<div role="alert">Something went wrong</div>}> + <Suspense + fallback={ + <div className="min-h-[200px] flex items-center justify-center"> + <Loader2 className="w-6 h-6 animate-spin" /> + </div> + } + >Also consider:
- Adding tooltips for disabled buttons to explain why they're disabled
- Adding confirmation dialogs for irreversible actions
- Implementing proper focus management for better keyboard navigation
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
landing/src/routes/claim.lazy.tsx
(1 hunks)
🔇 Additional comments (1)
landing/src/routes/claim.lazy.tsx (1)
55-57
: LGTM!
The route definition follows the TanStack Router pattern correctly.
const onRegister = async () => { | ||
if (!account || noPoints) return; | ||
setRegisterLoading(true); | ||
await register_to_leaderboard({ | ||
signer: account, | ||
hyperstructure_contributed_to: hyperstructures, | ||
hyperstructure_shareholder_epochs: epochs, | ||
}).finally(() => setRegisterLoading(false)); | ||
}; | ||
|
||
const onClaim = async () => { | ||
if (!account) return; | ||
setClaimLoading(true); | ||
await claim_leaderboard_rewards({ signer: account, token: lordsAddress }).finally(() => setClaimLoading(false)); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix transaction handling and button state management
There are several critical issues in the transaction handling logic:
- The register button's disabled logic is commented out (line 159), which could lead to unnecessary transactions
- Missing error handling for failed transactions
- No user feedback during transaction processing
Apply these changes:
const onRegister = async () => {
if (!account || noPoints) return;
setRegisterLoading(true);
- await register_to_leaderboard({
- signer: account,
- hyperstructure_contributed_to: hyperstructures,
- hyperstructure_shareholder_epochs: epochs,
- }).finally(() => setRegisterLoading(false));
+ try {
+ await register_to_leaderboard({
+ signer: account,
+ hyperstructure_contributed_to: hyperstructures,
+ hyperstructure_shareholder_epochs: epochs,
+ });
+ // Add success toast/notification
+ } catch (error) {
+ // Add error toast/notification
+ console.error('Registration failed:', error);
+ } finally {
+ setRegisterLoading(false);
+ }
};
const onClaim = async () => {
if (!account) return;
setClaimLoading(true);
- await claim_leaderboard_rewards({ signer: account, token: lordsAddress }).finally(() => setClaimLoading(false));
+ try {
+ await claim_leaderboard_rewards({
+ signer: account,
+ token: lordsAddress
+ });
+ // Add success toast/notification
+ } catch (error) {
+ // Add error toast/notification
+ console.error('Claim failed:', error);
+ } finally {
+ setClaimLoading(false);
+ }
};
Committable suggestion skipped: line range outside the PR's diff.
function Claim() { | ||
const { | ||
setup: { | ||
systemCalls: { claim_leaderboard_rewards, register_to_leaderboard }, | ||
}, | ||
} = useDojo(); | ||
|
||
const { account, address } = useAccount(); | ||
|
||
const [registerLoading, setRegisterLoading] = useState(false); | ||
const [claimLoading, setClaimLoading] = useState(false); | ||
|
||
const { points, isLoading: isPointsLoading } = useLeaderboardEntry(address || ""); | ||
const { leaderboard, isLoading: isLeaderboardLoading } = useLeaderboardStatus(); | ||
const { winnerAddress, isLoading: isWinnerLoading } = useGameWinner(); | ||
|
||
const hasRegistered = points > 0; | ||
|
||
const registrationEnd = leaderboard?.registration_end_timestamp; | ||
const isRegistrationPeriodActive = registrationEnd && Math.floor(Date.now() / 1000) < Number(registrationEnd); | ||
|
||
const yourShare = Number(leaderboard?.total_points) | ||
? ((points / Number(leaderboard?.total_points)) * 100).toFixed(2) | ||
: "0"; | ||
|
||
const { hyperstructures, isLoading: isHsLoading } = useGetPlayerHyperstructureContributions(address || ""); | ||
const { epochs, isLoading: isEpochsLoading } = useGetEpochs(address || ""); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve error handling and loading state management
The current implementation could benefit from better error handling and loading state management:
+ const [error, setError] = useState<string | null>(null);
+
+ // Consolidate loading states into a single object
+ const loadingStates = {
+ points: isPointsLoading,
+ leaderboard: isLeaderboardLoading,
+ winner: isWinnerLoading,
+ hyperstructures: isHsLoading,
+ epochs: isEpochsLoading
+ };
+
+ // Add error handling to data fetching hooks
+ useEffect(() => {
+ if (!address) {
+ setError("Please connect your wallet");
+ return;
+ }
+ setError(null);
+ }, [address]);
Consider:
- Adding error states for failed data fetches
- Consolidating loading states into a single object for better maintainability
- Adding user feedback for connection status
Committable suggestion skipped: line range outside the PR's diff.
const RegistrationCountdown = ({ registrationEnd }: { registrationEnd: string | undefined }) => { | ||
const [timeLeft, setTimeLeft] = useState({ hours: "00", minutes: "00", seconds: "00" }); | ||
|
||
useEffect(() => { | ||
if (!registrationEnd) return; | ||
|
||
const timer = setInterval(() => { | ||
const now = Math.floor(Date.now() / 1000); | ||
const end = Number(registrationEnd); | ||
if (now >= end) { | ||
setTimeLeft({ hours: "00", minutes: "00", seconds: "00" }); | ||
clearInterval(timer); | ||
return; | ||
} | ||
|
||
const diff = end - now; | ||
const hours = Math.floor(diff / 3600); | ||
const minutes = Math.floor((diff % 3600) / 60); | ||
const seconds = diff % 60; | ||
setTimeLeft({ | ||
hours: String(hours).padStart(2, "0"), | ||
minutes: String(minutes).padStart(2, "0"), | ||
seconds: String(seconds).padStart(2, "0"), | ||
}); | ||
}, 1000); | ||
|
||
return () => clearInterval(timer); | ||
}, [registrationEnd]); | ||
|
||
return ( | ||
<div className="bg-white/5 backdrop-blur-sm rounded-xl p-8 text-center"> | ||
<h2 className="text-2xl font-bold text-primary mb-4">Registration Countdown</h2> | ||
<div className="text-3xl text-primary font-semibold"> | ||
{timeLeft.hours}:{timeLeft.minutes}:{timeLeft.seconds} | ||
</div> | ||
</div> | ||
); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance RegistrationCountdown component robustness and performance
Several improvements can be made to the countdown implementation:
-const RegistrationCountdown = ({ registrationEnd }: { registrationEnd: string | undefined }) => {
+interface RegistrationCountdownProps {
+ registrationEnd: string | undefined;
+}
+
+const RegistrationCountdown: React.FC<RegistrationCountdownProps> = ({ registrationEnd }) => {
const [timeLeft, setTimeLeft] = useState({ hours: "00", minutes: "00", seconds: "00" });
useEffect(() => {
- if (!registrationEnd) return;
+ const end = Number(registrationEnd);
+ if (!registrationEnd || isNaN(end)) return;
const timer = setInterval(() => {
const now = Math.floor(Date.now() / 1000);
- const end = Number(registrationEnd);
if (now >= end) {
setTimeLeft({ hours: "00", minutes: "00", seconds: "00" });
clearInterval(timer);
return;
}
const diff = end - now;
+ if (diff < 0) {
+ setTimeLeft({ hours: "00", minutes: "00", seconds: "00" });
+ clearInterval(timer);
+ return;
+ }
+
const hours = Math.floor(diff / 3600);
const minutes = Math.floor((diff % 3600) / 60);
const seconds = diff % 60;
setTimeLeft({
hours: String(hours).padStart(2, "0"),
minutes: String(minutes).padStart(2, "0"),
seconds: String(seconds).padStart(2, "0"),
});
- }, 1000);
+ }, 1000); // Consider increasing interval to reduce re-renders
return () => clearInterval(timer);
}, [registrationEnd]);
Improvements:
- Added proper TypeScript interface for props
- Added validation for invalid registrationEnd values
- Moved Number conversion outside interval to avoid repeated conversions
- Added handling for negative time differences
📝 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 RegistrationCountdown = ({ registrationEnd }: { registrationEnd: string | undefined }) => { | |
const [timeLeft, setTimeLeft] = useState({ hours: "00", minutes: "00", seconds: "00" }); | |
useEffect(() => { | |
if (!registrationEnd) return; | |
const timer = setInterval(() => { | |
const now = Math.floor(Date.now() / 1000); | |
const end = Number(registrationEnd); | |
if (now >= end) { | |
setTimeLeft({ hours: "00", minutes: "00", seconds: "00" }); | |
clearInterval(timer); | |
return; | |
} | |
const diff = end - now; | |
const hours = Math.floor(diff / 3600); | |
const minutes = Math.floor((diff % 3600) / 60); | |
const seconds = diff % 60; | |
setTimeLeft({ | |
hours: String(hours).padStart(2, "0"), | |
minutes: String(minutes).padStart(2, "0"), | |
seconds: String(seconds).padStart(2, "0"), | |
}); | |
}, 1000); | |
return () => clearInterval(timer); | |
}, [registrationEnd]); | |
return ( | |
<div className="bg-white/5 backdrop-blur-sm rounded-xl p-8 text-center"> | |
<h2 className="text-2xl font-bold text-primary mb-4">Registration Countdown</h2> | |
<div className="text-3xl text-primary font-semibold"> | |
{timeLeft.hours}:{timeLeft.minutes}:{timeLeft.seconds} | |
</div> | |
</div> | |
); | |
}; | |
interface RegistrationCountdownProps { | |
registrationEnd: string | undefined; | |
} | |
const RegistrationCountdown: React.FC<RegistrationCountdownProps> = ({ registrationEnd }) => { | |
const [timeLeft, setTimeLeft] = useState({ hours: "00", minutes: "00", seconds: "00" }); | |
useEffect(() => { | |
const end = Number(registrationEnd); | |
if (!registrationEnd || isNaN(end)) return; | |
const timer = setInterval(() => { | |
const now = Math.floor(Date.now() / 1000); | |
if (now >= end) { | |
setTimeLeft({ hours: "00", minutes: "00", seconds: "00" }); | |
clearInterval(timer); | |
return; | |
} | |
const diff = end - now; | |
if (diff < 0) { | |
setTimeLeft({ hours: "00", minutes: "00", seconds: "00" }); | |
clearInterval(timer); | |
return; | |
} | |
const hours = Math.floor(diff / 3600); | |
const minutes = Math.floor((diff % 3600) / 60); | |
const seconds = diff % 60; | |
setTimeLeft({ | |
hours: String(hours).padStart(2, "0"), | |
minutes: String(minutes).padStart(2, "0"), | |
seconds: String(seconds).padStart(2, "0"), | |
}); | |
}, 1000); // Consider increasing interval to reduce re-renders | |
return () => clearInterval(timer); | |
}, [registrationEnd]); | |
return ( | |
<div className="bg-white/5 backdrop-blur-sm rounded-xl p-8 text-center"> | |
<h2 className="text-2xl font-bold text-primary mb-4">Registration Countdown</h2> | |
<div className="text-3xl text-primary font-semibold"> | |
{timeLeft.hours}:{timeLeft.minutes}:{timeLeft.seconds} | |
</div> | |
</div> | |
); | |
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
landing/src/routes/claim.lazy.tsx (2)
125-126
: Fix typo in points label"Registred" is misspelled. It should be "Registered".
- <span>Total Points Registred:</span> + <span>Total Points Registered:</span>
109-113
: Improve accessibility for loading statesThe loading indicators need proper ARIA attributes for better accessibility:
- <Loader2 className="w-10 h-10 animate-spin" /> + <Loader2 className="w-10 h-10 animate-spin" aria-label="Loading content" role="status" /> - {registerLoading ? <Loader2 className="w-4 h-4 animate-spin" /> : "Register"} + {registerLoading ? <Loader2 className="w-4 h-4 animate-spin" aria-label="Registering" role="status" /> : "Register"} - {claimLoading ? <Loader2 className="w-4 h-4 animate-spin" /> : "Claim Rewards"} + {claimLoading ? <Loader2 className="w-4 h-4 animate-spin" aria-label="Claiming rewards" role="status" /> : "Claim Rewards"}Also applies to: 161-162, 169-170
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
landing/src/components/modules/app-sidebar.tsx
(2 hunks)landing/src/routes/claim.lazy.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- landing/src/components/modules/app-sidebar.tsx
🔇 Additional comments (4)
landing/src/routes/claim.lazy.tsx (4)
16-53
: Enhance countdown implementation robustness
The countdown implementation needs improvements in type safety, performance, and error handling.
Previous review comment already covers the necessary improvements:
- Adding TypeScript interface for props
- Validating registrationEnd values
- Optimizing Number conversion
- Handling negative time differences
55-57
: LGTM!
The route definition is correctly implemented using TanStack Router's createLazyFileRoute.
59-90
: Enhance error handling and user feedback
The component needs better error handling and user feedback:
Previous review comment already covers:
- Adding error states for failed data fetches
- Consolidating loading states
- Adding user feedback for connection status
91-105
: Improve transaction handling
The transaction handlers need better error handling and user feedback.
Previous review comment already covers:
- Adding try-catch blocks
- Implementing proper error handling
- Adding user feedback during transactions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
landing/src/components/modules/season-registration-timer.tsx (1)
1-3
: Consider separating the hook imports from library imports.
Currently, the custom hook (useLeaderboardStatus
) and React hooks (useEffect
,useState
) are imported together. While this is entirely functional, sometimes grouping external library imports (likereact
) and internal imports (like@/...
) can improve readability, especially in larger codebases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
landing/src/components/modules/season-registration-timer.tsx
(1 hunks)landing/src/components/modules/top-navigation-view.tsx
(2 hunks)
🔇 Additional comments (5)
landing/src/components/modules/season-registration-timer.tsx (3)
9-10
: Confirm the availability of registration_end_timestamp
.
Ensure that the leaderboard
object always provides registration_end_timestamp
or gracefully handles when it does not exist or is null
. Currently, the existence check suffices, but watch out for potential undefined or null references if the API changes in the future.
14-32
: Correct and efficient countdown logic.
The core countdown logic is well-structured and understandable. The if (now >= end)
condition reliably stops the interval, preventing unnecessary updates once the timer reaches zero. This avoids memory leaks and redundant rendering.
37-45
: Presentational clarity.
Your UI elements clearly display the time left. The light styling (with a background blur and simple text) is minimal and effective. This straightforward design fosters a solid user experience.
landing/src/components/modules/top-navigation-view.tsx (2)
13-13
: Import statement is consistent with the file structure.
Importing SeasonRegistrationTimer
inline here follows the existing pattern within this file. This looks good.
67-67
: Great addition of the SeasonRegistrationTimer
component.
Inserting the timer alongside the existing SeasonStartTimer
adds clarity to the registration phase in parallel with the season start countdown. This integration appears consistent with the design, and no props are required.
Summary by CodeRabbit
Release Notes
New Features
SeasonRegistrationTimer
component to display the countdown until registration closes.Bug Fixes
Chores