-
Notifications
You must be signed in to change notification settings - Fork 49
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
use blocktimestamp hook #2553
use blocktimestamp hook #2553
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThe changes in the Changes
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 23 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: 0
🧹 Nitpick comments (1)
client/src/ui/components/entities/Entity.tsx (1)
Line range hint
53-67
: Consider simplifying the renderEntityStatus logic.The nested ternary conditions can be flattened for better readability.
const renderEntityStatus = useMemo(() => { - return nextBlockTimestamp ? ( - arrival.arrivesAt <= nextBlockTimestamp ? ( - <div className="flex ml-auto italic animate-pulse self-center bg-brown/20 rounded-md px-2 py-1"> - Waiting to offload to {getEntityName(arrival.recipientEntityId)} - </div> - ) : ( - <div className="flex ml-auto italic animate-pulse self-center bg-brown/20 rounded-md px-2 py-1"> - Arriving in {formatTime(Number(entity.arrivalTime) - nextBlockTimestamp)} to{" "} - {getEntityName(arrival.recipientEntityId)} - </div> - ) - ) : null; + if (!nextBlockTimestamp) return null; + + const statusClassName = "flex ml-auto italic animate-pulse self-center bg-brown/20 rounded-md px-2 py-1"; + + return ( + <div className={statusClassName}> + {arrival.arrivesAt <= nextBlockTimestamp + ? `Waiting to offload to ${getEntityName(arrival.recipientEntityId)}` + : `Arriving in ${formatTime(Number(entity.arrivalTime) - nextBlockTimestamp)} to ${getEntityName(arrival.recipientEntityId)}`} + </div> + ); }, [nextBlockTimestamp, arrival.recipientEntityId, arrival.hasResources, entity.arrivalTime]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
client/src/ui/components/entities/Entity.tsx
(3 hunks)
🔇 Additional comments (3)
client/src/ui/components/entities/Entity.tsx (3)
6-6
: LGTM! Import changes are clean and purposeful.
The explicit imports improve code clarity and the replacement of useUIStore with useNextBlockTimestamp aligns with the refactoring goals.
Also applies to: 13-13
Line range hint 69-82
: LGTM! Clean and efficient resource rendering logic.
The memoization is well-implemented with correct dependencies, and the filtering/mapping logic is clear and concise.
40-40
: Verify loading state handling after removing isSyncing.
While the code change simplifies the state management, please ensure that users still get appropriate feedback during loading/syncing operations.
✅ Verification successful
Loading states are appropriately handled without isSyncing
The removal of isSyncing state is acceptable because:
- The component uses
nextBlockTimestamp
for real-time updates and displays appropriate loading/waiting states through therenderEntityStatus
memo - The hook
useNextBlockTimestamp
maintains its own internal state management with automatic updates - User feedback is preserved through animated states (animate-pulse) and clear status messages showing arrival times
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any existing loading state handling in related components
rg -l "loading|syncing|pending" --type tsx --glob "client/src/ui/**/*"
# Check if useNextBlockTimestamp handles loading states internally
ast-grep --pattern 'export function useNextBlockTimestamp() {
$$$
}'
Length of output: 173
Script:
#!/bin/bash
# Search for any existing loading state handling in related components
rg -l "loading|syncing|pending" --glob "*.tsx" client/src/ui/
# Check if useNextBlockTimestamp handles loading states internally
rg -A 5 "useNextBlockTimestamp" client/src/
Length of output: 34937
Script:
#!/bin/bash
# Check the implementation of useNextBlockTimestamp hook
rg -A 10 "const useNextBlockTimestamp = \(\) => {" client/src/hooks/useNextBlockTimestamp.tsx
# Check if there are any loading states in Entity.tsx
rg "loading|syncing|pending" client/src/ui/components/entities/Entity.tsx
Length of output: 313
Script:
#!/bin/bash
# Check the implementation of useNextBlockTimestamp hook
rg -A 10 "const useNextBlockTimestamp = " client/src/hooks/useNextBlockTimestamp.tsx
# Check if there are any loading states in Entity.tsx
cat client/src/ui/components/entities/Entity.tsx
Length of output: 4959
Summary by CodeRabbit