-
Notifications
You must be signed in to change notification settings - Fork 9
feat : D-1 announcement asstets : linkedin and insta #444
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThree new asset-generation modules are introduced for event announcements: a reusable RoundedSpeakers component for displaying speaker avatars, plus two announcement generators for Instagram (1080x1350) and desktop (1920x1080) formats. Both generators fetch event data, co-organizer logos, speaker avatars, and render layered announcement compositions. Route entries for both generators are registered in the routes manifest. Changes
Sequence DiagramsequenceDiagram
participant Route as Asset Route
participant Generator as d1announcement(Config)
participant DataFetcher as Data Layer
participant Renderer as JSX Renderer
Route->>Generator: Receive route params {id}
Generator->>DataFetcher: Fetch event data by id
DataFetcher-->>Generator: Event object
Generator->>DataFetcher: Fetch cover image (base64)
Generator->>DataFetcher: Fetch co-organizer logos (base64)
Generator->>DataFetcher: Fetch speaker avatars (base64, max 3)
DataFetcher-->>Generator: All image assets
Generator->>Renderer: Render Frame with composition<br/>(BgImage, LogoWithFriends,<br/>RoundedSpeakers, event details)
Renderer-->>Route: JSX.Element (announcement image)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The additions introduce multiple new asset generator modules with data-fetching logic and UI composition. While the patterns are consistent and straightforward (fetch → compute → render), review requires validating data flow integrity, image asset handling, layout correctness across two formats, and route registration accuracy. Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
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: 2
🧹 Nitpick comments (1)
src/pages/events/[id]/assets/_d-1-announcement.tsx (1)
19-230: Consider extracting shared logic to reduce duplication.The desktop and Instagram announcement generators share substantial code for data fetching and layout structure. Consider extracting:
- A shared data-fetching helper that returns
{ event, postCover, coOrganizersLogos, speakerImages }- A base component with shared layout elements
- Platform-specific layout configurations passed as props
This would improve maintainability and ensure consistency across announcement variants.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/generated-assets/components/RoundedSpeakers.tsx(1 hunks)src/pages/events/[id]/assets/_d-1-announcement-insta.tsx(1 hunks)src/pages/events/[id]/assets/_d-1-announcement.tsx(1 hunks)src/routes.gen.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/pages/events/[id]/assets/_d-1-announcement.tsx (7)
src/generated-assets/image.ts (2)
AssetImageConfig(11-15)getAstroImageBase64(171-174)src/generated-assets/components/Frame.tsx (1)
Frame(4-35)src/generated-assets/components/BgImage.tsx (1)
BgImage(1-63)src/generated-assets/components/LogoWithFriends.tsx (1)
LogoWithFriends(6-63)src/generated-assets/theme.ts (1)
COLORS(1-6)src/generated-assets/components/RoundedSpeakers.tsx (1)
RoundedSpeakers(1-18)src/lib/events.ts (1)
getEventDisplayDate(315-328)
src/pages/events/[id]/assets/_d-1-announcement-insta.tsx (7)
src/generated-assets/image.ts (2)
AssetImageConfig(11-15)getAstroImageBase64(171-174)src/generated-assets/components/Frame.tsx (1)
Frame(4-35)src/generated-assets/components/BgImage.tsx (1)
BgImage(1-63)src/generated-assets/components/LogoWithFriends.tsx (1)
LogoWithFriends(6-63)src/generated-assets/theme.ts (1)
COLORS(1-6)src/generated-assets/components/RoundedSpeakers.tsx (1)
RoundedSpeakers(1-18)src/lib/events.ts (1)
getEventDisplayDate(315-328)
🪛 Biome (2.1.2)
src/generated-assets/components/RoundedSpeakers.tsx
[error] 5-14: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (5)
src/routes.gen.ts (1)
22-23: LGTM! Route entries align with new asset modules.The new route entries for
_d-1-announcement-instaand_d-1-announcementcorrectly correspond to the asset generation modules introduced in this PR.src/pages/events/[id]/assets/_d-1-announcement.tsx (2)
83-83: Verify the hardcoded "01" days value matches the D-1 announcement intent.The hardcoded "01" value is consistent with the filename
_d-1-announcement.tsx. If this asset is intended exclusively for one-day-before announcements, this is correct. However, if you plan to create similar assets for D-2, D-3, etc., consider parameterizing the day count to reduce duplication.
131-131: Verify the hardcoded "100+ people" text is accurate for all events.The text "Join us to meet 100+ people" is hardcoded and may not reflect the actual expected attendance for different events. Consider whether this should be:
- Parameterized based on event data
- Made configurable per event
- Kept as a generic marketing message
src/pages/events/[id]/assets/_d-1-announcement-insta.tsx (2)
86-86: Verify the hardcoded "01" days value matches the D-1 announcement intent.The hardcoded "01" value is consistent with the filename
_d-1-announcement-insta.tsx. If this asset is intended exclusively for one-day-before announcements, this is correct. However, if you plan to create similar assets for D-2, D-3, etc., consider parameterizing the day count to reduce duplication.
134-134: Verify the hardcoded "100+ people" text is accurate for all events.The text "Join us to meet 100+ people" is hardcoded and may not reflect the actual expected attendance for different events. Consider whether this should be:
- Parameterized based on event data
- Made configurable per event
- Kept as a generic marketing message
| export const RoundedSpeakers = (props: { speakerImages: string[] }) => { | ||
| return ( | ||
| <div style={{ display: "flex" }}> | ||
| {props.speakerImages.map((image, index) => ( | ||
| <img | ||
| src={image} | ||
| style={{ | ||
| borderRadius: 100, | ||
| width: 80, | ||
| height: 80, | ||
| marginLeft: index === 0 ? 0 : -25, | ||
| border: "solid 4px black", | ||
| }} | ||
| /> | ||
| ))} | ||
| </div> | ||
| ); | ||
| }; |
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.
Add key prop to mapped images.
React requires a unique key prop for each element in an iterable to efficiently track and reconcile components during re-renders.
Apply this diff to add the key prop:
- {props.speakerImages.map((image, index) => (
+ {props.speakerImages.map((image, index) => (
<img
+ key={image}
src={image}
style={{📝 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.
| export const RoundedSpeakers = (props: { speakerImages: string[] }) => { | |
| return ( | |
| <div style={{ display: "flex" }}> | |
| {props.speakerImages.map((image, index) => ( | |
| <img | |
| src={image} | |
| style={{ | |
| borderRadius: 100, | |
| width: 80, | |
| height: 80, | |
| marginLeft: index === 0 ? 0 : -25, | |
| border: "solid 4px black", | |
| }} | |
| /> | |
| ))} | |
| </div> | |
| ); | |
| }; | |
| export const RoundedSpeakers = (props: { speakerImages: string[] }) => { | |
| return ( | |
| <div style={{ display: "flex" }}> | |
| {props.speakerImages.map((image, index) => ( | |
| <img | |
| key={image} | |
| src={image} | |
| style={{ | |
| borderRadius: 100, | |
| width: 80, | |
| height: 80, | |
| marginLeft: index === 0 ? 0 : -25, | |
| border: "solid 4px black", | |
| }} | |
| /> | |
| ))} | |
| </div> | |
| ); | |
| }; |
🧰 Tools
🪛 Biome (2.1.2)
[error] 5-14: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🤖 Prompt for AI Agents
In src/generated-assets/components/RoundedSpeakers.tsx around lines 1 to 18, the
<img> elements rendered inside props.speakerImages.map lack a unique key prop;
add a stable unique key on the <img> (for example key={image ??
`speaker-${index}`} or key={`speaker-${index}`} if images may not be unique) to
satisfy React's list reconciliation, leaving other attributes unchanged.
| {event.data._computed.city?.data.name},{" "} | ||
| {event.data._computed.country?.data.name} |
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.
Add null-safety for city and country data.
The code directly accesses event.data._computed.city?.data.name and event.data._computed.country?.data.name without checking if city or country themselves are defined. If these computed fields are null or undefined, this will cause a runtime error.
Apply this diff to add defensive checks:
- {event.data._computed.city?.data.name},{" "}
- {event.data._computed.country?.data.name}
+ {event.data._computed.city?.data.name ?? "Unknown City"},{" "}
+ {event.data._computed.country?.data.name ?? "Unknown Country"}Or conditionally render the entire block:
+ {event.data._computed.city && event.data._computed.country && (
<div
style={{
display: "flex",
fontSize: 26,
fontWeight: 500,
lineHeight: 1.2,
textTransform: "uppercase",
opacity: 0.6,
}}
>
{event.data._computed.city.data.name},{" "}
{event.data._computed.country.data.name}
</div>
+ )}📝 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.
| {event.data._computed.city?.data.name},{" "} | |
| {event.data._computed.country?.data.name} | |
| {event.data._computed.city?.data.name ?? "Unknown City"},{" "} | |
| {event.data._computed.country?.data.name ?? "Unknown Country"} |
🤖 Prompt for AI Agents
In src/pages/events/[id]/assets/_d-1-announcement-insta.tsx around lines
225-226, the code directly accesses event.data._computed.city?.data.name and
event.data._computed.country?.data.name which can throw if city or country are
null/undefined; update the render to first check that event.data._computed.city
and event.data._computed.country exist (or use optional chaining for each level:
event.data._computed.city?.data?.name and
event.data._computed.country?.data?.name), provide sensible fallbacks (empty
string or "Unknown") and ensure you don’t render a stray comma when one value is
missing by conditionally joining the non-empty parts or conditionally rendering
the entire city/country block only when at least one value is present.

Closes #436


Summary by CodeRabbit
Release Notes