feat: add venue cfp banners #3976
Conversation
thulieblack
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
derberg and
sambhavgupta0705
as code owners
WalkthroughThis pull request refines the banner cycling logic in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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
Documentation and Community
|
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3976 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 21 21
Lines 667 667
Branches 113 113
=========================================
Hits 667 667 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/campaigns/banners.ts (1)
7-7: Fix typo in comment.There's a typo in the comment: "G et" should be "Get".
- const currentDate = new Date(); // G et the current date + const currentDate = new Date(); // Get the current date
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/campaigns/AnnouncementHero.tsx(4 hunks)components/campaigns/banners.ts(1 hunks)
🔇 Additional comments (6)
components/campaigns/banners.ts (1)
27-63: Great job adding the new conference venue banners!The new banners for various AsyncAPI Conference editions are well-structured and consistent.
I notice that the Munich Edition uses a different link pattern (Typeform URL) compared to the other venues (conference.asyncapi.com URL). Is this intentional?
components/campaigns/AnnouncementHero.tsx (5)
40-40: Improved banner cycling logic.Using the modulo operation ensures the banner index properly wraps around when it reaches the end of the visible banners.
45-45: Good dependency array update.Changing the dependency array from
[activeIndex]to[numberOfVisibleBanners]is more appropriate since the interval should only be reset when the number of banners changes, not on every index change.
66-68: Simplified banner visibility logic.The new approach to only render the active banner rather than conditionally hiding inactive banners should improve performance.
81-81: Consistent use of visibility variable.Using the
isVisiblevariable for theactiveBannerprop improves code readability and maintains consistency with the conditional rendering logic.
93-93: Simplified active index comparison.Direct comparison of
activeIndex === indexis cleaner than the previous implementation.
|
@derberg wdyt? |
|
@thulieblack 👏🏼 👏🏼 👏🏼 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
components/campaigns/banners.ts (3)
7-7: Minor typo in comment.There's a space in the middle of the word "Get" in the comment on line 7.
- const currentDate = new Date(); // G et the current date + const currentDate = new Date(); // Get the current date
6-16: Consider updating the shouldShowBanner function.The current function only checks if the current date is after the deadline, but there may be cases where you want to start showing banners only after a specific start date too (not too early).
Consider enhancing the function to support a start date parameter:
-export function shouldShowBanner(cfpDeadline: string) { +export function shouldShowBanner(cfpDeadline: string, startDate?: string) { const currentDate = new Date(); // Get the current date const deadline = new Date(cfpDeadline); // Convert the cfpDeadline string to a Date object + + // If a start date is provided, check if the current date is before the start date + if (startDate && currentDate < new Date(startDate)) { + return false; + } // Check if the current date is after the deadline if (currentDate > deadline) { return false; } return true; }
18-81: Consider adding banner validation.With multiple banners and dates, it would be beneficial to add some validation to ensure all required fields are present and formatted correctly.
Consider adding a TypeScript interface for the banner structure and validation helpers:
interface Banner { title: string; city: string; dateLocation: string; cfaText: string; eventName: string; cfpDeadline: string; link: string; } // Helper function to validate a banner function validateBanner(banner: Banner): boolean { // Check if all required fields are present const requiredFields = ['title', 'city', 'dateLocation', 'cfaText', 'eventName', 'cfpDeadline', 'link']; for (const field of requiredFields) { if (!banner[field as keyof Banner]) { console.warn(`Missing required field "${field}" in banner for ${banner.city}`); return false; } } // Validate date format try { new Date(banner.cfpDeadline); } catch (e) { console.warn(`Invalid date format for cfpDeadline in banner for ${banner.city}`); return false; } return true; } // Use this to validate all banners during development export function validateAllBanners(): boolean { return banners.every(validateBanner); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/campaigns/banners.ts(2 hunks)
🔇 Additional comments (2)
components/campaigns/banners.ts (2)
19-27: Singapore banner has different wording and link pattern.The Singapore edition banner has different wording compared to other banners:
- Uses "Get Free Tickets" instead of "Apply To Speak"
- Uses "you get a Free Ticket" instead of "the end of Call for Speakers"
- Uses a coupon-based URL instead of the conference.asyncapi.com domain pattern
This appears intentional, as it offers free tickets rather than a call for speakers.
28-36: Well-structured banners with consistent formatting.The banners for Munich, Lagos, Bangalore, London, Paris, and Online editions follow a consistent structure with appropriate dates and CFP deadlines. All CFP deadlines are set before their respective event dates which is correct.
For the newly added venues (Lagos, Bangalore, London, Paris, Online), the links follow a consistent pattern of
https://conference.asyncapi.com/venue/{CityName}.Also applies to: 37-45, 46-54, 55-63, 64-72, 73-81
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
components/campaigns/banners.ts (1)
18-81: 💡 Verification agent🧩 Analysis chain
Verify that the banner URLs are correct and accessible.
The newly added venue links follow a different pattern than the existing banners. Ensure that all URLs (especially the venue-specific ones like
https://conference.asyncapi.com/venue/Lagos) resolve correctly and lead to the appropriate registration pages.
🏁 Script executed:
#!/bin/bash # Extract and check all banner URLs echo "Extracting all banner URLs from components/campaigns/banners.ts..." grep -o "link: '[^']*'" components/campaigns/banners.ts | sed "s/link: '//" | sed "s/'$//" > /tmp/banner_urls.txt echo "Checking URL accessibility (note: this doesn't guarantee the pages exist, just that the domains resolve)..." while read url; do domain=$(echo "$url" | sed -E 's|https?://([^/]+).*|\1|') echo "Checking domain: $domain for URL: $url" if host "$domain" > /dev/null 2>&1; then echo "✅ Domain resolves: $domain" else echo "❌ Cannot resolve domain: $domain" fi done < /tmp/banner_urls.txtLength of output: 4096
Action Required: Banner URL Domains Unreachable
The DNS resolution checks indicate that the domains used in the banner URLs are currently not resolving. Specifically:
- ticket.apidays.global (Singapore Edition)
- apidays.typeform.com (Munich Edition)
- conference.asyncapi.com (used for Lagos, Bangalore, London, Paris, and Online Editions)
Please verify that these URLs are correct and that the corresponding DNS records are properly configured. Ensure that they lead to the intended registration pages or update them as necessary.
🧹 Nitpick comments (1)
components/campaigns/banners.ts (1)
18-81: Consider sorting banners chronologically by event date.The banners are not ordered by event date. For example, London (September) appears after Bangalore (October), and Online (October) appears after Paris (December). Sorting them chronologically would make the list more intuitive to maintain.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/campaigns/banners.ts(3 hunks)
🔇 Additional comments (4)
components/campaigns/banners.ts (4)
7-7: Grammar fix makes the comment clearer.The comment now properly explains the purpose of the variable with improved spacing.
37-45: Added new venue banners for multiple locations.The new banners follow a consistent format for Lagos, Bangalore, London, Paris, and Online editions, with appropriate dates and CFP deadlines. Each has the same URL pattern (
https://conference.asyncapi.com/venue/[City]).Also applies to: 46-54, 55-63, 64-72, 73-80
19-27:Details
✅ Verification successful
Singapore edition uses different CTA compared to other events.
While other events use "Apply To Speak" as the call-to-action text, this one uses "Get Free Tickets" and has a different
eventNamevalue. This appears to be intentional since the link points to a ticketing page with a coupon code, but verify this is the expected behavior.
🏁 Script executed:
#!/bin/bash # Check for consistency in CTA text usage across banners echo "Analyzing CTA text and eventName consistency across banners..." grep -A 5 "cfaText:" components/campaigns/banners.tsLength of output: 1605
Confirmed: Special Behavior for Singapore Edition is Intended
After verifying the consistency across the banners in
components/campaigns/banners.ts, we observed that the Singapore banner uniquely usescfaText: 'Get Free Tickets'andeventName: 'you get a Free Ticket', while all other events use "Apply To Speak" with the event name "the end of Call for Speakers". Given that the Singapore edition links to a ticketing page with a coupon code, this deviation in CTA and event name is intentional.No changes are required.
18-81:Details
✅ Verification successful
Ensure displayed banners will transition correctly with the updated list.
Verify that the banner cycling logic in the
AnnouncementHerocomponent can handle this expanded list of banners correctly, as mentioned in the AI summary.
🏁 Script executed:
#!/bin/bash # Look for the banner cycling logic in the AnnouncementHero component echo "Searching for the banner cycling logic in the AnnouncementHero component..." find . -name "AnnouncementHero.tsx" -exec cat {} \; | grep -B 5 -A 5 "activeIndex" || echo "AnnouncementHero.tsx not found in current directory"Length of output: 1683
Banner Cycling Logic Confirmed
The banner cycling logic in the
AnnouncementHerocomponent was verified to correctly iterate through the expanded list of banners using theactiveIndexstate and conditional rendering. The component properly maps over the filteredvisibleBannersand renders the dot indicators that update the active banner viagoToIndex.
- Verified: The mapping of
visibleBannersand usage ofactiveIndexfor both banner display and pagination is implemented as expected.- Next Step: While the code-level check confirms the logic handles an expanded banner list, please ensure to test the UI to visually confirm that transitions and carousel animations behave correctly with the updated list.
|
/rtm |
6 participants
Add more CFP venue to the banner and modify the Announcement Hero code
Summary by CodeRabbit
New Features
Bug Fixes
currentDatevariable in the banner display logic.