-
Notifications
You must be signed in to change notification settings - Fork 33
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
Website: Include site manifest files and fix on clean air forum sessions #2539
Conversation
Warning Rate limit exceeded@OchiengPaul442 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 0 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (18)
📝 WalkthroughWalkthroughThis pull request introduces several changes across the project. In the Next.js configuration, three new permanent redirect rules have been added. A new web manifest file is included, and obsolete file header comments have been removed from multiple icon components. Several page components under both the Clean Air Forum and Clean Air Network have been refactored to add metadata exports and rename default exports to “Page,” along with updated import paths. Layout and navigation components have also been updated for improved SEO metadata and consistent directory structure. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant N as Next.js Server
U->>N: Request "/clean-air"
N-->>U: Permanent redirect to "/clean-air-network/about"
sequenceDiagram
participant U as User
participant T as TabNavigation
participant L as Next.js Link
U->>T: Clicks on a tab
T->>L: Builds URL (appending ?slug=value if exists)
L-->>U: Navigates to the constructed URL
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
🧹 Nitpick comments (23)
src/website2/src/views/cleanairforum/BannerSection.tsx (1)
11-11
: Consider using a strongly typed interface instead of 'any'While the component declaration has been simplified by inlining the props type, using
any
for thedata
prop bypasses TypeScript's type checking benefits. This can lead to potential runtime errors if properties are accessed incorrectly.I suggest creating a proper interface that explicitly defines the properties used from the
data
object:interface BannerSectionData { title: string; title_subtext: string; registration_link?: string; background_image_url: string; start_date: string; end_date: string; start_time: string; end_time: string; location_name: string; } const BannerSection = ({ data }: { data: BannerSectionData }) => {This will provide better type safety, documentation, and IDE autocomplete support.
src/website2/src/views/cleanAirNetwork/TabNavigation.tsx (2)
9-14
: Consider using route constants instead of hardcoded stringsThe tab routes are defined as string literals. For better maintainability, consider extracting these paths to a constants file that can be imported and reused across the application.
-const tabs = [ - { label: 'About', value: '/clean-air-network/about' }, - { label: 'Membership', value: '/clean-air-network/membership' }, - { label: 'Events', value: '/clean-air-network/events' }, - { label: 'Resources', value: '/clean-air-network/resources' }, -]; +import { ROUTES } from '@/constants/routes'; + +const tabs = [ + { label: 'About', value: ROUTES.CLEAN_AIR_NETWORK.ABOUT }, + { label: 'Membership', value: ROUTES.CLEAN_AIR_NETWORK.MEMBERSHIP }, + { label: 'Events', value: ROUTES.CLEAN_AIR_NETWORK.EVENTS }, + { label: 'Resources', value: ROUTES.CLEAN_AIR_NETWORK.RESOURCES }, +];
19-24
: Special case for 'about' tab is intentional but could be clarifiedThe
isActiveTab
function has a special case for the 'about' tab, requiring an exact match while other tabs usestartsWith
. This is likely intentional to prevent the 'about' tab from being highlighted when visiting sub-routes of other tabs. Consider adding a comment explaining this special case.const isActiveTab = (tabValue: string) => { + // For 'about' tab, require exact match to avoid it being highlighted + // when visiting other sections that might share a prefix if (tabValue === '/clean-air-network/about') { return pathname === tabValue; } return pathname.startsWith(tabValue); };src/website2/src/views/cleanairforum/resources/ResourcesPage.tsx (2)
9-16
: Consider moving utility function to a dedicated utilities file.The
getFileNameFromUrl
function is well-implemented with proper error handling, but it might be better placed in a utilities module for reusability across the application.If this function could be used elsewhere, consider moving it to an appropriate utility file:
+// Add to a file like src/utils/fileUtils.ts: +export const getFileNameFromUrl = (url: string | null | undefined): string | null => { + if (!url || typeof url !== 'string') { + console.error('Invalid URL:', url); + return null; + } + const segments = url.split('/'); + return segments.pop() || null; +}; // Then in this file: +import { getFileNameFromUrl } from '@/utils/fileUtils';
65-67
: Add a more user-friendly message when no event is selected.Instead of returning null when no event is selected, consider providing a more user-friendly message.
if (!selectedEvent) { - return null; + return ( + <div className="text-center py-8"> + <p className="text-gray-500">No event selected. Please select an event to view resources.</p> + </div> + ); }src/website2/src/views/cleanairforum/logistics/LogisticsPage.tsx (3)
13-21
: Loading state handling could be improved.The component currently shows a loading state when
selectedEvent
is not available, but doesn't handle potential error states or provide user feedback during data loading.Consider adding an error state and improving the loading experience:
if (!selectedEvent) { - return <Loading />; + return ( + <div className="min-h-[60vh] flex items-center justify-center"> + <Loading /> + </div> + ); }
31-36
: Type 'any' should be replaced with a proper interface.Using
any
type reduces type safety and can lead to runtime errors.Consider creating an interface for the section structure:
interface ForumSection { id: string; content: string; pages: string[]; title?: string; // Add other relevant properties }
85-91
: Remove unnecessary Fragment.This Fragment wrapper is redundant as it only contains a single map operation.
- {logisticsSections && logisticsSections.length > 0 && ( - <> - {logisticsSections.map((section: any) => ( - <SectionDisplay key={section.id} section={section} /> - ))} - </> - )} + {logisticsSections && logisticsSections.length > 0 && + logisticsSections.map((section: any) => ( + <SectionDisplay key={section.id} section={section} /> + )) + }🧰 Tools
🪛 Biome (1.9.4)
[error] 86-90: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
src/website2/src/views/cleanairforum/sessions-programs/ProgramsPage.tsx (2)
94-98
: Improve type safety in filter operation.The section filtering uses any type and has a complex condition. This could be simplified and made more type-safe.
Create a type for the section and simplify the filtering logic:
interface EventSection { id: string; pages: string[]; content: string; title?: string; } // Then in the component: const sessionSections = selectedEvent.sections?.filter((section: EventSection) => section.pages.includes('session') && isValidHTMLContent(renderContent(section.content)) );
118-123
: Remove unnecessary Fragment wrappers.There are multiple unnecessary Fragment wrappers that can be simplified.
- {sessionSections && sessionSections.length > 0 && ( - <> - {sessionSections.map((section: any) => ( - <SectionDisplay key={section.id} section={section} /> - ))} - </> - )} + {sessionSections && sessionSections.length > 0 && + sessionSections.map((section: any) => ( + <SectionDisplay key={section.id} section={section} /> + )) + } - <> - {selectedEvent.programs?.map((program: any) => ( - <AccordionItem - key={program.id} - title={program.title} - subText={program.sub_text} - sessions={program.sessions} - isOpen={openAccordion === program.id} - onToggle={() => handleToggle(program.id)} - /> - ))} - </> + {selectedEvent.programs?.map((program: any) => ( + <AccordionItem + key={program.id} + title={program.title} + subText={program.sub_text} + sessions={program.sessions} + isOpen={openAccordion === program.id} + onToggle={() => handleToggle(program.id)} + /> + ))}Also applies to: 125-136
🧰 Tools
🪛 Biome (1.9.4)
[error] 118-122: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
src/website2/src/views/cleanairforum/speakers/SpeakersPage.tsx (2)
25-36
: Improve type safety and category filtering logic.The filtering logic uses string literals and lacks type safety with the
any
type.Define an interface and use constants for categories:
interface Person { id: string; category: string; name: string; bio?: string; image?: string; // Add other relevant properties } // Constants for categories const KEYNOTE_CATEGORIES = ['Key Note Speaker', 'Committee Member and Key Note Speaker']; const SPEAKER_CATEGORIES = ['Speaker', 'Speaker and Committee Member']; // Then in filtering: const keyNoteSpeakers = selectedEvent.persons?.filter( (person: Person) => KEYNOTE_CATEGORIES.includes(person.category) ) || [];
136-142
: Remove unnecessary Fragment.This Fragment wrapper is redundant and can be simplified.
- {speakersExtraSections && speakersExtraSections.length > 0 && ( - <> - {speakersExtraSections.map((section: any) => ( - <SectionDisplay key={section.id} section={section} /> - ))} - </> - )} + {speakersExtraSections && speakersExtraSections.length > 0 && + speakersExtraSections.map((section: any) => ( + <SectionDisplay key={section.id} section={section} /> + )) + }🧰 Tools
🪛 Biome (1.9.4)
[error] 137-141: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
src/website2/src/views/cleanairforum/sponsorship/SponsorshipPage.tsx (1)
52-57
: Possible unnecessary React Fragment.
Consider removing this fragment if there is only one top-level element, or if wrapping with<div>
is sufficient.51 {sponsorshipSections && sponsorshipSections.length > 0 && ( -52 <> 53 {sponsorshipSections.map((section: any) => ( 54 <SectionDisplay key={section.id} section={section} /> 55 ))} -56 </> 57 )}🧰 Tools
🪛 Biome (1.9.4)
[error] 52-56: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
src/website2/src/views/cleanairforum/partners/PartnersPage.tsx (1)
68-72
: Slight fragment optimization.
When there’s a single array of children returned, you can remove the fragment for cleaner JSX.67 {partnersSections && partnersSections.length > 0 && ( -68 <> 69 {partnersSections.map((section: any) => ( 70 <SectionDisplay key={section.id} section={section} /> 71 ))} -72 </> 73 )}🧰 Tools
🪛 Biome (1.9.4)
[error] 68-72: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
src/website2/src/app/clean-air-forum/sponsorships/page.tsx (1)
5-9
: Good use of the Next.js Metadata object.Adding page-specific metadata helps improve SEO. Consider adding Open Graph or Twitter Card tags (e.g.,
openGraph
,src/website2/src/app/clean-air-forum/program-committee/page.tsx (1)
5-9
: Metadata is well-structured.This descriptive title and summary will boost discoverability on search engines. Adding more targeted metadata (for example, relevant keywords or extended SEO properties) could further improve visibility.
src/website2/src/app/clean-air-forum/partners/page.tsx (1)
5-9
: SEO metadata is a solid addition.This makes the Partners page discoverable to both end-users and search engines. You could also enhance it with additional meta properties if desired.
src/website2/src/views/cleanairforum/program-committee/CommitteePage.tsx (1)
88-93
: Remove the redundant Fragment.
A Fragment is not needed here because you can return the mapped elements directly.Apply this diff to simplify:
-{committeeSections.length > 0 && ( - <> - {committeeSections.map((section: any) => ( - <SectionDisplay key={section.id} section={section} /> - ))} - </> -)} +{committeeSections.length > 0 && + committeeSections.map((section: any) => ( + <SectionDisplay key={section.id} section={section} /> + )) }🧰 Tools
🪛 Biome (1.9.4)
[error] 88-92: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
src/website2/src/views/cleanairforum/glossary/GlossaryPage.tsx (3)
39-43
: Replace 'any' type with a proper interface or type.Using the 'any' type reduces type safety. Consider defining a proper interface for the section object or using the existing types from your codebase.
- const glossarySections = selectedEvent.sections?.filter((section: any) => { + const glossarySections = selectedEvent.sections?.filter((section: { + id: string; + pages: string[]; + content: string; + }) => { if (!section.pages.includes('glossary')) return false; const html = renderContent(section.content); return html.trim().length > 0; });
105-109
: Simplify unnecessary Fragment.The fragment is redundant since it only contains a single child element. You can remove it to simplify the code.
{glossarySections && glossarySections.length > 0 && ( - <> {glossarySections.map((section: any) => ( <SectionDisplay key={section.id} section={section} /> ))} - </> )}🧰 Tools
🪛 Biome (1.9.4)
[error] 105-109: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
106-106
: Replace 'any' type in map function.Similar to the previous instance, replace the 'any' type with a proper interface for better type safety.
-{glossarySections.map((section: any) => ( +{glossarySections.map((section) => ( <SectionDisplay key={section.id} section={section} /> ))}src/website2/src/views/cleanairforum/TabNavigation.tsx (2)
1-1
: Update file path comment.The comment indicates a different file path than the actual location. Consider updating it to match the current file structure.
-// components/layouts/TabNavigation.tsx +// src/website2/src/views/cleanairforum/TabNavigation.tsx
19-29
: Consider extracting tabs to a configuration file.The tab list is hardcoded, which makes it less maintainable. Consider moving this configuration to a separate file or a constants file for easier management.
You could create a
constants/navigation.ts
file with:export const CLEAN_AIR_FORUM_TABS = [ { href: '/clean-air-forum/about', text: 'About' }, { href: '/clean-air-forum/program-committee', text: 'Programme Committee' }, // ... other tabs ];And then import it here:
-// Define the tabs list. -const tabs = [ - { href: '/clean-air-forum/about', text: 'About' }, - { href: '/clean-air-forum/program-committee', text: 'Programme Committee' }, - { href: '/clean-air-forum/sessions', text: 'Call for Sessions' }, - { href: '/clean-air-forum/speakers', text: 'Speakers' }, - { href: '/clean-air-forum/partners', text: 'Partners' }, - { href: '/clean-air-forum/sponsorships', text: 'Sponsorships' }, - { href: '/clean-air-forum/logistics', text: 'Travel Logistics' }, - { href: '/clean-air-forum/glossary', text: 'Glossary' }, - { href: '/clean-air-forum/resources', text: 'Resources' }, -]; +import { CLEAN_AIR_FORUM_TABS } from '@/constants/navigation'; + +// Use the tabs from the constants file +const tabs = CLEAN_AIR_FORUM_TABS;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
src/website2/public/Logo.png
is excluded by!**/*.png
src/website2/public/apple-icon.png
is excluded by!**/*.png
src/website2/public/favicon.ico
is excluded by!**/*.ico
src/website2/public/icon.png
is excluded by!**/*.png
src/website2/public/icon.svg
is excluded by!**/*.svg
src/website2/public/web-app-manifest-192x192.png
is excluded by!**/*.png
src/website2/public/web-app-manifest-512x512.png
is excluded by!**/*.png
src/website2/src/app/favicon.ico
is excluded by!**/*.ico
📒 Files selected for processing (48)
src/website2/next.config.mjs
(1 hunks)src/website2/public/assets/icons/Icon1.tsx
(1 hunks)src/website2/public/assets/icons/Icon2.tsx
(1 hunks)src/website2/public/assets/icons/Icon3.tsx
(1 hunks)src/website2/public/assets/icons/Icon4.tsx
(1 hunks)src/website2/public/assets/icons/Icon5.tsx
(1 hunks)src/website2/public/assets/icons/Icon6.tsx
(1 hunks)src/website2/public/assets/icons/Icon7.tsx
(1 hunks)src/website2/public/assets/icons/Icon8.tsx
(1 hunks)src/website2/public/manifest.json
(1 hunks)src/website2/src/app/clean-air-forum/about/page.tsx
(1 hunks)src/website2/src/app/clean-air-forum/glossary/page.tsx
(1 hunks)src/website2/src/app/clean-air-forum/layout.tsx
(1 hunks)src/website2/src/app/clean-air-forum/logistics/page.tsx
(1 hunks)src/website2/src/app/clean-air-forum/partners/page.tsx
(1 hunks)src/website2/src/app/clean-air-forum/program-committee/page.tsx
(1 hunks)src/website2/src/app/clean-air-forum/resources/page.tsx
(1 hunks)src/website2/src/app/clean-air-forum/sessions/page.tsx
(1 hunks)src/website2/src/app/clean-air-forum/speakers/page.tsx
(1 hunks)src/website2/src/app/clean-air-forum/sponsorships/page.tsx
(1 hunks)src/website2/src/app/clean-air-network/about/page.tsx
(1 hunks)src/website2/src/app/clean-air-network/events/[id]/page.tsx
(1 hunks)src/website2/src/app/clean-air-network/events/page.tsx
(1 hunks)src/website2/src/app/clean-air-network/membership/page.tsx
(1 hunks)src/website2/src/app/clean-air-network/page.tsx
(0 hunks)src/website2/src/app/clean-air-network/resources/page.tsx
(1 hunks)src/website2/src/app/layout.tsx
(3 hunks)src/website2/src/components/layouts/Footer.tsx
(3 hunks)src/website2/src/components/layouts/Navbar.tsx
(1 hunks)src/website2/src/components/layouts/NewsLetter.tsx
(1 hunks)src/website2/src/components/layouts/NotificationBanner.tsx
(2 hunks)src/website2/src/views/Forum/TabNavigation.tsx
(0 hunks)src/website2/src/views/about/AboutPage.tsx
(1 hunks)src/website2/src/views/cleanAirNetwork/TabNavigation.tsx
(1 hunks)src/website2/src/views/cleanAirNetwork/about/CleanAirPage.tsx
(1 hunks)src/website2/src/views/cleanAirNetwork/events/EventsPage.tsx
(1 hunks)src/website2/src/views/cleanAirNetwork/membership/MemberPage.tsx
(1 hunks)src/website2/src/views/cleanairforum/BannerSection.tsx
(1 hunks)src/website2/src/views/cleanairforum/TabNavigation.tsx
(1 hunks)src/website2/src/views/cleanairforum/about/AboutPage.tsx
(2 hunks)src/website2/src/views/cleanairforum/glossary/GlossaryPage.tsx
(1 hunks)src/website2/src/views/cleanairforum/logistics/LogisticsPage.tsx
(1 hunks)src/website2/src/views/cleanairforum/partners/PartnersPage.tsx
(1 hunks)src/website2/src/views/cleanairforum/program-committee/CommitteePage.tsx
(1 hunks)src/website2/src/views/cleanairforum/resources/ResourcesPage.tsx
(1 hunks)src/website2/src/views/cleanairforum/sessions-programs/ProgramsPage.tsx
(1 hunks)src/website2/src/views/cleanairforum/speakers/SpeakersPage.tsx
(1 hunks)src/website2/src/views/cleanairforum/sponsorship/SponsorshipPage.tsx
(1 hunks)
💤 Files with no reviewable changes (2)
- src/website2/src/app/clean-air-network/page.tsx
- src/website2/src/views/Forum/TabNavigation.tsx
✅ Files skipped from review due to trivial changes (17)
- src/website2/public/assets/icons/Icon6.tsx
- src/website2/public/assets/icons/Icon2.tsx
- src/website2/public/assets/icons/Icon1.tsx
- src/website2/public/assets/icons/Icon5.tsx
- src/website2/public/assets/icons/Icon3.tsx
- src/website2/public/assets/icons/Icon7.tsx
- src/website2/src/app/clean-air-network/events/[id]/page.tsx
- src/website2/public/assets/icons/Icon4.tsx
- src/website2/src/app/clean-air-forum/layout.tsx
- src/website2/public/assets/icons/Icon8.tsx
- src/website2/src/views/about/AboutPage.tsx
- src/website2/src/components/layouts/NewsLetter.tsx
- src/website2/src/components/layouts/Navbar.tsx
- src/website2/src/views/cleanAirNetwork/membership/MemberPage.tsx
- src/website2/src/views/cleanAirNetwork/events/EventsPage.tsx
- src/website2/public/manifest.json
- src/website2/src/views/cleanAirNetwork/about/CleanAirPage.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/website2/src/views/cleanairforum/speakers/SpeakersPage.tsx
[error] 81-81: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 137-141: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
src/website2/src/views/cleanairforum/program-committee/CommitteePage.tsx
[error] 78-78: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 88-92: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
src/website2/src/views/cleanairforum/partners/PartnersPage.tsx
[error] 60-60: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 68-72: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
src/website2/src/views/cleanairforum/sessions-programs/ProgramsPage.tsx
[error] 48-48: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 66-66: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 110-110: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 118-122: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
[error] 125-136: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
[error] 147-147: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
src/website2/src/views/cleanairforum/logistics/LogisticsPage.tsx
[error] 53-53: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 75-75: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 86-90: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
src/website2/src/views/cleanairforum/sponsorship/SponsorshipPage.tsx
[error] 43-43: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 52-56: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
src/website2/src/views/cleanairforum/glossary/GlossaryPage.tsx
[error] 95-95: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 105-109: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🪛 GitHub Actions: deploy-to-preview-environment
src/website2/src/app/clean-air-forum/logistics/page.tsx
[error] 1-1: Module not found: Can't resolve '@/views/cleanAirForum/logistics/LogisticsPage'
src/website2/src/app/clean-air-forum/glossary/page.tsx
[error] 1-1: Module not found: Can't resolve '@/views/cleanAirForum/glossary/GlossaryPage'
src/website2/src/app/clean-air-forum/about/page.tsx
[error] 1-1: Module not found: Can't resolve '@/views/cleanAirForum/about/AboutPage'
src/website2/src/app/clean-air-forum/partners/page.tsx
[error] 1-1: Module not found: Can't resolve '@/views/cleanAirForum/partners/PartnersPage'
🔇 Additional comments (59)
src/website2/next.config.mjs (1)
49-63
: Clean URL structure with effective redirects in place.The new redirects are well-structured and maintain a consistent pattern, ensuring users will be properly directed to the Clean Air Network about page. All redirects are appropriately marked as permanent (301), which is good for SEO as it passes link equity to the destination URL.
src/website2/src/app/clean-air-network/events/page.tsx (5)
1-1
: Good addition of Metadata import.The Metadata import from Next.js is essential for proper SEO configuration.
3-3
: Updated import path follows project convention.The import statement now follows the established project path convention.
5-9
: Strong SEO metadata with descriptive content.The metadata includes a well-structured title that follows a clear hierarchy (page name | section | site name) and a comprehensive description that explains the purpose of the events page. This will improve search engine visibility.
11-11
: Component naming follows React conventions.Changing from lowercase
page
to PascalCasePage
follows React component naming best practices.
19-19
: Default export matches component name.The default export properly reflects the renamed component.
src/website2/src/app/clean-air-network/about/page.tsx (4)
1-4
: Well-structured imports.The imports are properly organized with Next.js imports separated from component imports.
5-9
: Detailed metadata enhances SEO.The metadata contains a clear title and a comprehensive description that effectively explains the purpose of the Clean Air Network section. This will improve search engine visibility and user understanding.
11-17
: Clean component structure.The Page component has a simple and clear structure, which is consistent with other pages in the application.
19-19
: Proper default export.The default export follows the same pattern as other page components in the application.
src/website2/src/app/clean-air-network/membership/page.tsx (5)
1-1
: Necessary Metadata import added.The Metadata import from Next.js enables proper SEO configuration.
3-3
: Import path follows project convention.The import statement for MemberPage uses the established project path structure.
5-9
: Excellent SEO metadata implementation.The metadata includes a proper title hierarchy and a detailed description that clearly explains the membership benefits and purpose. This will improve search engine visibility and click-through rates.
11-11
: React naming convention followed.The component has been renamed from lowercase
page
to PascalCasePage
, which aligns with React component naming conventions.
19-19
: Export matches component name.The default export properly reflects the renamed component.
src/website2/src/app/layout.tsx (5)
42-79
: Well-structured and comprehensive keywords listThe extensive list of air quality related keywords is well-organized and comprehensive, covering all major aspects of air quality monitoring, pollutants, and environmental health. This will significantly improve SEO for relevant search queries.
94-110
: Good implementation of favicon and app iconsThe addition of various favicon formats and sizes follows best practices for cross-platform support. This ensures the site displays properly on different devices and platforms including Apple devices, Android, and Windows.
145-151
: Analytics code formatting improvedThe Google Analytics initialization code has been well-formatted with proper indentation, making it more readable while maintaining the same functionality.
115-117
:✅ Verification successful
Verify manifest.json exists and is properly configured
The manifest.json file reference is a good addition for PWA support, but we should ensure this file exists in the public directory with the correct configuration.
🏁 Script executed:
#!/bin/bash # Check if the manifest.json file exists and validate its basic structure if [ -f "src/website2/public/manifest.json" ]; then echo "manifest.json exists" cat src/website2/public/manifest.json | jq . || echo "Invalid JSON in manifest.json" else echo "manifest.json does not exist in expected location" fiLength of output: 584
Manifest Verification Confirmed
The manifest.json file exists in the expected location (
src/website2/public/manifest.json
) and its structure appears valid and correctly configured for PWA support. The configuration includes essential fields such asname
,short_name
,icons
,theme_color
,background_color
, anddisplay
, which match our requirements.
- Location confirmed:
src/website2/public/manifest.json
- Content validation: Valid JSON with proper fields
No further action is required.
124-126
:❓ Verification inconclusive
Ensure image assets exist for social sharing
The Open Graph and Twitter card images are set to use icon.png. Make sure this image exists in the public directory and is optimized for social sharing (ideally 1200x630px for Open Graph).
Also applies to: 133-133
🏁 Script executed:
#!/bin/bash # Check if the icon.png file exists if [ -f "src/website2/public/icon.png" ]; then echo "icon.png exists" # Try to determine image dimensions identify -format "Image dimensions: %wx%h" src/website2/public/icon.png 2>/dev/null || echo "Could not determine dimensions (imagemagick not available)" else echo "icon.png does not exist in expected location" fiLength of output: 283
Social Sharing Asset Verification Update
The automated check confirms that
icon.png
exists insrc/website2/public
. However, the script couldn't confirm the image dimensions due to the unavailability of ImageMagick. Please manually verify that the image is optimized for social sharing (ideally 1200x630px for Open Graph). This review applies to the meta tags in lines 124-126 and 133.src/website2/src/views/cleanAirNetwork/TabNavigation.tsx (1)
31-43
: Accessible navigation implementationThe navigation tabs implementation properly uses semantic HTML with the
nav
element and appropriate ARIA attributes. The active tab styling with both color and border provides good visual indication of the current section.src/website2/src/components/layouts/Footer.tsx (3)
4-5
: Updated Twitter branding to XGood update to import
FaXTwitter
instead ofFaTwitter
to reflect Twitter's rebranding to X.
64-64
: Updated Twitter icon to XThe Twitter icon has been appropriately updated to the new X branding.
32-34
:❓ Verification inconclusive
Improved semantic HTML with h1 tag
Changing from a paragraph to an h1 tag for the tagline improves semantic HTML structure. However, ensure this doesn't create duplicate h1 tags on the page, as there should typically be only one h1 per page.
🏁 Script executed:
#!/bin/bash # Check for multiple h1 tags in the codebase layout files echo "Checking for h1 tags in layout components..." rg -A 1 "<h1" --type=tsx src/website2/src/components/layoutsLength of output: 189
Potential Duplicate h1 Tag Verification
The use of an h1 tag in the footer improves the semantic structure, but we need to confirm that this change doesn't inadvertently introduce duplicate h1 tags on the same page. Note that the initial script failed because ripgrep didn't recognize "tsx" as a file type. Please manually verify using an updated command such as:
rg -A 1 '<h1' -g '*.tsx' src/website2/src/components/layoutsEnsure that across the rendered page there is only one h1 tag. If you find additional h1 tags, adjust the structure accordingly.
src/website2/src/app/clean-air-network/resources/page.tsx (2)
5-9
: Great addition of metadata for SEOThe addition of metadata with a descriptive title and detailed description will significantly improve SEO for this page. The description is well-written, informative, and includes relevant keywords.
11-17
: Component naming follows Next.js conventionsRenaming the component to
Page
follows Next.js conventions for page components. The simple wrapper implementation correctly delegates to the actualResourcePage
component, maintaining a clean separation of concerns.Also applies to: 19-19
src/website2/src/components/layouts/NotificationBanner.tsx (2)
13-13
: Route updated to reference the about page.The constant
CLEAN_AIR_NETWORK_ROUTE
has been updated to point to a more specific route (/clean-air-network/about
instead of/clean-air-network
). This change aligns with the updated site structure where the about page is now the landing destination.
55-55
: Comment removal doesn't affect functionality.The comment
// Prevent double event firing
has been removed while keeping the actual functionality (e.stopPropagation()
). The code continues to work as intended by preventing event bubbling.src/website2/src/app/clean-air-forum/about/page.tsx (2)
1-9
: Good addition of metadata for SEO.The addition of Metadata type and export improves the page's SEO by providing a descriptive title and detailed description for search engines. This follows Next.js best practices for metadata handling.
🧰 Tools
🪛 GitHub Actions: deploy-to-preview-environment
[error] 1-1: Module not found: Can't resolve '@/views/cleanAirForum/about/AboutPage'
11-11
: Component naming follows React conventions.Renaming the component from
page
toPage
follows React's PascalCase naming convention for components, which is a good practice.Also applies to: 19-19
src/website2/src/views/cleanairforum/about/AboutPage.tsx (2)
10-10
: Import path updated to reflect directory restructuring.The import path for SectionDisplay has been updated to use the new directory structure, ensuring proper module resolution.
30-30
: Simplified component type declaration.Removing the explicit
React.FC
type annotation simplifies the code while maintaining type safety through TypeScript's type inference.src/website2/src/app/clean-air-forum/speakers/page.tsx (3)
1-9
: Metadata implementation looks good and enhances SEO.The metadata export with title and description will improve search engine visibility and provide better context for this page in search results.
11-13
: Clean component refactor.Nicely refactored to a simpler component that delegates rendering to the dedicated
SpeakersPage
component, improving separation of concerns.
3-3
:❓ Verification inconclusive
Verify import path case consistency.
The import path uses
cleanAirForum
while the directory structure appears to usecleanairforum
in other files. Ensure the paths are consistent across the codebase to avoid potential issues on case-sensitive file systems.
🏁 Script executed:
#!/bin/bash # Check for inconsistencies in import paths related to cleanAirForum vs cleanairforum rg -i "cleanAirForum|cleanairforum" --type=tsx --type=ts | grep -i "import"Length of output: 103
Action Required: Verify Import Path Case Consistency
The original review comment raises a valid point about potential mismatches between the case used in the import statement and the actual directory names. However, the initial verification command returned an error due to unsupported file type handling (i.e., "tsx"). This means our current check wasn’t conclusive.
To proceed, please run the following revised shell script—which uses explicit file glob patterns—to verify that all import paths consistently use the correct case:
#!/bin/bash echo "Checking tsx files for 'cleanAirForum'..." rg -i 'import.*cleanAirForum' -g '*.tsx' echo "Checking tsx files for 'cleanairforum'..." rg -i 'import.*cleanairforum' -g '*.tsx' echo "Checking ts files for 'cleanAirForum'..." rg -i 'import.*cleanAirForum' -g '*.ts' echo "Checking ts files for 'cleanairforum'..." rg -i 'import.*cleanairforum' -g '*.ts'Once these commands are executed, please ensure that the import paths in your codebase reflect the correct casing (i.e., based on whether the directory is named
cleanAirForum
orcleanairforum
). This is especially important for environments with case-sensitive file systems.src/website2/src/views/cleanairforum/logistics/LogisticsPage.tsx (2)
53-56
: dangerouslySetInnerHTML usage is secured with DOMPurify.While using dangerouslySetInnerHTML is flagged, the implementation correctly sanitizes content with DOMPurify before rendering, which mitigates XSS risks.
🧰 Tools
🪛 Biome (1.9.4)
[error] 53-53: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
75-78
: Same secure pattern for HTML rendering.The HTML content is properly sanitized before being rendered to the DOM.
🧰 Tools
🪛 Biome (1.9.4)
[error] 75-75: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
src/website2/src/views/cleanairforum/sessions-programs/ProgramsPage.tsx (1)
29-36
: Good error handling in formatTime function.The function properly handles potential date formatting errors and provides a fallback, which prevents the UI from crashing.
src/website2/src/views/cleanairforum/speakers/SpeakersPage.tsx (1)
81-84
: HTML content is properly sanitized.The use of DOMPurify to sanitize HTML content before rendering is a good security practice.
🧰 Tools
🪛 Biome (1.9.4)
[error] 81-81: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
src/website2/src/app/clean-air-forum/sessions/page.tsx (4)
1-3
: Looks clean!
No issues found with these imports.
5-9
: Great use of metadata.
Providing SEO metadata in this manner is beneficial for search engine visibility.
11-13
: Straightforward component definition.
This pattern is consistent and minimal, which keeps things clean and maintainable.
15-15
: Export structure is correct.
No further concerns here.src/website2/src/views/cleanairforum/sponsorship/SponsorshipPage.tsx (1)
43-44
: Validate HTML sanitization.
While DOMPurify is used to sanitize the HTML, please verify that any external or user-generated content is properly handled. This helps prevent XSS attacks or unintended HTML injection.🧰 Tools
🪛 Biome (1.9.4)
[error] 43-43: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
src/website2/src/views/cleanairforum/partners/PartnersPage.tsx (1)
60-61
: Double-check sanitization usage.
Even thoughDOMPurify.sanitize()
is applied, confirm that this content is never derived from unsafe sources. If there’s any user input, ensure robust handling to avert security risks.🧰 Tools
🪛 Biome (1.9.4)
[error] 60-60: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
src/website2/src/app/clean-air-forum/sponsorships/page.tsx (3)
1-3
: Imports look clean and concise.Everything is correctly referencing Next.js and the external SponsorshipPage component. Please confirm that
'@/views/cleanAirForum/sponsorship/SponsorshipPage'
is the intended path and that the file structure matches the import to avoid build errors.
11-13
: Functional component is straightforward.Delegating all rendering to
SponsorshipPage
is a clear approach that adheres to the new layout structure.
15-15
: Exporting Page as default is consistent.This pattern aligns well with Next.js conventions for page components.
src/website2/src/app/clean-air-forum/program-committee/page.tsx (3)
1-3
: Imports are aligned with the new architecture.Make sure the
CommitteePage
file path ('@/views/cleanAirForum/program-committee/CommitteePage'
) is valid and unchanged, as refactoring or reorganizing files can lead to broken imports.
11-13
: Simple pass-through component is optimal.Good job separating data logic from the top-level page component. This separation enhances maintainability.
15-15
: Default export is consistent.Keeping the exported identifier as
Page
is a clean practice in Next.js 13 file-based routing.src/website2/src/app/clean-air-forum/partners/page.tsx (2)
11-13
: Minimal page component usage is neat.Delegating functionality to the
PartnersPage
maintains a consistent separation of concerns.
15-15
: Default export asPage
.This naming convention cleanly respects Next.js' recommended approach to file-based routing.
src/website2/src/views/cleanairforum/program-committee/CommitteePage.tsx (1)
78-81
: Consider verifying adequacy of DOMPurify sanitization.
AlthoughDOMPurify.sanitize
is used, the linter warns aboutdangerouslySetInnerHTML
. Make sure all user-supplied content is sanitized, and there are no regressions from potential XSS attacks.Would you like me to provide a script to confirm that each content source is trusted before using
dangerouslySetInnerHTML
?🧰 Tools
🪛 Biome (1.9.4)
[error] 78-78: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
src/website2/src/app/clean-air-forum/resources/page.tsx (1)
1-16
: Looks good overall!
The minimal structure with a metadata export and a simple Page component is clear and maintainable.src/website2/src/app/clean-air-forum/glossary/page.tsx (1)
3-3
:⚠️ Potential issue✅ Verification successful
Fix the import path to resolve pipeline failures.
The pipeline error indicates that'@/views/cleanAirForum/glossary/GlossaryPage'
cannot be found. Adjust the import path to match the actual folder name (most likely'cleanairforum'
).Here is a suggested fix:
-import GlossaryPage from '@/views/cleanAirForum/glossary/GlossaryPage'; +import GlossaryPage from '@/views/cleanairforum/glossary/GlossaryPage';To verify, run this script to check for the correct file path:
🏁 Script executed:
#!/bin/bash # Verify the existence of GlossaryPage.tsx under cleanairforum/glossary directory fd 'GlossaryPage.tsx' | grep -i 'cleanairforum/glossary'Length of output: 117
Import Path Verified – Please Update to Lowercase Directory Name
The verification script confirms that the correct file exists at
src/website2/src/views/cleanairforum/glossary/GlossaryPage.tsx
, so the import should refer to the lowercase folder name.
- File:
src/website2/src/app/clean-air-forum/glossary/page.tsx
(Line 3)- Action: Change the import path from
@/views/cleanAirForum/glossary/GlossaryPage
to@/views/cleanairforum/glossary/GlossaryPage
.Suggested diff:
-import GlossaryPage from '@/views/cleanAirForum/glossary/GlossaryPage'; +import GlossaryPage from '@/views/cleanairforum/glossary/GlossaryPage';src/website2/src/views/cleanairforum/glossary/GlossaryPage.tsx (1)
95-97
: Consider an alternative to dangerouslySetInnerHTML.While you're correctly using DOMPurify to sanitize the HTML content, consider using a React-friendly HTML renderer like react-html-parser or a markdown renderer if possible.
Even with DOMPurify, make sure the content is coming from a trusted source and all user-provided content is properly sanitized.
🧰 Tools
🪛 Biome (1.9.4)
[error] 95-95: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
src/website2/src/app/clean-air-forum/logistics/page.tsx (2)
5-9
: Good addition of metadata for SEO.Adding metadata for title and description is excellent for SEO optimization. This will help improve visibility of the Clean Air Forum logistics page in search engines.
11-13
: Component pattern is consistent with other pages.The pattern of creating a simple Page component that renders a more complex component is consistent with the restructuring seen in other pages. This separation of concerns is a good practice.
|
||
import AboutPage from '@/views/Forum/AboutPage'; | ||
import AboutPage from '@/views/cleanAirForum/about/AboutPage'; |
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.
Fix import path case sensitivity.
The import path is causing a build failure due to case sensitivity issues.
-import AboutPage from '@/views/cleanAirForum/about/AboutPage';
+import AboutPage from '@/views/cleanairforum/about/AboutPage';
The pipeline failure indicates that the module cannot be resolved. The actual directory path uses lowercase 'cleanairforum' rather than the camelCase 'cleanAirForum' in your import.
📝 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.
import AboutPage from '@/views/cleanAirForum/about/AboutPage'; | |
import AboutPage from '@/views/cleanairforum/about/AboutPage'; |
{selectedEvent.forum_resources?.map( | ||
(resource: any, resourceIndex: number) => ( | ||
<div key={resource.id} className="mb-8"> | ||
<h2 className="text-2xl font-bold text-gray-900 mb-4"> | ||
{resource.resource_title} | ||
</h2> | ||
{resource.resource_sessions?.map( | ||
(session: any, sessionIndex: number) => ( | ||
<AccordionItem | ||
key={session.id} | ||
session={session} | ||
isOpen={ | ||
allExpanded || | ||
openAccordions[resourceIndex]?.[sessionIndex] || | ||
false | ||
} | ||
toggleAccordion={() => | ||
handleToggleAccordion(resourceIndex, sessionIndex) | ||
} | ||
/> | ||
), | ||
)} | ||
<Divider className="bg-black p-0 m-0 h-[1px] w-full" /> | ||
</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.
🛠️ Refactor suggestion
Add null checking for forum_resources and resource_sessions.
The component doesn't check if forum_resources
or resource_sessions
exist before mapping over them, which could lead to runtime errors.
- {selectedEvent.forum_resources?.map(
+ {selectedEvent.forum_resources && selectedEvent.forum_resources.length > 0 ? (
+ selectedEvent.forum_resources.map(
(resource: any, resourceIndex: number) => (
<div key={resource.id} className="mb-8">
<h2 className="text-2xl font-bold text-gray-900 mb-4">
{resource.resource_title}
</h2>
- {resource.resource_sessions?.map(
+ {resource.resource_sessions && resource.resource_sessions.length > 0 ? (
+ resource.resource_sessions.map(
(session: any, sessionIndex: number) => (
<AccordionItem
key={session.id}
session={session}
isOpen={
allExpanded ||
openAccordions[resourceIndex]?.[sessionIndex] ||
false
}
toggleAccordion={() =>
handleToggleAccordion(resourceIndex, sessionIndex)
}
/>
),
- )}
+ ) : (
+ <p className="text-gray-500">No sessions available for this resource.</p>
+ )}
<Divider className="bg-black p-0 m-0 h-[1px] w-full" />
</div>
- ),
- )}
+ ))
+ ) : (
+ <p className="text-gray-500">No resources available for this event.</p>
+ )}
📝 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.
{selectedEvent.forum_resources?.map( | |
(resource: any, resourceIndex: number) => ( | |
<div key={resource.id} className="mb-8"> | |
<h2 className="text-2xl font-bold text-gray-900 mb-4"> | |
{resource.resource_title} | |
</h2> | |
{resource.resource_sessions?.map( | |
(session: any, sessionIndex: number) => ( | |
<AccordionItem | |
key={session.id} | |
session={session} | |
isOpen={ | |
allExpanded || | |
openAccordions[resourceIndex]?.[sessionIndex] || | |
false | |
} | |
toggleAccordion={() => | |
handleToggleAccordion(resourceIndex, sessionIndex) | |
} | |
/> | |
), | |
)} | |
<Divider className="bg-black p-0 m-0 h-[1px] w-full" /> | |
</div> | |
), | |
)} | |
{selectedEvent.forum_resources && selectedEvent.forum_resources.length > 0 ? ( | |
selectedEvent.forum_resources.map( | |
(resource: any, resourceIndex: number) => ( | |
<div key={resource.id} className="mb-8"> | |
<h2 className="text-2xl font-bold text-gray-900 mb-4"> | |
{resource.resource_title} | |
</h2> | |
{resource.resource_sessions && resource.resource_sessions.length > 0 ? ( | |
resource.resource_sessions.map( | |
(session: any, sessionIndex: number) => ( | |
<AccordionItem | |
key={session.id} | |
session={session} | |
isOpen={ | |
allExpanded || | |
openAccordions[resourceIndex]?.[sessionIndex] || | |
false | |
} | |
toggleAccordion={() => | |
handleToggleAccordion(resourceIndex, sessionIndex) | |
} | |
/> | |
), | |
) | |
) : ( | |
<p className="text-gray-500">No sessions available for this resource.</p> | |
)} | |
<Divider className="bg-black p-0 m-0 h-[1px] w-full" /> | |
</div> | |
), | |
) | |
) : ( | |
<p className="text-gray-500">No resources available for this event.</p> | |
)} |
const AccordionItem = ({ session, isOpen, toggleAccordion }: any) => { | ||
return ( | ||
<div className="bg-gray-100 rounded-lg shadow-sm py-2 px-4 mb-4"> | ||
<div | ||
className="flex justify-between items-center cursor-pointer" | ||
onClick={toggleAccordion} | ||
> | ||
<h3 className="text-lg font-semibold text-blue-700"> | ||
{session.session_title} | ||
</h3> | ||
<span>{isOpen ? <FaChevronUp /> : <FaChevronDown />}</span> | ||
</div> | ||
{isOpen && ( | ||
<div className="mt-4"> | ||
{session?.resource_files?.map((file: any) => ( | ||
<div key={file.id} className="mb-4"> | ||
<div className="flex items-start space-x-2 pl-4"> | ||
<div> | ||
<p className="text-sm text-gray-900 font-semibold"> | ||
{file.resource_summary} | ||
</p> | ||
<a | ||
href={file.file_url} | ||
target="_blank" | ||
rel="noopener noreferrer" | ||
className="text-red-600 flex items-center" | ||
> | ||
<FaFilePdf className="mr-2" /> | ||
{getFileNameFromUrl(file.file_url)} | ||
</a> | ||
</div> | ||
</div> | ||
</div> | ||
))} | ||
</div> | ||
)} | ||
</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.
🛠️ Refactor suggestion
Replace any
types with proper interfaces for better type safety.
The AccordionItem
component uses any
type, which reduces type safety. Consider defining proper interfaces for the component props.
+interface ResourceFile {
+ id: string;
+ resource_summary: string;
+ file_url: string;
+}
+interface Session {
+ id: string;
+ session_title: string;
+ resource_files?: ResourceFile[];
+}
+interface AccordionItemProps {
+ session: Session;
+ isOpen: boolean;
+ toggleAccordion: () => void;
+}
-const AccordionItem = ({ session, isOpen, toggleAccordion }: any) => {
+const AccordionItem = ({ session, isOpen, toggleAccordion }: AccordionItemProps) => {
📝 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.
const AccordionItem = ({ session, isOpen, toggleAccordion }: any) => { | |
return ( | |
<div className="bg-gray-100 rounded-lg shadow-sm py-2 px-4 mb-4"> | |
<div | |
className="flex justify-between items-center cursor-pointer" | |
onClick={toggleAccordion} | |
> | |
<h3 className="text-lg font-semibold text-blue-700"> | |
{session.session_title} | |
</h3> | |
<span>{isOpen ? <FaChevronUp /> : <FaChevronDown />}</span> | |
</div> | |
{isOpen && ( | |
<div className="mt-4"> | |
{session?.resource_files?.map((file: any) => ( | |
<div key={file.id} className="mb-4"> | |
<div className="flex items-start space-x-2 pl-4"> | |
<div> | |
<p className="text-sm text-gray-900 font-semibold"> | |
{file.resource_summary} | |
</p> | |
<a | |
href={file.file_url} | |
target="_blank" | |
rel="noopener noreferrer" | |
className="text-red-600 flex items-center" | |
> | |
<FaFilePdf className="mr-2" /> | |
{getFileNameFromUrl(file.file_url)} | |
</a> | |
</div> | |
</div> | |
</div> | |
))} | |
</div> | |
)} | |
</div> | |
); | |
}; | |
interface ResourceFile { | |
id: string; | |
resource_summary: string; | |
file_url: string; | |
} | |
interface Session { | |
id: string; | |
session_title: string; | |
resource_files?: ResourceFile[]; | |
} | |
interface AccordionItemProps { | |
session: Session; | |
isOpen: boolean; | |
toggleAccordion: () => void; | |
} | |
const AccordionItem = ({ session, isOpen, toggleAccordion }: AccordionItemProps) => { | |
return ( | |
<div className="bg-gray-100 rounded-lg shadow-sm py-2 px-4 mb-4"> | |
<div | |
className="flex justify-between items-center cursor-pointer" | |
onClick={toggleAccordion} | |
> | |
<h3 className="text-lg font-semibold text-blue-700"> | |
{session.session_title} | |
</h3> | |
<span>{isOpen ? <FaChevronUp /> : <FaChevronDown />}</span> | |
</div> | |
{isOpen && ( | |
<div className="mt-4"> | |
{session?.resource_files?.map((file: any) => ( | |
<div key={file.id} className="mb-4"> | |
<div className="flex items-start space-x-2 pl-4"> | |
<div> | |
<p className="text-sm text-gray-900 font-semibold"> | |
{file.resource_summary} | |
</p> | |
<a | |
href={file.file_url} | |
target="_blank" | |
rel="noopener noreferrer" | |
className="text-red-600 flex items-center" | |
> | |
<FaFilePdf className="mr-2" /> | |
{getFileNameFromUrl(file.file_url)} | |
</a> | |
</div> | |
</div> | |
</div> | |
))} | |
</div> | |
)} | |
</div> | |
); | |
}; |
dangerouslySetInnerHTML={{ __html: renderContent(subText) }} | ||
/> |
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.
Unsanitized dangerouslySetInnerHTML in AccordionItem.
Unlike other instances in this file, this dangerouslySetInnerHTML is using renderContent directly without DOMPurify sanitization.
Apply DOMPurify to sanitize the content before rendering:
- dangerouslySetInnerHTML={{ __html: renderContent(subText) }}
+ dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(renderContent(subText)) }}
📝 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.
dangerouslySetInnerHTML={{ __html: renderContent(subText) }} | |
/> | |
dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(renderContent(subText)) }} | |
/> |
🧰 Tools
🪛 Biome (1.9.4)
[error] 48-48: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
const ProgramsPage = () => { | ||
const { selectedEvent } = useForumData(); | ||
const [openAccordion, setOpenAccordion] = useState<string | null>(null); | ||
|
||
if (!selectedEvent) { | ||
return null; | ||
} |
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.
🛠️ Refactor suggestion
Improve empty state handling.
The component returns null when selectedEvent is not available, which might lead to an abrupt UI experience.
Provide a better user experience with a loading state or placeholder:
if (!selectedEvent) {
- return null;
+ return (
+ <div className="px-4 prose max-w-none lg:px-0 min-h-[60vh] flex items-center justify-center">
+ <p>Loading program information...</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.
const ProgramsPage = () => { | |
const { selectedEvent } = useForumData(); | |
const [openAccordion, setOpenAccordion] = useState<string | null>(null); | |
if (!selectedEvent) { | |
return null; | |
} | |
const ProgramsPage = () => { | |
const { selectedEvent } = useForumData(); | |
const [openAccordion, setOpenAccordion] = useState<string | null>(null); | |
if (!selectedEvent) { | |
return ( | |
<div className="px-4 prose max-w-none lg:px-0 min-h-[60vh] flex items-center justify-center"> | |
<p>Loading program information...</p> | |
</div> | |
); | |
} |
import { Metadata } from 'next'; | ||
|
||
import DOMPurify from 'dompurify'; | ||
import React from 'react'; | ||
import PartnersPage from '@/views/cleanAirForum/partners/PartnersPage'; |
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.
Pipeline failure due to module lookup.
The import path '@/views/cleanAirForum/partners/PartnersPage'
appears to be incorrect or inconsistent with your folder naming convention, causing a “Module not found” error. Ensure that the directory names match the casing in the import. For example, if your folder is spelled “cleanairforum” in lowercase, apply this fix:
- import PartnersPage from '@/views/cleanAirForum/partners/PartnersPage';
+ import PartnersPage from '@/views/cleanairforum/partners/PartnersPage';
📝 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.
import { Metadata } from 'next'; | |
import DOMPurify from 'dompurify'; | |
import React from 'react'; | |
import PartnersPage from '@/views/cleanAirForum/partners/PartnersPage'; | |
import { Metadata } from 'next'; | |
import PartnersPage from '@/views/cleanairforum/partners/PartnersPage'; |
🧰 Tools
🪛 GitHub Actions: deploy-to-preview-environment
[error] 1-1: Module not found: Can't resolve '@/views/cleanAirForum/partners/PartnersPage'
<Link | ||
href={href} | ||
target="_blank" | ||
className="text-blue-600 hover:underline" | ||
> | ||
{event.title} | ||
</Link> | ||
</li> |
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.
🛠️ Refactor suggestion
Add security attributes to external links.
When using target="_blank"
, it's recommended to add rel="noopener noreferrer"
to prevent potential security vulnerabilities like reverse tabnabbing.
<Link
href={href}
target="_blank"
+ rel="noopener noreferrer"
className="text-blue-600 hover:underline"
>
{event.title}
</Link>
📝 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.
<Link | |
href={href} | |
target="_blank" | |
className="text-blue-600 hover:underline" | |
> | |
{event.title} | |
</Link> | |
</li> | |
<Link | |
href={href} | |
target="_blank" | |
rel="noopener noreferrer" | |
className="text-blue-600 hover:underline" | |
> | |
{event.title} | |
</Link> | |
</li> |
import { ForumEvent } from '@/types/forum'; | ||
import { isValidGlossaryContent } from '@/utils/glossaryValidator'; | ||
import { renderContent } from '@/utils/quillUtils'; | ||
import SectionDisplay from '@/views/cleanAirForum/SectionDisplay'; |
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.
Fix import path case inconsistency.
The import path uses 'cleanAirForum' (camelCase) while the actual directory path in the file structure appears to be 'cleanairforum' (lowercase). This could cause build failures on case-sensitive file systems like Linux.
-import SectionDisplay from '@/views/cleanAirForum/SectionDisplay';
+import SectionDisplay from '@/views/cleanairforum/SectionDisplay';
📝 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.
import SectionDisplay from '@/views/cleanAirForum/SectionDisplay'; | |
import SectionDisplay from '@/views/cleanairforum/SectionDisplay'; |
<Link | ||
key={index} | ||
href={buildHref(link.href)} |
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.
🛠️ Refactor suggestion
Avoid using array index as key.
Using array index as a React key can lead to unexpected behavior when the list items are reordered or modified. Consider using a more stable identifier.
<Link
- key={index}
+ key={link.href}
href={buildHref(link.href)}
className={`relative flex-shrink-0 no-underline text-gray-700 hover:text-gray-900 transition ${
isActiveTab(link.href) ? 'font-semibold text-gray-900' : ''
}`}
>
📝 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.
<Link | |
key={index} | |
href={buildHref(link.href)} | |
<Link | |
key={link.href} | |
href={buildHref(link.href)} | |
className={`relative flex-shrink-0 no-underline text-gray-700 hover:text-gray-900 transition ${ | |
isActiveTab(link.href) ? 'font-semibold text-gray-900' : '' | |
}`} | |
> |
|
||
import DOMPurify from 'dompurify'; | ||
import React from 'react'; | ||
import LogisticsPage from '@/views/cleanAirForum/logistics/LogisticsPage'; |
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.
Fix the import path for LogisticsPage component.
The build pipeline is failing because it cannot find the LogisticsPage component. There appears to be a case sensitivity issue in the import path.
-import LogisticsPage from '@/views/cleanAirForum/logistics/LogisticsPage';
+import LogisticsPage from '@/views/cleanairforum/logistics/LogisticsPage';
Note: Ensure that the component actually exists at this path. If it doesn't, you'll need to create it first.
📝 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.
import LogisticsPage from '@/views/cleanAirForum/logistics/LogisticsPage'; | |
import LogisticsPage from '@/views/cleanairforum/logistics/LogisticsPage'; |
New Website2 changes available for preview here |
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.
thanks @OchiengPaul442
Summary of Changes (What does this PR do?)
Status of maturity (all need to be checked before merging):
Screenshots (optional)
Summary by CodeRabbit
New Features
Enhancements