-
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
Fix laggy social search #2460
Fix laggy social search #2460
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThe changes in the Changes
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 16 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
🧹 Outside diff range and nitpick comments (3)
client/src/ui/components/worldmap/players/PlayersPanel.tsx (3)
76-88
: Consider these performance and safety improvementsWhile the filtering logic is correct, there are a few optimizations that could be made:
- Cache the lowercase search term to avoid multiple transformations
- Use optional chaining for safer structure filtering
const filteredPlayers = useMemo(() => { const term = searchTerm.toLowerCase(); return searchTerm === "" ? playersWithStructures : playersWithStructures.filter((player) => { const nameMatch = player.name.toLowerCase().includes(term); if (nameMatch) return true; const addressMatch = toHexString(player.address).toLowerCase().includes(term); if (addressMatch) return true; - return player.structures.some((structure) => structure && structure.toLowerCase().includes(term)); + return player.structures.some((structure) => structure?.toLowerCase()?.includes(term) ?? false); }); }, [playersWithStructures, searchTerm]);🧰 Tools
🪛 Biome (1.9.4)
[error] 86-86: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
108-116
: Consider adding these UX improvementsThe search handlers work well but could benefit from some common UX patterns:
- Trim whitespace from search input
- Add minimum length validation (e.g., 2 characters)
- Add debouncing to prevent rapid-fire searches
+ import { debounce } from 'lodash'; // Add to imports const handleSearch = () => { + const trimmedValue = inputValue.trim(); + if (trimmedValue.length < 2) return; setSearchTerm(inputValue); }; + const debouncedSearch = debounce(handleSearch, 300); const handleKeyDown = (e: KeyboardEvent<HTMLInputElement>) => { if (e.key === "Enter") { - handleSearch(); + debouncedSearch(); } };
120-129
: Add accessibility improvements to search UIConsider enhancing the search interface with these accessibility features:
- Add appropriate ARIA labels
- Show loading state during search
<div className="flex gap-2 mb-4"> <TextInput placeholder="Search players/realms/structures..." + aria-label="Search players, realms, and structures" onChange={(value) => setInputValue(value)} onKeyDown={handleKeyDown} className="flex-1" /> - <Button onClick={handleSearch} variant="primary"> + <Button + onClick={handleSearch} + variant="primary" + aria-label="Perform search" + disabled={isLoading} + > - Search + {isLoading ? 'Searching...' : 'Search'} </Button> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
client/src/ui/components/worldmap/players/PlayersPanel.tsx
(5 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
client/src/ui/components/worldmap/players/PlayersPanel.tsx
[error] 86-86: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (1)
client/src/ui/components/worldmap/players/PlayersPanel.tsx (1)
32-33
: Great performance optimization!
Separating the input state (inputValue
) from the search state (searchTerm
) is an excellent way to reduce unnecessary re-renders. This change addresses the laggy search issue by only triggering filtered results when the search is actually performed, rather than on every keystroke.
Summary by CodeRabbit
New Features
Bug Fixes
Style