137 create reusable organizationcard component#174
Open
DankaMarci wants to merge 3 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughA new ChangesOrganizationCard extraction and adoption
Estimated code review effort: 3 (Moderate) | ~35 minutes Sequence Diagram(s)sequenceDiagram
participant Page as Közélet Page
participant Helper as getOrganizationCardProps
participant OrganizationCard
participant Sections as Section Subcomponents
Page->>Helper: normalize dictionary org data
Helper-->>Page: OrganizationCardProps
Page->>OrganizationCard: render(props)
OrganizationCard->>Sections: render header, events, gallery, contacts, join CTA
Sections-->>OrganizationCard: rendered JSX
OrganizationCard-->>Page: composed card markup
Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 2
🧹 Nitpick comments (2)
src/lib/social-utils.tsx (1)
120-201: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract repeated label-matching conditions into shared helpers.
The web/domain, email, and X/Twitter label-matching conditions are each duplicated across
getSocialIcon,getSocialPriority, andgetSocialName. Adding a new TLD or adjusting email detection requires changes in three places. Extracting these into shared predicates eliminates the triplication and ensures consistency.♻️ Proposed refactor: shared label predicates
+function isWebLabel(l: string): boolean { + return ( + l.includes("weblap") || + l.includes("web") || + l.includes("honlap") || + l.includes("mvk.bme.hu") || + l.includes(".hu") || + l.includes(".eu") || + l.includes(".com") || + l.includes(".org") + ); +} + +function isEmailLabel(l: string): boolean { + return l.includes("@") || l.includes("email") || l.includes("e-mail"); +} + +function isXTwitterLabel(l: string): boolean { + return l === "x" || l.includes("twitter"); +} + export function getSocialIcon(label: string, className = "h-4 w-4") { const l = label.toLowerCase(); if (l.includes("facebook")) return <FacebookIcon className={className} />; if (l.includes("instagram")) return <InstagramIcon className={className} />; if (l.includes("linkedin")) return <LinkedInIcon className={className} />; if (l.includes("youtube")) return <YouTubeIcon className={className} />; if (l.includes("tiktok")) return <TiktokIcon className={className} />; - if (l === "x" || l.includes("twitter")) { + if (isXTwitterLabel(l)) { return <XSocialIcon className={className} />; } if (l.includes("linktr.ee")) return <LinktreeIcon className={className} />; if (l.includes("threads")) return <ThreadsIcon className={className} />; - if (l.includes("@") || l.includes("email") || l.includes("e-mail")) { + if (isEmailLabel(l)) { return <Mail className={className} />; } - if ( - l.includes("weblap") || - l.includes("web") || - l.includes("honlap") || - l.includes("mvk.bme.hu") || - l.includes(".hu") || - l.includes(".eu") || - l.includes(".com") || - l.includes(".org") - ) { + if (isWebLabel(l)) { return <Globe className={className} />; } return <ExternalLink className={className} />; }Apply the same predicates in
getSocialPriorityandgetSocialNameto replace their inline copies.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/social-utils.tsx` around lines 120 - 201, The label-matching logic for web/domain, email, and X/Twitter is duplicated in getSocialIcon, getSocialPriority, and getSocialName, so extract shared predicate helpers in social-utils.tsx and reuse them from all three functions. Update the matching in getSocialPriority and getSocialName to call the same helpers used by getSocialIcon so any future TLD or email rule change only needs one edit and stays consistent.src/app/(app)/[lang]/kozelet/ontevekenykorok/page.tsx (1)
9-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract duplicated
Organizationtype and helpers to a shared module.The
Organizationtype,getContactLabel, andgetOrganizationCardPropsare duplicated across all three refactored page files (ontevekenykorok,szakkollegiumok,versenycsapatok). The only meaningful difference isimageBasePathhandling — hardcoded here, parameterized elsewhere. Extracting these to a shared utility module underorganization-card/would eliminate ~50 lines of triplicated code and ensure consistent prop normalization.♻️ Proposed shared module
// src/components/common/organization-card/organization-utils.ts import type { OrganizationCardProps } from "./types"; import type { Locale } from "`@/i18n-config`"; export type Organization = { id: string; title: string; description: string[]; social_title: string; social_links: { label: string; url: string }[]; images?: string[]; stats?: OrganizationCardProps["stats"]; events?: OrganizationCardProps["events"]; activities?: OrganizationCardProps["activities"]; departments?: OrganizationCardProps["departments"]; target_audience?: OrganizationCardProps["targetAudience"]; targetAudience?: OrganizationCardProps["targetAudience"]; join_url?: string; joinUrl?: string; join_text?: string; joinText?: string; }; export function getContactLabel(label: string) { return label.replace(/:$/, ""); } export function getOrganizationCardProps( organization: Organization, locale: Locale, imageBasePath?: string, ): OrganizationCardProps { return { name: organization.title, stats: organization.stats, presentation: organization.description, events: organization.events, activities: organization.activities, departments: organization.departments, targetAudience: organization.targetAudience ?? organization.target_audience, socialLinks: organization.social_links, galleryImages: organization.images, imageBasePath, joinUrl: organization.joinUrl ?? organization.join_url, joinText: organization.joinText ?? organization.join_text, labels: { contacts: getContactLabel(organization.social_title) }, locale, }; }Then each page imports from this module instead of redefining locally.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`(app)/[lang]/kozelet/ontevekenykorok/page.tsx around lines 9 - 53, The Organization type and the helper functions getContactLabel and getOrganizationCardProps are duplicated across the refactored page components, so move them into a shared utility module under organization-card and import them from there. Keep the prop normalization logic centralized in that shared helper, and make imageBasePath an optional parameter so the different pages can pass their specific base path while reusing the same mapping logic. Update the local usages in the page component to use the shared Organization and getOrganizationCardProps symbols instead of redefining them.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/common/organization-card/sections/EventsSection.tsx`:
- Around line 51-67: The fallback key for non-linked items in EventsSection can
collide when multiple events share the same title. Update the events.map
rendering so the non-href branch uses a stable unique key that includes the
array index (or another unique field) instead of event.title alone, while
keeping the existing key behavior for linked events. Focus on the
EventsSection.tsx mapping logic around EventCard and the current key
expressions.
In `@src/components/common/organization-card/utils.tsx`:
- Around line 39-57: renderTextContent is treating every ReactNode array as an
array of strings, which breaks when content contains React elements. Update the
logic in renderTextContent to only call slice and parseFormattedText for string
items, and render non-string paragraphs directly or handle them in a separate
branch. Use the renderTextContent symbol and the paragraph mapping block to
locate the fix.
---
Nitpick comments:
In `@src/app/`(app)/[lang]/kozelet/ontevekenykorok/page.tsx:
- Around line 9-53: The Organization type and the helper functions
getContactLabel and getOrganizationCardProps are duplicated across the
refactored page components, so move them into a shared utility module under
organization-card and import them from there. Keep the prop normalization logic
centralized in that shared helper, and make imageBasePath an optional parameter
so the different pages can pass their specific base path while reusing the same
mapping logic. Update the local usages in the page component to use the shared
Organization and getOrganizationCardProps symbols instead of redefining them.
In `@src/lib/social-utils.tsx`:
- Around line 120-201: The label-matching logic for web/domain, email, and
X/Twitter is duplicated in getSocialIcon, getSocialPriority, and getSocialName,
so extract shared predicate helpers in social-utils.tsx and reuse them from all
three functions. Update the matching in getSocialPriority and getSocialName to
call the same helpers used by getSocialIcon so any future TLD or email rule
change only needs one edit and stays consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e15377b4-8877-4184-be33-763857bb9b2c
📒 Files selected for processing (16)
src/app/(app)/[lang]/kozelet/ontevekenykorok/page.tsxsrc/app/(app)/[lang]/kozelet/szakkollegiumok/page.tsxsrc/app/(app)/[lang]/kozelet/versenycsapatok/page.tsxsrc/components/common/OrganizationCard.tsxsrc/components/common/organization-card/sections/ContactsSection.tsxsrc/components/common/organization-card/sections/EventsSection.tsxsrc/components/common/organization-card/sections/GallerySection.tsxsrc/components/common/organization-card/sections/Header.tsxsrc/components/common/organization-card/sections/JoinCta.tsxsrc/components/common/organization-card/sections/ListSections.tsxsrc/components/common/organization-card/sections/Section.tsxsrc/components/common/organization-card/sections/TextSections.tsxsrc/components/common/organization-card/sections/index.tssrc/components/common/organization-card/types.tssrc/components/common/organization-card/utils.tsxsrc/lib/social-utils.tsx
Comment on lines
+51
to
+67
| {events.map((event) => | ||
| event.href ? ( | ||
| <a | ||
| key={`${event.title}-${event.href}`} | ||
| href={event.href} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="block h-full" | ||
| > | ||
| <EventCard event={event} /> | ||
| </a> | ||
| ) : ( | ||
| <div key={event.title}> | ||
| <EventCard event={event} /> | ||
| </div> | ||
| ), | ||
| )} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Non-linked event keys can collide on duplicate titles.
When event.href is absent, the key falls back to event.title alone. Two events with the same title (and no href) will produce duplicate React keys, causing reconciliation warnings and potential render bugs.
🔑 Proposed fix: include index in fallback key
event.href ? (
<a
key={`${event.title}-${event.href}`}
href={event.href}
target="_blank"
rel="noopener noreferrer"
className="block h-full"
>
<EventCard event={event} />
</a>
) : (
- <div key={event.title}>
+ <div key={`${event.title}-${index}`}>
<EventCard event={event} />
</div>
),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/common/organization-card/sections/EventsSection.tsx` around
lines 51 - 67, The fallback key for non-linked items in EventsSection can
collide when multiple events share the same title. Update the events.map
rendering so the non-href branch uses a stable unique key that includes the
array index (or another unique field) instead of event.title alone, while
keeping the existing key behavior for linked events. Focus on the
EventsSection.tsx mapping logic around EventCard and the current key
expressions.
Comment on lines
+39
to
+57
| export function renderTextContent(content: TextContent) { | ||
| if (Array.isArray(content)) { | ||
| return ( | ||
| <div className="space-y-4"> | ||
| {content.map((paragraph, index) => ( | ||
| <p key={`${paragraph.slice(0, 28)}-${index}`}> | ||
| {parseFormattedText(paragraph)} | ||
| </p> | ||
| ))} | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| if (typeof content === "string") { | ||
| return <p>{parseFormattedText(content)}</p>; | ||
| } | ||
|
|
||
| return content; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
renderTextContent crashes on ReactNode arrays.
TextContent includes ReactNode, which itself can be an array (e.g., [<span>…</span>]). Array.isArray(content) matches such arrays, then paragraph.slice(0, 28) throws because ReactElement has no .slice method. parseFormattedText(paragraph) also expects a string and would break on non-string elements.
Guard each paragraph before treating it as a string:
🐛 Proposed fix
export function renderTextContent(content: TextContent) {
- if (Array.isArray(content)) {
+ if (Array.isArray(content) && content.every((p) => typeof p === "string")) {
return (
<div className="space-y-4">
{content.map((paragraph, index) => (
<p key={`${paragraph.slice(0, 28)}-${index}`}>
{parseFormattedText(paragraph)}
</p>
))}
</div>
);
}Alternatively, if you want to support mixed arrays, handle non-string paragraphs separately:
if (Array.isArray(content)) {
return (
<div className="space-y-4">
{content.map((paragraph, index) =>
- <p key={`${paragraph.slice(0, 28)}-${index}`}>
- {parseFormattedText(paragraph)}
- </p>
+ typeof paragraph === "string" ? (
+ <p key={`${paragraph.slice(0, 28)}-${index}`}>
+ {parseFormattedText(paragraph)}
+ </p>
+ ) : (
+ <p key={index}>{paragraph}</p>
+ )
)}
</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.
Suggested change
| export function renderTextContent(content: TextContent) { | |
| if (Array.isArray(content)) { | |
| return ( | |
| <div className="space-y-4"> | |
| {content.map((paragraph, index) => ( | |
| <p key={`${paragraph.slice(0, 28)}-${index}`}> | |
| {parseFormattedText(paragraph)} | |
| </p> | |
| ))} | |
| </div> | |
| ); | |
| } | |
| if (typeof content === "string") { | |
| return <p>{parseFormattedText(content)}</p>; | |
| } | |
| return content; | |
| } | |
| export function renderTextContent(content: TextContent) { | |
| if (Array.isArray(content) && content.every((p) => typeof p === "string")) { | |
| return ( | |
| <div className="space-y-4"> | |
| {content.map((paragraph, index) => ( | |
| <p key={`${paragraph.slice(0, 28)}-${index}`}> | |
| {parseFormattedText(paragraph)} | |
| </p> | |
| ))} | |
| </div> | |
| ); | |
| } | |
| if (typeof content === "string") { | |
| return <p>{parseFormattedText(content)}</p>; | |
| } | |
| return content; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/common/organization-card/utils.tsx` around lines 39 - 57,
renderTextContent is treating every ReactNode array as an array of strings,
which breaks when content contains React elements. Update the logic in
renderTextContent to only call slice and parseFormattedText for string items,
and render non-string paragraphs directly or handle them in a separate branch.
Use the renderTextContent symbol and the paragraph mapping block to locate the
fix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes