detailPage #128
detailPage #128
Conversation
WalkthroughThe changes in this pull request primarily involve modifications to two files: Changes
Possibly related PRs
Suggested reviewers
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: 5
🧹 Outside diff range and nitpick comments (1)
src/routes/events/$eventId.tsx (1)
94-95: Format event start and end times for better readabilityDisplaying raw timestamps may not be user-friendly. Consider formatting
event.start_timeandevent.end_timeto a more readable date and time format.Apply this diff to format the event times:
<p>{event.start_time}~{event.end_time}</p> + <p>{new Date(event.start_time).toLocaleString()} ~ {new Date(event.end_time).toLocaleString()}</p>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/routeTree.gen.ts(2 hunks)src/routes/events/$eventId.tsx(1 hunks)
🔇 Additional comments (1)
src/routeTree.gen.ts (1)
7-9: Acknowledged auto-generated file with updates
The file is auto-generated, and the added id properties for routes enhance route identification. No action needed.
src/routes/events/$eventId.tsx
Outdated
| <p>學生學習護照</p> | ||
| <p>實數類型 {event.fee} 小時</p> | ||
| </div> |
There was a problem hiding this comment.
Verify the correct usage of event.fee
The event.fee property is used to represent both 'hours' and 'participant limit', which may lead to confusion. Ensure that event.fee is correctly used or consider using separate properties for clarity.
Also applies to: 148-150
src/routes/events/$eventId.tsx
Outdated
| <li>{event.user_id}</li> | ||
| </ol> |
There was a problem hiding this comment.
Avoid displaying event.user_id directly to users
Displaying event.user_id may not be meaningful to users and could expose sensitive information. Consider showing the organizer's name or a more user-friendly identifier.
src/routes/events/$eventId.tsx
Outdated
| <div className='flex'> | ||
| <svg className="w-6 h-6 text-gray-800 dark:text-white" aria-hidden="true" xmlns="http://www.w3.org/2000/svg" width="24" height="24" fill="none" viewBox="0 0 24 24"> | ||
| <path stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M8 17.345a4.76 4.76 0 0 0 2.558 1.618c2.274.589 4.512-.446 4.999-2.31.487-1.866-1.273-3.9-3.546-4.49-2.273-.59-4.034-2.623-3.547-4.488.486-1.865 2.724-2.899 4.998-2.31.982.236 1.87.793 2.538 1.592m-3.879 12.171V21m0-18v2.2"/> | ||
| </svg> | ||
| <p>活動價格</p> | ||
| </div> |
There was a problem hiding this comment.
Display the event price
The event price label is shown without the actual price. Include the price from event.price to inform users about the cost.
Apply this diff to include the event price:
{/* 活動價格 */}
<div className='flex'>
<svg className="w-6 h-6 text-gray-800 dark:text-white" /* SVG content */ />
- <p>活動價格</p>
+ <p>活動價格：{event.price}</p>
</div>Committable suggestion skipped: line range outside the PR's diff.
src/routes/events/$eventId.tsx
Outdated
| <div id="item1" className="carousel-item w-full"> | ||
| <img src="https://i.pinimg.com/originals/68/21/7d/68217da82cbeb3cc833ed2acdea7c738.gif" className="w-full" /> | ||
| </div> | ||
| <div id="item2" className="carousel-item w-full"> | ||
| <img src="https://i.pinimg.com/originals/79/de/27/79de27bafeb77f608ef74b3d02e727ba.gif"className="w-full" /> | ||
| </div> | ||
| <div id="item3" className="carousel-item w-full"> | ||
| <img src="https://i.pinimg.com/originals/df/f2/75/dff275ddc4aea5548575fd78d218965d.gif" className="w-full" /> | ||
| </div> | ||
| <div id="item4" className="carousel-item w-full"> | ||
| <img src="https://i.pinimg.com/originals/fc/b4/ee/fcb4ee6e201ec8f0de461a9f1ac65d2e.gif" className="w-full" /> | ||
| </div> | ||
| </div> | ||
| {/* 換頁點點-要根據使用者提供的張數更改個數 */} | ||
| <div className="absolute bottom-4 flex w-full justify-center gap-2 py-2"> | ||
| <a href="#item1" className="btn btn-xs">1</a> | ||
| <a href="#item2" className="btn btn-xs">2</a> | ||
| <a href="#item3" className="btn btn-xs">3</a> | ||
| <a href="#item4" className="btn btn-xs">4</a> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make the carousel dynamic based on event images
Currently, the carousel images and navigation buttons are hardcoded with static image URLs and a fixed number of items. To improve scalability and maintainability, consider fetching the images from the event data and dynamically generating the carousel items and navigation buttons based on the actual number of images.
Apply this diff to make the carousel dynamic:
- <div className="carousel w-full">
- <div id="item1" className="carousel-item w-full">
- <img src="https://i.pinimg.com/originals/68/21/7d/68217da82cbeb3cc833ed2acdea7c738.gif" className="w-full" />
- </div>
- <!-- Additional hardcoded items -->
- </div>
+ <div className="carousel w-full">
+ {event.images.map((imageUrl, index) => (
+ <div key={index} id={`item${index + 1}`} className="carousel-item w-full">
+ <img src={imageUrl} className="w-full" />
+ </div>
+ ))}
+ </div>
- <div className="absolute bottom-4 flex w-full justify-center gap-2 py-2">
- <a href="#item1" className="btn btn-xs">1</a>
- <!-- Additional hardcoded navigation buttons -->
- </div>
+ <div className="absolute bottom-4 flex w-full justify-center gap-2 py-2">
+ {event.images.map((_, index) => (
+ <a key={index} href={`#item${index + 1}`} className="btn btn-xs">
+ {index + 1}
+ </a>
+ ))}
+ </div>Committable suggestion skipped: line range outside the PR's diff.
src/routes/events/$eventId.tsx
Outdated
| <div className='flex'> | ||
| <svg className="w-6 h-6 text-gray-800 dark:text-white" aria-hidden="true" xmlns="http://www.w3.org/2000/svg" width="24" height="24" fill="none" viewBox="0 0 24 24"> | ||
| <path stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 13a3 3 0 1 0 0-6 3 3 0 0 0 0 6Z"/> | ||
| <path stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M17.8 13.938h-.011a7 7 0 1 0-11.464.144h-.016l.14.171c.1.127.2.251.3.371L12 21l5.13-6.248c.194-.209.374-.429.54-.659l.13-.155Z"/> | ||
| </svg> | ||
| </div> |
There was a problem hiding this comment.
Display the event location
The event location is currently not shown. Including the location helps users know where the event is held.
Apply this diff to display the event location:
{/* 活動地點 */}
<div className='flex'>
<svg className="w-6 h-6 text-gray-800 dark:text-white" /* SVG content */ />
+ <p>{event.location}</p>
</div>Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
src/routes/events/$eventId.tsx (1)
23-40: Consider using Tailwind classes instead of inline stylesThe inline styles object could be replaced with Tailwind utility classes for better maintainability and consistency with the rest of the codebase.
-const styles = { - body:{ - backgroundColor:'#3E3E3E' - }, - container:{ - flex: 1, - backgroundColor: '#FFFFFF' - }, - card:{ - borderRadius: "15px", - backgroundColor:'#FFFFFF', - margin: "1rem 1.5rem", - padding: "1rem" - }, - icon:{ - margin:"0.5rem" - }, -}Replace usages with Tailwind classes:
style={styles.body}→className="bg-[#3E3E3E]"style={styles.container}→className="flex-1 bg-white"style={styles.card}→className="rounded-2xl bg-white m-6 p-4"style={styles.icon}→className="m-2"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/routes/events/$eventId.tsx(2 hunks)
🔇 Additional comments (5)
src/routes/events/$eventId.tsx (5)
52-76: Make the carousel dynamic based on event images
The carousel currently uses hardcoded images and navigation buttons. This should be made dynamic based on the event's images.
#!/bin/bash
# Check if events table has an images column
rg -A 5 "images.*from.*events" 118-118: Display the event price
The price section only shows the label without the actual price.
238-238: Verify the correct usage of event.fee and user information
- The
event.feeproperty is used for both hours and participant limit - The
event.user_idis displayed directly to users
Also applies to: 251-252
270-275: Work in progress items align with PR objectives
The TODO comments match the PR objectives regarding:
- Supabase timezone display
- Carousel display buttons
- Favorite function
- Header display
111-111:
Add missing location display
The location section is empty but has the icon prepared.
+<p>{event.location}</p>Likely invalid or redundant comment.
| <button className="btn absolute bottom-1 right-1" | ||
| onClick={() => { | ||
| if (join) { | ||
| (document.getElementById("cancel_modal") as HTMLFormElement).showModal(); | ||
| } else { | ||
| (document.getElementById("join_modal") as HTMLFormElement).showModal(); | ||
| } | ||
| }}> | ||
| {join ? "已報名" : "報名活動"} | ||
| </button> | ||
|
|
||
| <dialog id="join_modal" className="modal"> | ||
| <div className="modal-box w-11/12 max-w-5xl"> | ||
| <h3 className="text-xl flex items-center justify-center"> | ||
| <svg className="w-6 h-6 text-gray-800 dark:text-white" aria-hidden="true" xmlns="http://www.w3.org/2000/svg" width="24" height="24" fill="none" viewBox="0 0 24 24"> | ||
| <path stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 5.365V3m0 2.365a5.338 5.338 0 0 1 5.133 5.368v1.8c0 2.386 1.867 2.982 1.867 4.175 0 .593 0 1.292-.538 1.292H5.538C5 18 5 17.301 5 16.708c0-1.193 1.867-1.789 1.867-4.175v-1.8A5.338 5.338 0 0 1 12 5.365ZM8.733 18c.094.852.306 1.54.944 2.112a3.48 3.48 0 0 0 4.646 0c.638-.572 1.236-1.26 1.33-2.112h-6.92Z"/> | ||
| </svg> | ||
| </h3> | ||
| <p className="py-4 text-xl flex items-center justify-center">確定要報名此活動嗎？</p> | ||
| <div className="modal-action justify-between"> | ||
| <button className="btn w-1/2" | ||
| onClick={()=>{ | ||
| setJoin(true); | ||
| if(document){ | ||
| (document.getElementById('joinSuccess_modal') as HTMLFormElement).showModal(); | ||
| (document.getElementById("join_modal") as HTMLFormElement).close(); | ||
| }}} | ||
| >好</button> | ||
| <dialog id="joinSuccess_modal" className="modal"> | ||
| <div className="modal-box w-11/12 max-w-5xl"> | ||
| <div className='flex item-center justify-center'> | ||
| <svg className="w-6 h-6 text-gray-800 dark:text-white" aria-hidden="true" xmlns="http://www.w3.org/2000/svg" width="24" height="24" fill="none" viewBox="0 0 24 24"> | ||
| <path stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 5.365V3m0 2.365a5.338 5.338 0 0 1 5.133 5.368v1.8c0 2.386 1.867 2.982 1.867 4.175 0 .593 0 1.292-.538 1.292H5.538C5 18 5 17.301 5 16.708c0-1.193 1.867-1.789 1.867-4.175v-1.8A5.338 5.338 0 0 1 12 5.365ZM8.733 18c.094.852.306 1.54.944 2.112a3.48 3.48 0 0 0 4.646 0c.638-.572 1.236-1.26 1.33-2.112h-6.92Z"/> | ||
| </svg> | ||
| <h3 className="font-bold text-lg">通知</h3> | ||
| </div> | ||
| <p className="py-4 flex item-center justify-center">活動報名成功！</p> | ||
| <div className="modal-action"> | ||
| <form method="dialog" className='w-full'> | ||
| <button className="btn w-full">好</button> | ||
| </form> | ||
| </div> | ||
| </div> | ||
| </dialog> | ||
| <form method="dialog" className='w-1/2'> | ||
| <button className="btn w-full">取消</button> | ||
| </form> | ||
| </div> | ||
| </div> | ||
| </dialog> | ||
| <dialog id="cancel_modal" className="modal"> | ||
| <div className="modal-box w-11/12 max-w-5xl"> | ||
| <div> | ||
| <svg className="w-6 h-6 text-gray-800 dark:text-white" aria-hidden="true" xmlns="http://www.w3.org/2000/svg" width="24" height="24" fill="none" viewBox="0 0 24 24"> | ||
| <path stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 5.365V3m0 2.365a5.338 5.338 0 0 1 5.133 5.368v1.8c0 2.386 1.867 2.982 1.867 4.175 0 .593 0 1.292-.538 1.292H5.538C5 18 5 17.301 5 16.708c0-1.193 1.867-1.789 1.867-4.175v-1.8A5.338 5.338 0 0 1 12 5.365ZM8.733 18c.094.852.306 1.54.944 2.112a3.48 3.48 0 0 0 4.646 0c.638-.572 1.236-1.26 1.33-2.112h-6.92Z"/> | ||
| </svg> | ||
| <h3 className="text-xl flex items-center justify-center">確定要取消報名此活動嗎？</h3> | ||
| </div> | ||
| <div className="modal-action justify-between"> | ||
| <button | ||
| className="btn w-1/2" | ||
| onClick={() => { | ||
| setJoin(false); | ||
| (document.getElementById("cancel_modal") as HTMLFormElement).close(); | ||
| (document.getElementById("cancelSuccess_modal") as HTMLFormElement).showModal(); | ||
| }} | ||
| > | ||
| 好 | ||
| </button> | ||
| <dialog id="cancelSuccess_modal" className="modal"> | ||
| <div className="modal-box w-11/12 max-w-5xl"> | ||
| <div className='flex item-center justify-center'> | ||
| <svg className="w-6 h-6 text-gray-800 dark:text-white" aria-hidden="true" xmlns="http://www.w3.org/2000/svg" width="24" height="24" fill="none" viewBox="0 0 24 24"> | ||
| <path stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 5.365V3m0 2.365a5.338 5.338 0 0 1 5.133 5.368v1.8c0 2.386 1.867 2.982 1.867 4.175 0 .593 0 1.292-.538 1.292H5.538C5 18 5 17.301 5 16.708c0-1.193 1.867-1.789 1.867-4.175v-1.8A5.338 5.338 0 0 1 12 5.365ZM8.733 18c.094.852.306 1.54.944 2.112a3.48 3.48 0 0 0 4.646 0c.638-.572 1.236-1.26 1.33-2.112h-6.92Z"/> | ||
| </svg> | ||
| <h3 className="font-bold text-lg">通知</h3> | ||
| </div> | ||
| <p className="py-4 flex item-center justify-center">活動取消報名成功！</p> | ||
| <div className="modal-action"> | ||
| <form method="dialog" className='w-full'> | ||
| <button className="btn w-full">好</button> | ||
| </form> | ||
| </div> | ||
| </div> | ||
| </dialog> | ||
| <form method="dialog" className="w-1/2"> | ||
| <button className="btn w-full">取消</button> | ||
| </form> | ||
| </div> | ||
| </div> | ||
| </dialog> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve modal implementation and error handling
The current modal implementation has several areas for improvement:
- TypeScript assertions could be avoided with proper typing
- Similar modal structures could be extracted into a reusable component
- Missing error handling for registration actions
Consider creating a reusable modal component:
type ModalProps = {
id: string;
title: string;
message: string;
onConfirm: () => Promise<void>;
onCancel: () => void;
};
const ConfirmationModal: React.FC<ModalProps> = ({
id,
title,
message,
onConfirm,
onCancel,
}) => {
const [error, setError] = useState<string | null>(null);
const handleConfirm = async () => {
try {
await onConfirm();
} catch (err) {
setError(err.message);
}
};
return (
<dialog id={id} className="modal">
{/* ... existing modal structure ... */}
</dialog>
);
};| <svg style={styles.icon} className="w-6 h-6 text-gray-800 dark:text-white" aria-hidden="true" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24"> | ||
| <path stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 8v4l3 3m6-3a9 9 0 1 1-18 0 9 9 0 0 1 18 0Z"/> | ||
| </svg> | ||
| <p>{event.start_time}~{event.end_time}</p> |
There was a problem hiding this comment.
Handle timezone display in event times
The direct display of start_time and end_time might show incorrect timezone information as mentioned in the PR objectives.
Consider using a date formatting library like date-fns or luxon to properly format the times:
-<p>{event.start_time}~{event.end_time}</p>
+<p>{format(parseISO(event.start_time), 'PPp')} ~ {format(parseISO(event.end_time), 'PPp')}</p>Committable suggestion skipped: line range outside the PR's diff.
In progress:
Summary by CodeRabbit
New Features
Documentation