feat : add search bar #130
Conversation
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
src/routes/events/index.tsx (3)
24-29: Consider implementing server-side pagination and searchThe current implementation fetches all events at once, which could lead to performance issues as the dataset grows. Consider:
- Implementing pagination to limit the initial data load
- Moving the search functionality to the server-side to reduce client-side data processing
Example implementation:
const ITEMS_PER_PAGE = 20; loader: async ({ search = '', page = 1 }) => { const { data, error } = await supabase .from('events') .select('*') .ilike('name', `%${search}%`) .range((page - 1) * ITEMS_PER_PAGE, page * ITEMS_PER_PAGE - 1); if (error) throw error; return { events: data }; }
Line range hint
79-90: Avoid duplicate event renderingThe same filtered events are being rendered twice in different sections ("最新揪人" and "最新活動"). Consider:
- Differentiating the events for each section
- Using separate filtered arrays based on event type or category
const recentInvites = filteredEvents.filter(event => event.type === 'invite'); const recentEvents = filteredEvents.filter(event => event.type === 'event');
134-136: Improve null date handlingUsing current date as fallback for null start_time might be misleading. Consider showing a placeholder or "TBD" instead.
- const startTime = event.start_time - ? new Date(event.start_time) - : new Date(); + const startTime = event.start_time + ? new Date(event.start_time) + : null; // In the render section: {startTime ? startTime.toLocaleString('zh-TW', { month: '2-digit', day: '2-digit', hour: '2-digit', minute: '2-digit', }) : '時間未定'}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/routes/events/index.tsx(5 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/routes/events/index.tsx
[error] 43-44: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (2)
src/routes/events/index.tsx (2)
2-3: LGTM! Clean imports and well-structured interface
The new imports support the search functionality, and the Event interface is properly typed with all necessary fields.
Also applies to: 9-18
51-73: LGTM! Well-implemented search bar UI
The search bar implementation includes good UX features:
- Clear button when text is present
- Placeholder text handling based on focus state
- Proper positioning of search icon and placeholder
| const filteredEvents = events.filter( | ||
| (event) => | ||
| event.name && | ||
| event.name.toLowerCase().includes(searchTerm.toLowerCase()) | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve event filtering logic
The current filtering logic has potential issues:
- Doesn't handle null event names properly
- Could use optional chaining as suggested by static analysis
- const filteredEvents = events.filter(
- (event) =>
- event.name &&
- event.name.toLowerCase().includes(searchTerm.toLowerCase())
- );
+ const filteredEvents = events.filter((event) =>
+ event.name?.toLowerCase().includes(searchTerm.toLowerCase()) ?? false
+ );📝 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 filteredEvents = events.filter( | |
| (event) => | |
| event.name && | |
| event.name.toLowerCase().includes(searchTerm.toLowerCase()) | |
| ); | |
| const filteredEvents = events.filter((event) => | |
| event.name?.toLowerCase().includes(searchTerm.toLowerCase()) ?? false | |
| ); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 43-44: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Summary by CodeRabbit
New Features
Bug Fixes
Documentation