feat : shows events info from supabase on event index page #123
Conversation
WalkthroughThe changes in this pull request include the removal of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CreateEventScreen
participant Supabase
User->>CreateEventScreen: Fill out event details
CreateEventScreen->>CreateEventScreen: Update inputs state
CreateEventScreen->>Supabase: Call addEvent with event details
Supabase-->>CreateEventScreen: Response
CreateEventScreen-->>User: Event created or error
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (10)
src/components/icons/ClockIcon.tsx (1)
6-6: Consider using a single export patternThe component has both named and default exports. While this works, it's generally better to stick to a single export pattern for consistency.
Choose either named or default export:
export const ClockIcon: React.FC<IconProps> = ({ fill, stroke, size }) => ( // ... component implementation ); -export default ClockIcon;Or:
-export const ClockIcon: React.FC<IconProps> = ({ fill, stroke, size }) => ( +const ClockIcon: React.FC<IconProps> = ({ fill, stroke, size }) => ( // ... component implementation ); export default ClockIcon;Also applies to: 12-12
src/components/icons/PinIcon.tsx (2)
6-10: LGTM! Clean and focused implementation.The component is well-structured and follows React best practices. The SVG implementation is correct and the use of
BasicIconpromotes consistency.Consider adding JSDoc comments to document the props:
+/** + * Pin location icon component + * @param {string} fill - The fill color of the icon + * @param {string} stroke - The stroke color of the icon + * @param {number} size - The size of the icon in pixels + */ export const PinIcon: React.FC<IconProps> = ({ fill, stroke, size }) => (
6-6: Consider using a single export pattern.The component has both named and default exports which could lead to inconsistent import patterns across the codebase.
Choose one export pattern:
-export const PinIcon: React.FC<IconProps> = ({ fill, stroke, size }) => ( +const PinIcon: React.FC<IconProps> = ({ fill, stroke, size }) => ( // ... implementation ... ); export default PinIcon;Or:
export const PinIcon: React.FC<IconProps> = ({ fill, stroke, size }) => ( // ... implementation ... ); -export default PinIcon;Also applies to: 12-12
src/routes/events/index.tsx (2)
37-37: Remove commented debug codeRemove the commented console.log statement as it's not needed in production code.
- // console.log(events[0].start_time)
90-105: Enhance event card presentation and null handlingSeveral improvements can be made to the event card:
- The placeholder image div should be replaced with actual event images
- Add null checks for event.name and event.location
- Show appropriate placeholder text when data is missing
- <div className="h-32 bg-gray-500" /> + {event.image_url ? ( + <img src={event.image_url} alt={event.name || 'Event'} className="h-32 w-full object-cover" /> + ) : ( + <div className="h-32 bg-gray-500 flex items-center justify-center text-gray-400">No Image</div> + )} <div className="p-2"> - <h3 className="text-lg mb-1">{event.name}</h3> + <h3 className="text-lg mb-1">{event.name || 'Untitled Event'}</h3> <p className="text-sm flex items-center"> <ClockIcon fill="currentColor" stroke="#ffffff" size={24} /> - {startTime.toLocaleString('zh-TW', { + {startTime ? startTime.toLocaleString('zh-TW', { month: '2-digit', day: '2-digit', hour: '2-digit', minute: '2-digit', - })} + }) : 'Time not set'} </p> <p className="text-sm flex items-center"> <PinIcon fill="currentColor" stroke="#ffffff" size={24} /> - {event.location} + {event.location || 'Location not specified'} </p>src/routes/events/create.tsx (4)
38-42: Consider adding TypeScript interface for form inputsFor better type safety and maintainability, consider defining an interface for the form inputs.
interface EventFormInputs { name: string; start_time: string; end_time: string; location: string; fee: number; description: string; }
Line range hint
65-82: Add form validation and error handlingThe form submission has several potential issues:
- No validation of required fields before submission
- No client-side validation of date ranges (end_time should be after start_time)
- No error feedback to users when submission fails
Consider adding:
async function addEvent(e: FormEvent) { e.preventDefault() // Validate required fields if (!inputs.name || !inputs.start_time || !inputs.end_time || !inputs.location) { alert('Please fill in all required fields'); return; } // Validate date range if (new Date(inputs.end_time) <= new Date(inputs.start_time)) { alert('End time must be after start time'); return; } try { const { data, error } = await supabase .from('events') .insert({ ...inputs, user_id: (await UserController.get()).id }) .select('*') .single() if (error) throw error; navigate({ to: '/events/$eventId', params: { 'eventId': data.id.toString() } }) } catch (error) { alert('Failed to create event: ' + error.message); } }
145-150: Consider using textarea for descriptionFor better user experience, consider using a textarea for the description field to allow for longer, multi-line content.
-<input +<textarea style={{...styles.input, height: '100px'}} className="rounded" placeholder="請介紹你的活動" + maxLength={500} value={inputs.description} onChange={(text) => { setInputs({ ...inputs, description: text.target.value }) }} -/> +></textarea>
Image upload functionality is missing in event creation
The verification confirms that the event creation implementation (
addEventfunction) only handles basic form data insertion into the 'events' table and lacks image upload functionality. There is no Supabase storage implementation or file upload handling in the codebase.
- The
addEventfunction insrc/routes/events/create.tsxneeds to be extended to handle image upload to Supabase storage before inserting the event data- Image URL from storage should be included in the event data
🔗 Analysis chain
Line range hint
156-164: Verify image upload implementationThe component includes image upload UI but the implementation for storing the image in Supabase storage appears to be missing.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for Supabase storage implementation rg -l "storage.from|createSignedUrl|uploadFile" --type ts # Check if there are any existing image upload implementations ast-grep --pattern 'supabase.storage.from($$$).upload($$$)'Length of output: 121
Script:
#!/bin/bash # Search for any Supabase storage related imports or usage rg "storage" --type ts -A 3 # Search for any file upload related functions rg "upload" --type ts -A 3 # Check for image handling functions ast-grep --pattern 'function $_(file: File) { $$$ }' # Look for event creation implementation rg "createEvent|addEvent" --type ts -A 5Length of output: 937
src/utils/database.types.ts (1)
59-59: Consider documenting location field constraintsSince this field will be used across multiple components (event creation, display, etc.), consider adding JSDoc comments to document any constraints or formatting expectations for the location field. This will help maintain consistency across the application.
For example:
- Is there an expected format for the location (e.g., address, coordinates)?
- Are there any length limitations?
- Should it support multi-line text?
Also applies to: 71-71, 83-83
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
.env.example(0 hunks)src/components/icons/ClockIcon.tsx(1 hunks)src/components/icons/PinIcon.tsx(1 hunks)src/components/index.ts(1 hunks)src/routes/events/create.tsx(3 hunks)src/routes/events/index.tsx(3 hunks)src/utils/database.types.ts(3 hunks)supabase/migrations/20241114141253_remote_schema.sql(1 hunks)
💤 Files with no reviewable changes (1)
- .env.example
✅ Files skipped from review due to trivial changes (1)
- supabase/migrations/20241114141253_remote_schema.sql
🔇 Additional comments (6)
src/components/icons/ClockIcon.tsx (2)
2-4: LGTM: Clean and well-structured imports
The imports are properly organized, using named imports for React and components. The IconProps interface import shows good TypeScript practices.
1-12: Verify icon usage across the application
Since this is a new icon component that's being added to support the event index page, let's verify its integration.
✅ Verification successful
Icon integration verified successfully
The ClockIcon is properly integrated and used in the events index page (src/routes/events/index.tsx). It's correctly imported along with other icons and used with the expected props (fill, stroke, size) to display event timing information.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check where the ClockIcon is being used and ensure it's properly imported
# Check for ClockIcon imports
echo "Checking ClockIcon imports:"
rg -l "import.*ClockIcon" --type ts --type tsx
# Check for ClockIcon usage in JSX
echo -e "\nChecking ClockIcon usage in components:"
rg "<ClockIcon" --type ts --type tsx -A 2
Length of output: 308
Script:
#!/bin/bash
# Let's try with a different approach using ripgrep without file type filters
# and also search for both default and named imports
echo "Checking ClockIcon imports and usage:"
rg "import.*ClockIcon|<ClockIcon" -A 2
# Also search for potential file references
echo -e "\nChecking for ClockIcon file references:"
rg "ClockIcon" --type ts --type javascript --type jsx --type typescript
Length of output: 824
src/components/index.ts (1)
10-12: LGTM! Well-organized exports
The new icon exports are properly placed in the Icons section and maintain alphabetical ordering, following the established pattern in the file.
src/routes/events/index.tsx (1)
5-16: LGTM! Well-structured Event interface
The Event interface properly defines all necessary fields with appropriate types, including the newly added location field.
src/utils/database.types.ts (2)
71-71: LGTM! Consistent type definitions across Row, Insert, and Update types
The location field is correctly defined as an optional nullable string (string | null) in both Insert and Update types, maintaining consistency with the Row type definition.
Also applies to: 83-83
59-59: Verify database schema alignment for the new location field
The addition of the location field to the events table type definition looks correct. Let's verify that this matches the actual database schema.
✅ Verification successful
Database schema and type definitions are correctly aligned
The migration file confirms that the location column is added to the events table as a nullable text field, which perfectly matches the TypeScript type definition (string | null). The schema changes are consistent with the type definitions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the database schema matches the type definition
# Look for the corresponding migration file that adds the location column
fd -e sql -x grep -l "location" {}
Length of output: 91
Script:
#!/bin/bash
# Let's check the content of the migration file to verify the location column definition
cat ./supabase/migrations/20241114141253_remote_schema.sql
Length of output: 120
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
src/routes/events/index.tsx (3)
2-2: Consider consolidating icon importsThe
Clockicon is imported directly from flowbite-react-icons while other icons are imported from the components barrel file. Consider moving the Clock icon to your components for consistency.-import { Clock } from "flowbite-react-icons/solid";Then add it to your components barrel file.
38-38: Remove debug codeRemove commented out console.log statements before merging.
- // console.log(events[0].start_time)
108-114: Improve accessibility and internationalizationSeveral improvements needed:
- Add aria-labels to icons for screen readers
- Consider extracting hardcoded Chinese strings to a translation file
- Make locale configurable instead of hardcoding 'zh-TW'
- <Clock fill="currentColor" stroke="#ffffff" size={24} /> + <Clock fill="currentColor" stroke="#ffffff" size={24} aria-label="時間" /> - <h3 className="font-bold text-2xl">通知</h3> + <h3 className="font-bold text-2xl">{t('notification')}</h3> - toLocaleString('zh-TW', { + toLocaleString(userLocale, {Also applies to: 75-76
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
src/routes/events/index.tsx (3)
1-3: Consider consolidating icon importsThe
Clockicon is imported from 'flowbite-react-icons/solid' while other icons are imported from '../../components'. Consider moving all icon imports to be consistent with the project's component structure.-import { Clock } from "flowbite-react-icons/solid"; +import { BellIcon, Clock, Header, PinIcon, PlusIcon } from '../../components'; -import { BellIcon, Header, PinIcon, PlusIcon } from '../../components';
100-122: Consider using a date formatting libraryFor more robust date handling and consistent formatting across the application, consider using a date formatting library like
date-fnsorluxon.Example with date-fns:
import { format } from 'date-fns'; import { zhTW } from 'date-fns/locale'; // In EventCard: const formattedDate = startTime ? format(startTime, 'MM/dd HH:mm', { locale: zhTW }) : '時間未提供';
41-62: Consider adding error boundaries and paginationFor production readiness, consider:
- Adding error boundaries to gracefully handle rendering failures
- Implementing pagination or infinite scroll for the events list to handle large datasets
- Adding loading states while events are being fetched
This will improve the user experience and application reliability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/routes/events/index.tsx(4 hunks)
🔇 Additional comments (2)
src/routes/events/index.tsx (2)
44-61: Fix duplicate event display in different sections
Both "最新揪人" and "最新活動" sections are displaying the exact same events without any filtering or differentiation.
67-70: Replace direct DOM manipulation with React refs
Using document.getElementById is an anti-pattern in React.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/routes/events/index.tsx (2)
6-17: Consider adding type constraints for better data validation.The interface could benefit from more specific type constraints:
typefield could be an enum or union type to restrict valid valuesfeecould have a minimum value constraintstart_timeandend_timecould ensure end_time is after start_timeExample implementation:
type EventType = 1 | 2; // Replace with actual event types interface Event { created_at: string; description: string | null; end_time: string | null; fee: number | null; id: number; name: string | null; start_time: string | null; type: EventType | null; user_id: string; location: string | null; // Runtime validation could be added via class methods or validation functions }
103-103: Make card width responsive for better mobile experience.The hardcoded width (
w-40) might not be optimal for all screen sizes.Consider using responsive classes:
- <div className="flex-shrink-0 w-40 bg-gray-700 rounded-lg overflow-hidden text-white"> + <div className="flex-shrink-0 w-40 sm:w-48 md:w-56 bg-gray-700 rounded-lg overflow-hidden text-white">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/routes/events/index.tsx(4 hunks)
🔇 Additional comments (3)
src/routes/events/index.tsx (3)
45-61:
Events are duplicated across sections without filtering.
The same events are displayed in both "最新揪人" and "最新活動" sections without any differentiation.
Consider filtering events based on their type:
- {events.map((event) => (
+ {events.filter(event => event.type === 1).map((event) => (
<EventCard key={event.id} event={event} />
))}
// ... later in the code ...
- {events.map((event) => (
+ {events.filter(event => event.type === 2).map((event) => (
<EventCard key={event.id} event={event} />
))}67-70:
Replace direct DOM manipulation with React refs.
Using document.getElementById is an anti-pattern in React.
Use useRef hook instead:
+ const modalRef = useRef<HTMLDialogElement>(null);
- if (document) {
- (document.getElementById('my_modal_4') as HTMLFormElement).showModal();
- }
+ modalRef.current?.showModal();
// Update the dialog element:
- <dialog id="my_modal_4" className="modal">
+ <dialog ref={modalRef} className="modal">101-101:
Fix potential null reference errors in date handling.
The code has two issues with date handling:
- Using
new Date()as fallback might be misleading - No null check before calling
toLocaleString
Apply these fixes:
- const startTime = event.start_time ? new Date(event.start_time) : new Date();
+ const startTime = event.start_time ? new Date(event.start_time) : null;
- {startTime.toLocaleString('zh-TW', {
+ {startTime?.toLocaleString('zh-TW', {
month: '2-digit',
day: '2-digit',
hour: '2-digit',
minute: '2-digit',
- })}
+ }) || '時間未提供'}Also applies to: 109-115
Summary by CodeRabbit
Release Notes
New Features
ClockIconandPinIconcomponents for enhanced visual representation.EventCardcomponent and Tailwind CSS styling.Bug Fixes
.env.examplefile to streamline environment variable setup.Database Updates
locationproperty to the events model for better event detail management.locationadded to the events table in the database.