inform attendees about schedule update when a speaker withdrawls #475
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe changes introduce a status field to schedule items to mark talks as "cancelled" or "replacement", update the UI component to display visual indicators, and add filtering logic across the codebase to exclude cancelled talks from various processing pipelines. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/Schedule/CardConference.astro(3 hunks)src/content/events/2025-malaysia-kuala-lumpur/index.mdx(1 hunks)src/schemas/events.ts(1 hunks)src/styles/globals.css(1 hunks)tailwind.config.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/components/Schedule/CardConference.astro (1)
src/pages/events/[id].html.md/index.ts (3)
item(84-90)event(66-93)item(76-77)
src/schemas/events.ts (3)
src/schemas/forKidsEvent.ts (1)
SchemaContext(5-19)src/schemas/news.ts (1)
SchemaContext(4-13)src/lib/events.ts (1)
EventDetails(22-27)
🔇 Additional comments (5)
src/content/events/2025-malaysia-kuala-lumpur/index.mdx (1)
38-43: LGTM! Clear demonstration of the cancellation/replacement feature.The schedule correctly marks the original talk as cancelled and introduces a replacement at the same time slot, effectively demonstrating the new status-driven UI functionality.
tailwind.config.mjs (1)
20-20: LGTM! Consistent with existing color token pattern.The
successcolor token follows the same HSL variable pattern as other semantic colors in the theme.src/schemas/events.ts (1)
150-152: LGTM! Well-structured schema addition.The
statusfield is properly typed with sensible enum values and includes a default value of"published"to ensure backward compatibility with existing schedule items.src/components/Schedule/CardConference.astro (2)
66-75: Good implementation of conditional title styling.The use of the
cnutility to conditionally applyline-throughstyling based on cancellation status provides a clear visual indicator to users.
77-92: Effective visual treatment for cancelled talks.Applying
grayscaleto the speakers grid when a talk is cancelled provides appropriate visual feedback while maintaining information visibility.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/events.ts (1)
53-81: Consider filtering cancelled talks when building thetalksarray.The current implementation includes cancelled talks in
event.data._computed.talks(lines 53-64) but then excludes their speakers when buildingevent.data._computed.speakers(lines 72-76). This creates an inconsistency where cancelled talks exist in the computed data but are filtered out at the display layer.For better clarity and efficiency:
- Filter cancelled talks once when building the
talksarray- Remove the runtime guard in speaker resolution (lines 72-76)
- This aligns with the filtering approach in other files and avoids processing cancelled talks multiple times
Apply this refactor:
const talks = ( event.data.schedule ? await Promise.all( (event.data.schedule?.items ?? []).map(async (item) => { - if (!item.slug) { + if (!item.slug || item.status === "cancelled") { return; } return await getEntry("talks", item.slug.id); }), ) : [] ).filter((i) => !!i); const speakers = ( await Promise.all( talks.flatMap( (talk) => talk.data.speakers?.map(async (speaker) => { if (!speaker) return; - const scheduleTalk = event.data.schedule?.items?.find( - (item) => item.slug?.id === talk.id, - ); - const isSpeakerTalkCanceled = scheduleTalk?.status === "cancelled"; - if (isSpeakerTalkCanceled) return; return await getEntry("people", speaker.id.id); }) ?? [], ), ) ).filter((i) => !!i);
♻️ Duplicate comments (2)
src/components/Schedule/CardConference.astro (2)
32-35: Remove debug console.log and unnecessaryawait.Line 35 contains a debug log that should not be committed to production. Line 32 uses
awaitonArray.find(), which returns synchronously and doesn't requireawait.Apply this diff:
-const scheduleItem = await (event.data.schedule?.items ?? []).find( +const scheduleItem = (event.data.schedule?.items ?? []).find( (conference) => conference.slug?.id === talk?.id, ); -console.log("lena", scheduleItem);
57-64: Clean up redundant and incorrect CSS classes.Line 58 has several issues:
from-accentis used without a gradient utility (should bebg-gradient-to-rto have effect, or removed)text-accent-foreground text-whitedeclares text color twice- Lines 60-61 repeat
text-whitewhen it's already inherited from the parent containerApply this diff:
{scheduleItem?.status === "replacement" && ( - <div class="h-full w-fit justify-center rounded-full bg-success from-accent px-2 py-1 text-accent-foreground text-white"> + <div class="h-full w-fit justify-center rounded-full bg-success px-2 py-1 text-white"> <div class="flex items-center gap-2"> - <FiPlus className="h-4 w-4 text-white" /> - <span class="text-sm font-medium text-white">New Talk</span> + <FiPlus className="h-4 w-4" /> + <span class="text-sm font-medium">New Talk</span> </div> </div> )}
🧹 Nitpick comments (1)
src/pages/events/[id].html.md/index.ts (1)
66-99: Consider refactoring to reduce filtering duplication.The
displaySchedulefunction filters cancelled talks twice—once for thetalksarray (lines 68-73) and once foritemsWithTime(lines 78-79). Both filters checkscheduleTalk?.status !== "cancelled"on the same schedule items, which duplicates logic and makes maintenance harder.Consider extracting the non-cancelled schedule items once at the beginning of the function:
const displaySchedule = async (event: EventWithComputed) => { if (isEmpty(event.data._computed.talks)) return ""; + + const nonCancelledScheduleItems = event.data.schedule?.items?.filter( + (item) => item.status !== "cancelled" + ) ?? []; + - const talks = event.data._computed.talks.filter((talk) => { - const scheduleTalk = event.data.schedule?.items?.find( - (item) => item.slug?.id === talk.id, - ); - return scheduleTalk?.status !== "cancelled"; - }); + const talks = event.data._computed.talks.filter((talk) => + nonCancelledScheduleItems.some((item) => item.slug?.id === talk.id) + ); + - const itemsWithTime = - event.data.schedule?.items?.filter( - (item) => - (item.type === "conference" || item.type === "roundtable") && - item.startTime && - item.status !== "cancelled", - ) ?? []; + const itemsWithTime = nonCancelledScheduleItems.filter( + (item) => + (item.type === "conference" || item.type === "roundtable") && + item.startTime + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/Schedule/CardConference.astro(3 hunks)src/lib/events.ts(1 hunks)src/pages/events/[id].html.md/index.ts(2 hunks)src/pages/events/[id]/assets/_utils.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/events.ts (2)
src/pages/events/[id]/talks/[talkId]/assets/_utils.ts (2)
speaker(12-27)talkId(4-30)src/schemas/talks.ts (1)
z(5-28)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/components/Schedule/CardConference.astro (1)
63-70: Clean up redundant and incorrect CSS classes.The "New" badge contains several CSS issues previously flagged:
from-accentis used without a gradient utility (should bebg-gradient-to-ror removed)text-accent-foreground text-whitedeclares text color twice- Lines 66-67 repeat
text-whitewhen already inherited from parentjustify-centerhas no effect (parent div is not a flex container; only the child div on line 65 is)Apply this diff to clean up the classes:
- <div class="isolate h-full w-fit justify-center rounded-full bg-success from-accent px-2 py-1 text-accent-foreground text-white"> + <div class="h-full w-fit rounded-full bg-success px-2 py-1 text-white"> <div class="flex items-center gap-1"> - <IoSparklesSharp className="h-4 w-4 text-white" /> - <span class="text-sm font-medium text-white">New</span> + <IoSparklesSharp className="h-4 w-4" /> + <span class="text-sm font-medium">New</span> </div> </div>
🧹 Nitpick comments (1)
src/components/Schedule/CardConference.astro (1)
55-70: Consider adding screen reader text for status badges.The "Cancelled" and "New" badges are visual-only. Screen reader users won't be informed that a talk has been cancelled or is a replacement, which impacts accessibility.
Consider adding visually hidden text or aria-labels:
{scheduleItem?.status === "cancelled" && ( <div class="h-full w-fit rounded-full bg-gradient-to-r from-destructive to-destructive/80 px-2 py-1 text-destructive-foreground"> <div class="flex items-center gap-1"> <FiXCircle className="h-4 w-4" /> - <span class="text-sm font-medium">Cancelled</span> + <span class="text-sm font-medium" aria-label="This talk has been cancelled">Cancelled</span> </div> </div> )} {scheduleItem?.status === "replacement" && ( <div class="h-full w-fit rounded-full bg-success px-2 py-1 text-white"> <div class="flex items-center gap-1"> <IoSparklesSharp className="h-4 w-4" /> - <span class="text-sm font-medium">New</span> + <span class="text-sm font-medium" aria-label="This is a new replacement talk">New</span> </div> </div> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Schedule/CardConference.astro(4 hunks)
🔇 Additional comments (4)
src/components/Schedule/CardConference.astro (4)
3-3: LGTM! Icon imports support the new status badges.The sparkles icon (
IoSparklesSharp) for the "New" badge addresses the previous user feedback about the plus icon being confusing. The imports are appropriate for the status-aware UI.Also applies to: 8-9
32-34: LGTM! Past review feedback addressed.The unnecessary
awaitand debugconsole.loghave been removed as requested in the previous review.
74-88: LGTM! Conditional styling for cancelled talks.The speakers grid correctly applies grayscale styling when a talk is cancelled, providing appropriate visual feedback to users.
90-106: LGTM! Layered grayscale styling for status awareness.The conditional grayscale on the container combined with the existing grayscale on the badges creates a layered visual effect that appropriately mutes cancelled talks.
|
Nice for the icon |
houssembaltii
force-pushed
the
feat/inform-attendees-about-schedule-update
branch
from
4add3f4 to
0467e23
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/events/[id].html.md/index.ts (1)
68-100: Potential index mismatch between filtered talks and time labels.The code assumes a 1:1 correspondence between the
talksarray (lines 70-75) anditemsWithTime(lines 76-82), but they're filtered independently:
talksfiltersevent.data._computed.talksby finding corresponding schedule items with status !== "cancelled"itemsWithTimefiltersevent.data.schedule?.itemsby type, startTime presence, and status !== "cancelled"If the schedule contains items without slugs, or if the ordering differs between computed talks and schedule items,
formattedItems[index]on line 93 could reference the wrong time or go out of bounds.Consider matching each talk to its corresponding schedule item for time lookup:
const displaySchedule = async (event: EventWithComputed) => { if (isEmpty(event.data._computed.talks)) return ""; const talks = event.data._computed.talks.filter((talk) => { const scheduleTalk = event.data.schedule?.items?.find( (item) => item.slug?.id === talk.id, ); return scheduleTalk?.status !== "cancelled"; }); - const itemsWithTime = - event.data.schedule?.items?.filter( - (item) => - (item.type === "conference" || item.type === "roundtable") && - item.startTime && - item.status !== "cancelled", - ) ?? []; - - const formattedItems = itemsWithTime.map((item) => - dayjs(item.startTime).format("hh:mmA"), - ); return `## Schedule ${( await Promise.all( - talks.map(async (item, index) => { - const timeStart = formattedItems[index]; + talks.map(async (item) => { + const scheduleItem = event.data.schedule?.items?.find( + (schedItem) => schedItem.slug?.id === item.id, + ); + const timeStart = scheduleItem?.startTime + ? dayjs(scheduleItem.startTime).format("hh:mmA") + : "TBD"; return `- [${item.data.title}](${lunalink( ROUTES.events[":id"].talks[":talkId"].__path, { id: event.id, talkId: item.id }, )}): ${timeStart} by ${(await Promise.all(item.data.speakers.map(async (speaker) => (await getEntry(speaker.id)).data.name))).join(", ")}`; }), ) ).join("\n")}`; };
♻️ Duplicate comments (1)
src/components/Schedule/CardConference.astro (1)
63-70: Clean up redundant CSS classes on the "New" badge.Line 64 contains several redundant or incorrect classes that were flagged in a previous review but not fully addressed:
from-accenthas no effect without a gradient utility likebg-gradient-to-rtext-accent-foreground text-whitedeclares text color twiceisolateis unclear in purpose here- Lines 66-67 repeat
text-whitewhen it should inherit from the parentApply this diff to clean up the classes:
{scheduleItem?.status === "replacement" && ( - <div class="isolate h-full w-fit justify-center rounded-full bg-success from-accent px-2 py-1 text-accent-foreground text-white"> + <div class="h-full w-fit justify-center rounded-full bg-success px-2 py-1 text-white"> <div class="flex items-center gap-1"> - <FiPlus className="h-4 w-4 text-white" /> - <span class="text-sm font-medium text-white">New</span> + <FiPlus className="h-4 w-4" /> + <span class="text-sm font-medium">New</span> </div> </div> )}Based on past review comments that weren't fully addressed.
🧹 Nitpick comments (1)
src/lib/events.ts (1)
72-78: Consider optimizing the schedule item lookup for better performance.The current implementation searches
event.data.schedule?.itemsfor each speaker, resulting in O(n×m) complexity. For events with many talks and speakers, this could impact performance.Consider building a lookup map before the speaker processing:
+ const scheduleItemMap = new Map( + (event.data.schedule?.items ?? []) + .filter((item) => item.slug) + .map((item) => [item.slug!.id, item]) + ); + const speakers = ( await Promise.all( talks.flatMap( (talk) => talk.data.speakers?.map(async (speaker) => { if (!speaker) return; - const scheduleTalk = event.data.schedule?.items?.find( - (item) => item.slug?.id === talk.id, - ); - const isSpeakerTalkCanceled = scheduleTalk?.status === "cancelled"; + const scheduleTalk = scheduleItemMap.get(talk.id); + const isSpeakerTalkCanceled = scheduleTalk?.status === "cancelled"; if (isSpeakerTalkCanceled) { return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/components/Schedule/CardConference.astro(4 hunks)src/content/events/2025-malaysia-kuala-lumpur/index.mdx(1 hunks)src/lib/events.ts(1 hunks)src/pages/events/[id].html.md/index.ts(2 hunks)src/pages/events/[id]/assets/_utils.ts(1 hunks)src/schemas/events.ts(1 hunks)src/styles/globals.css(1 hunks)tailwind.config.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/pages/events/[id]/assets/_utils.ts
- src/styles/globals.css
- tailwind.config.mjs
- src/content/events/2025-malaysia-kuala-lumpur/index.mdx
🧰 Additional context used
🧬 Code graph analysis (2)
src/schemas/events.ts (1)
src/schemas/news.ts (1)
SchemaContext(4-13)
src/lib/events.ts (2)
src/pages/events/[id]/talks/[talkId]/assets/_utils.ts (2)
talkId(4-30)speaker(12-27)src/pages/events/[id]/talks/[talkId]/assets/_square.tsx (1)
speaker(36-46)
🔇 Additional comments (3)
src/schemas/events.ts (1)
155-157: LGTM! Schema change is well-designed.The addition of the
statusenum field to schedule items is correctly implemented with:
- Clear semantic values ("cancelled", "replacement", "published")
- Sensible default value ("published") ensuring backward compatibility
- Proper integration with the existing schedule item structure
src/components/Schedule/CardConference.astro (2)
32-34: LGTM! Schedule item lookup is correct.The synchronous lookup correctly matches the talk to its schedule item, and the previous issue with unnecessary
awaithas been addressed.
73-106: LGTM! Conditional styling for cancelled talks is well-implemented.The grayscale effects applied to speakers and language badges when talks are cancelled provide clear visual feedback. The conditional logic using
cn()is clean and consistent.
|
New render 
|
yoannfleurydev
deleted the
feat/inform-attendees-about-schedule-update
branch
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.