-
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 #2538
Conversation
📝 WalkthroughWalkthroughThis pull request introduces multiple updates across the project. Three new permanent redirect rules were added to the Next.js configuration, several icon files had file path comments removed, and a new manifest was created. Multiple page components for both the Clean Air Forum and Clean Air Network were refactored—adding metadata, renaming components, and updating import paths. Additionally, layout files were updated for improved analytics and SEO integration, and the TabNavigation components were restructured to support dynamic URL handling. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant N as Next.js Server
participant R as Redirect Logic
participant P as Page Component
U->>N: Request URL (e.g., /clean-air)
N->>R: Evaluate redirect rules
alt Redirect rule matches
R-->>U: HTTP 308 Redirect to /clean-air-network/about
else No match
N->>P: Render requested page (with metadata)
P-->>U: Return rendered page
end
Possibly related PRs
Suggested reviewers
Poem
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 6
🧹 Nitpick comments (26)
src/website2/src/app/clean-air-network/about/page.tsx (1)
11-17
: Consider simplifying the component structure.The div wrapper around CleanAirPage appears unnecessary if CleanAirPage is already a complete component. Removing it would reduce DOM nesting.
const Page = () => { return ( - <div> <CleanAirPage /> - </div> ); };src/website2/src/app/clean-air-forum/about/page.tsx (1)
11-19
: Proper component naming convention.The component has been renamed from 'page' to 'Page', following React conventions for PascalCase component names. As with the other page component, consider removing the unnecessary div wrapper.
const Page = () => { return ( - <div> <AboutPage /> - </div> ); };src/website2/src/views/cleanairforum/partners/PartnersPage.tsx (3)
17-23
: Type safety improvement for partner objects.Using
any
type loses TypeScript's benefits. Consider creating an interface for partners.+interface Partner { + id: string; + partner_logo_url: string; + category: string; +} + const conveningPartners = selectedEvent.partners - ?.filter((partner: any) => partner.category === 'Co-Convening Partner') - .map((partner: any) => ({ + ?.filter((partner: Partner) => partner.category === 'Co-Convening Partner') + .map((partner: Partner) => ({ id: partner.id, logoUrl: partner.partner_logo_url, }));
68-72
: Unnecessary Fragment usage.This Fragment is redundant as it contains only a single child element.
- {partnersSections && partnersSections.length > 0 && ( - <> - {partnersSections.map((section: any) => ( - <SectionDisplay key={section.id} section={section} /> - ))} - </> - )} + {partnersSections && partnersSections.length > 0 && + partnersSections.map((section: any) => ( + <SectionDisplay key={section.id} section={section} /> + )) + }🧰 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)
48-52
: Consider extracting repeated HTML validation logic.The HTML validation pattern is repeated in multiple places in this component. Consider extracting this to a reusable helper function.
+ const isValidSectionForPage = (section: any, pageName: string): boolean => { + if (!section.pages.includes(pageName)) return false; + const sectionHTML = renderContent(section.content); + return isValidHTMLContent(sectionHTML); + }; + - const partnersSections = selectedEvent.sections?.filter((section: any) => { - if (!section.pages.includes('partners')) return false; - const sectionHTML = renderContent(section.content); - return isValidHTMLContent(sectionHTML); - }); + const partnersSections = selectedEvent.sections?.filter(section => + isValidSectionForPage(section, 'partners') + );src/website2/src/app/clean-air-forum/logistics/page.tsx (1)
5-9
: Provide more detailed keywords in metadata
Consider adding more keywords or metadata properties for enhanced SEO. The title and description here are a good start and match the page purpose effectively.src/website2/src/views/cleanairforum/logistics/LogisticsPage.tsx (1)
86-91
: Fragment usage
A fragment is valid since the.map(...)
call returns multiple elements. However, if preferred, this could be replaced by a parent<div>
for slightly clearer structure.🧰 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/speakers/SpeakersPage.tsx (2)
14-21
: Fallback strategy
Returningnull
ifselectedEvent
is not available is concise. If there's any loading delay, consider a loader or a placeholder for better user experience.
137-141
: Simplify or remove the fragment
This fragment only contains a map of<SectionDisplay />
. You could place them directly without a fragment or wrap them in a semantic container for clarity.🧰 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-56
: Remove unnecessary React fragment.The fragment around
sponsorshipSections.map
is not strictly required since you can directly return the mapped elements within the conditional check.Proposed change:
{sponsorshipSections && sponsorshipSections.length > 0 && ( - <> {sponsorshipSections.map((section: any) => ( <SectionDisplay key={section.id} section={section} /> ))} - </> )}🧰 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/TabNavigation.tsx (1)
32-37
: Forward-looking approach for building query parameters.Appending
?slug=
works for now, but if future enhancements require more query parameters, a more scalable approach to building URLs (e.g., using URLSearchParams) might be beneficial.src/website2/src/views/cleanairforum/program-committee/CommitteePage.tsx (4)
1-1
: Consider removing the ESLint directive.Disabling the exhaustive-deps rule across the entire component could lead to subtle bugs. Instead of disabling the rule completely, consider addressing the specific dependencies that trigger the warnings.
If there are specific dependencies you intentionally want to exclude, you can specify them in the dependency array with a preceding comment explaining why they're excluded:
- /* eslint-disable react-hooks/exhaustive-deps */
64-66
: Consider adding a loading state instead of null.Returning null when the selectedEvent is not available might lead to a poor user experience. Consider adding a loading indicator or skeleton to provide feedback to users.
- if (!selectedEvent) { - return null; - } + if (!selectedEvent) { + return <div className="flex justify-center py-8"><LoadingSpinner /></div>; + }
87-93
: Remove unnecessary Fragment.The Fragment wrapper is redundant since it only contains a single child element (the map function).
- {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)
28-46
: Consider adding type definitions for person and section objects.Using
any
type for persons and sections reduces type safety. Consider defining proper interfaces for these objects to improve code quality and catch potential errors at compile time.// Example types you could define interface Person { id: string; category: string; name: string; bio?: string; // other properties... } interface Section { id: string; content: string; pages: string[]; // other properties... }Then use these types instead of
any
:- return persons.filter( - (person: any) => + return persons.filter( + (person: Person) =>src/website2/src/views/cleanairforum/resources/ResourcesPage.tsx (3)
32-51
: Type safety for mapped elements.When mapping over collections, ensure proper type definitions for better code safety and readability.
-{session?.resource_files?.map((file: any) => ( +{session?.resource_files?.map((file: SessionResourceFile) => (
60-62
: Improve state management with proper TypeScript types.The state management uses partially typed structures but still relies on 'any' in critical areas.
-const [openAccordions, setOpenAccordions] = useState<{ - [resourceIndex: number]: { [sessionIndex: number]: boolean }; -}>({}); +interface AccordionState { + [resourceIndex: number]: { [sessionIndex: number]: boolean }; +} + +const [openAccordions, setOpenAccordions] = useState<AccordionState>({}); // And later in the handleExpandAll function: -const expandedAccordions: any = {}; +const expandedAccordions: AccordionState = {};Also applies to: 82-94
65-67
: Consider adding null handling for missing data.The component could benefit from more robust handling of potentially missing data.
Add appropriate null checks or fallback UI states when data might be unavailable.
src/website2/src/views/cleanairforum/sessions-programs/ProgramsPage.tsx (3)
14-20
: Good use of TypeScript interface.The
AccordionItemProps
interface is well-defined, but should avoid usingany[]
for sessions.- sessions: any[]; + interface Session { + id: string; + start_time: string; + session_title: string; + session_details: string; + } + sessions: Session[];
117-123
: Remove unnecessary fragments.The code uses empty fragments that aren't needed and were flagged by static analysis.
- {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)
94-98
: Define proper interfaces for section data.Using
any
types for section data reduces type safety. Consider creating interfaces.+interface Section { + id: string; + pages: string[]; + content: string; +} - const sessionSections = selectedEvent.sections?.filter((section: any) => { + const sessionSections = selectedEvent.sections?.filter((section: Section) => {src/website2/src/views/cleanairforum/glossary/GlossaryPage.tsx (5)
39-43
: Consider adding a type definition instead of usingany
type.Using a more specific type instead of
any
would improve type safety and code maintainability. If you have a defined type for sections, please use it here.- const glossarySections = selectedEvent.sections?.filter((section: any) => { + const glossarySections = selectedEvent.sections?.filter((section: SectionType) => {You'll need to import or define the appropriate
SectionType
interface.
62-64
: Consider using Next.js routing patterns for better maintainability.The URL construction works but could be improved using Next.js route patterns. This would make the code more maintainable if routing structures change.
- const href = `/clean-air-forum/about?slug=${encodeURIComponent( - event.unique_title, - )}`; + const href = { + pathname: '/clean-air-forum/about', + query: { slug: event.unique_title } + };
95-98
: Consider alternative approaches to dangerouslySetInnerHTML.While you're correctly using DOMPurify to sanitize the content,
dangerouslySetInnerHTML
is still flagged as a security concern. You could consider using an HTML parser library likehtml-react-parser
which has built-in sanitization.- <div - className="md:w-2/3" - dangerouslySetInnerHTML={{ - __html: DOMPurify.sanitize(glossaryHTML), - }} - ></div> + <div className="md:w-2/3"> + {parse(DOMPurify.sanitize(glossaryHTML))} + </div>You would need to import the parser:
import parse from 'html-react-parser';🧰 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)
105-109
: Remove unnecessary Fragment.The Fragment wrapper is unnecessary here since it only contains a single child. You can simplify this code.
- {glossarySections && glossarySections.length > 0 && ( - <> - {glossarySections.map((section: any) => ( - <SectionDisplay key={section.id} section={section} /> - ))} - </> - )} + {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)
16-114
: Consider adding error handling.The component currently doesn't handle potential errors that could occur when fetching or rendering data. You might want to add error boundary or try/catch blocks for more robust error handling.
You could implement a basic error state like this:
const GlossaryPage: React.FC = () => { const [error, setError] = React.useState<Error | null>(null); // Wrap data access in try/catch try { const { selectedEvent, eventTitles } = useForumData(); // Existing component logic... } catch (err) { setError(err instanceof Error ? err : new Error('Unknown error occurred')); return <div className="error-message">Sorry, something went wrong loading the glossary.</div>; } // Rest of component... };🧰 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)
[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)
📜 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 (47)
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
(2 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/AboutPage.tsx
(1 hunks)src/website2/src/views/cleanairforum/TabNavigation.tsx
(1 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/Icon1.tsx
- src/website2/public/assets/icons/Icon5.tsx
- src/website2/public/assets/icons/Icon7.tsx
- src/website2/public/assets/icons/Icon6.tsx
- src/website2/public/assets/icons/Icon2.tsx
- src/website2/public/assets/icons/Icon3.tsx
- src/website2/src/components/layouts/Navbar.tsx
- src/website2/src/views/about/AboutPage.tsx
- src/website2/public/assets/icons/Icon4.tsx
- src/website2/public/assets/icons/Icon8.tsx
- src/website2/src/views/cleanairforum/AboutPage.tsx
- src/website2/public/manifest.json
- src/website2/src/components/layouts/NewsLetter.tsx
- src/website2/src/app/clean-air-network/events/[id]/page.tsx
- src/website2/src/views/cleanAirNetwork/membership/MemberPage.tsx
- src/website2/src/views/cleanAirNetwork/about/CleanAirPage.tsx
- src/website2/src/views/cleanAirNetwork/events/EventsPage.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/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/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/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/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/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)
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)
🔇 Additional comments (49)
src/website2/src/app/clean-air-forum/layout.tsx (1)
14-14
: Import path update reflects directory restructuring.The import path has been updated to use the new
cleanAirForum
directory structure instead of the previousForum
directory. This change aligns with the broader project restructuring effort seen in other files, which improves organization and readability.src/website2/next.config.mjs (1)
49-63
:❓ Verification inconclusive
New redirect rules enhance routing for Clean Air Network.
The addition of these three permanent redirects ensures users accessing legacy URLs or shortened paths will be properly directed to the Clean Air Network about page. This is a good practice for maintaining backward compatibility while implementing a more organized URL structure.
However, verify that the destination pages exist and are properly implemented:
🏁 Script executed:
#!/bin/bash # Check if the destination page exists fd "about/page.tsx" src/website2/src/app/clean-air-network/Length of output: 59
Action required: Manually verify destination page existence
The new permanent redirect rules look solid for maintaining backward compatibility by routing legacy URLs to the Clean Air Network about page. However, the file search for the destination page (using
fd "about/page.tsx" src/website2/src/app/clean-air-network/
) produced no output. This could mean:
- The expected file (
about/page.tsx
) either doesn't exist or is located elsewhere.- The file might have a different name or path than anticipated.
To ensure everything works as intended, please manually verify that the destination page exists at the correct location.
- Verification needed: Confirm that the intended destination file for
/clean-air-network/about
is present and properly implemented.src/website2/src/components/layouts/Footer.tsx (3)
4-5
: Updated Twitter icon import to reflect X rebranding.The import statements have been updated to remove the outdated
FaTwitter
icon and replace it with the newerFaXTwitter
icon from thereact-icons/fa6
package, reflecting Twitter's rebranding to X.
32-34
: Semantic HTML improvement for tagline.Changed the tagline container from a
<p>
tag to a<h1>
tag, which improves semantic structure and accessibility. The text color has also been slightly adjusted fromtext-gray-800
totext-gray-700
for better visual hierarchy.
64-64
: Updated Twitter icon to X branding.The Twitter icon has been updated from
FaTwitter
toFaXTwitter
to reflect the platform's rebranding while maintaining the same styling and functionality.src/website2/src/app/clean-air-network/resources/page.tsx (2)
1-9
: Added metadata for improved SEO.The addition of metadata including title and description will significantly improve SEO for the Clean Air Network Resources page. The description is comprehensive and keyword-rich, which should help with search engine rankings.
11-17
: Simplified component structure.The page component has been refactored to follow the pattern used throughout the application, simplifying it to just render the appropriate view component. This improves maintainability and separation of concerns.
However, the component reference (
ResourcePage
vsResourcesPage
) needs to be corrected as noted in the previous comment.src/website2/src/app/clean-air-network/about/page.tsx (2)
1-4
: Good job with the proper imports and component organization.The imports are well-structured, with the Metadata type from Next.js and the appropriate component import from the views directory.
5-9
: Well-crafted metadata for SEO optimization.The metadata object provides a clear title and comprehensive description that accurately represents the page content. This will improve search engine visibility and user understanding of the page purpose.
src/website2/src/views/cleanAirNetwork/TabNavigation.tsx (4)
1-8
: Proper client-side directive and import setup.The 'use client' directive is correctly placed at the top, and all necessary imports are present for Next.js navigation functionality.
9-14
: Well-structured tab configuration.The tabs array provides a clean, maintainable way to define navigation options with appropriate labels and paths.
16-25
: Good active tab detection logic.The isActiveTab function has appropriate special case handling for the "about" tab with exact matching, while other tabs use startsWith for more flexible matching. This prevents unintended active states.
26-47
: Clean, responsive navigation implementation.The navigation component uses Tailwind CSS effectively for styling, handles overflow appropriately, and provides clear visual feedback for active tabs. The container class from mainConfig ensures consistency with the rest of the application.
src/website2/src/components/layouts/NotificationBanner.tsx (2)
13-13
: Correct route update for Clean Air Network.The updated route aligns with the new page structure and redirect rules mentioned in the PR objectives.
54-57
: Proper event handling preserved.The e.stopPropagation() call is correctly maintained to prevent event bubbling, which is necessary when handling clicks within a Link component.
src/website2/src/app/clean-air-forum/about/page.tsx (2)
1-3
: Appropriate imports with updated path.The Metadata import and updated path to the AboutPage component reflect the reorganized folder structure for better consistency.
5-9
: Well-defined metadata for SEO.The metadata object with title and description will improve search engine visibility and user understanding of the Clean Air Forum page.
src/website2/src/app/clean-air-network/events/page.tsx (1)
1-19
: Good implementation of metadata and proper component naming conventions.The changes to this file represent good practices in Next.js development:
- The addition of the
metadata
export provides essential SEO information- The component has been properly renamed from
page
toPage
to follow React component naming conventions- The import path has been updated to reflect what appears to be a directory restructuring from 'cleanairforum' to 'cleanAirNetwork'
All changes align with modern Next.js practices and follow proper component naming conventions.
src/website2/src/app/clean-air-network/membership/page.tsx (1)
1-19
: Well-structured page component with proper metadata implementation.This file demonstrates good Next.js practices:
- Appropriate metadata implementation for SEO purposes
- Clean component structure with proper naming conventions
- Consistent with the architectural changes being made across the application
The metadata is detailed and descriptive, which is excellent for SEO. The import path update to cleanAirNetwork is consistent with similar changes in other files, suggesting a coordinated refactoring effort.
src/website2/src/app/clean-air-forum/glossary/page.tsx (1)
1-15
: Effective simplification of page component.The changes here represent a good refactoring approach. The page is now a thin wrapper around the GlossaryPage component, following the separation of concerns principle. The metadata implementation enhances SEO capabilities.
This pattern of simplifying page components while moving the implementation details to dedicated components is consistent across the PR and follows good architectural practices.
src/website2/src/app/clean-air-forum/logistics/page.tsx (1)
11-15
: Straightforward page composition
Wrapping and returning<LogisticsPage />
fosters good modularity and keeps this page file clean.src/website2/src/views/cleanairforum/logistics/LogisticsPage.tsx (3)
1-12
: Imports and client directive are well-structured
Using'use client'
in Next.js is correct here, ensuring the component runs on the client side. Imports are neatly organized for clarity.
13-21
: Effective loading state
Returning<Loading />
on an emptyselectedEvent
avoids null reference errors or blank content. This is a straightforward and safe fallback.
41-56
: Cautious usage of dangerouslySetInnerHTML
Relying onDOMPurify.sanitize
is a strong defense, but remain vigilant about future updates or other entry points that might bypass sanitization.🧰 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)
src/website2/src/views/cleanairforum/speakers/SpeakersPage.tsx (2)
1-11
: Appropriate client-side usage
The file is properly marked'use client'
, and imports appear consistent with its intended functionality.
81-85
: Practice caution with dangerouslySetInnerHTML
UsingDOMPurify
is helpful, but keep the sanitization configuration robust to thwart potential XSS vulnerabilities.🧰 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 (2)
5-9
: Clear metadata definition
The provided title and description align well with the forum sessions, aiding search engine indexing.
11-15
: Utilizes the external ProgramsPage component
ExportingPage
to wrapProgramsPage
promotes a clean, modular architecture while streamlining page functionality.src/website2/src/views/cleanairforum/sponsorship/SponsorshipPage.tsx (3)
16-21
: Validate partner logo presence.You're filtering for sponsor partners, but if
partner_logo_url
is missing or empty, it might break the sponsor listing layout. Consider handling any missing logos gracefully.
43-46
: Ensure sanitization logic is sufficient.Using
DOMPurify.sanitize()
mitigates potential XSS risks withdangerouslySetInnerHTML
. Confirm that untrusted or user-generated content is thoroughly sanitized, and that no dangerous HTML attributes or tags are allowed in the final rendering.🧰 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)
59-77
: Well-structured sponsor display.The paginated layout for sponsors looks solid. This approach helps avoid clutter from large sponsor lists and ensures a more pleasant user experience.
src/website2/src/views/cleanairforum/TabNavigation.tsx (2)
15-16
: Beware partial URL matching for active tabs.Using
pathname.startsWith(path)
could incorrectly match URLs with similar prefixes. Consider stricter matching if you want narrower control of the active state.
40-57
: Handled dynamic tab navigation well.Mapping your
tabs
array intoLink
elements with an active state check ensures a tidy, maintainable UI.src/website2/src/app/layout.tsx (2)
36-37
: Check exposure of the GA environment variable.Although GA measurement IDs are typically not sensitive, confirm that exposing
NEXT_PUBLIC_GA_MEASUREMENT_ID
is intentional and consistent with your environment configuration.
84-153
: Comprehensive SEO and analytics setup.Your meta tags, manifest files, and GA snippet placement provide robust coverage for SEO and analytics. The
beforeInteractive
strategy ensures that tracking is established early for accurate metrics.src/website2/src/app/clean-air-forum/speakers/page.tsx (1)
1-15
: Well-structured page component with proper metadata implementation.This refactoring significantly improves the codebase by:
- Implementing Next.js metadata for better SEO
- Delegating the component logic to a dedicated view
- Creating a clean separation of concerns
The descriptive metadata enhances search engine visibility with an informative title and comprehensive description that accurately represents the speakers page content.
src/website2/src/app/clean-air-forum/sponsorships/page.tsx (1)
1-15
: Clean implementation with proper metadata structuring.This follows the same effective pattern as other pages in the Clean Air Forum section:
- Proper metadata export with descriptive title and detailed description
- Clean delegation to the specialized component implementation
- Simple, maintainable structure that separates concerns
The consistent approach across pages improves maintainability and developer experience.
src/website2/src/app/clean-air-forum/resources/page.tsx (1)
1-15
: Good implementation of consistent page pattern.This page maintains the consistent pattern seen across the Clean Air Forum section:
- Proper metadata implementation for SEO
- Clear separation between page structure and component logic
- Simple and maintainable component structure
The consistent implementation across the application demonstrates solid architectural planning.
src/website2/src/views/cleanairforum/program-committee/CommitteePage.tsx (3)
27-35
: Good use of memoization for performance optimization.The useMemo implementation effectively prevents unnecessary recalculations of committee members when other state changes. The filtering logic is clean and well-organized.
53-59
: Efficient filtering of sections with proper validation.Good pattern of combining filtering with validation in the same useMemo to ensure only valid sections are rendered. This helps prevent rendering empty or invalid content.
78-80
:❓ Verification inconclusive
Sanitization is good, but consider alternatives to dangerouslySetInnerHTML.
While you're correctly using DOMPurify for sanitization, using dangerouslySetInnerHTML still poses potential security risks. Consider using a more secure rendering approach if possible.
If you need to maintain the current approach, ensure that:
- Your DOMPurify configuration is restrictive enough
- The content source is trusted
- Consider adding additional content validation
🌐 Web query:
What are safer alternatives to dangerouslySetInnerHTML in React?
💡 Result:
When working with raw HTML content in React applications,
dangerouslySetInnerHTML
poses significant security risks due to potential XSS vulnerabilities[1][2][9]. Here are safer alternatives and best practices to consider:1. Use JSX for Static Content
For controlled or static HTML content, prefer composing React components directly with JSX:
const SafeComponent = () => ( <div> <p>This <b>content</b> avoids raw HTML injection entirely.</p> </div> );This approach leverages React’s automatic escaping of embedded strings[5][9].
2. Sanitize Content with DOMPurify
When handling third-party or user-generated HTML:
- Sanitize first using libraries like DOMPurify to strip malicious code[1][4][6]:
import DOMPurify from 'dompurify'; const SanitizedContent = ({ html }) => { const cleanHTML = DOMPurify.sanitize(html); return <div dangerouslySetInnerHTML={{ __html: cleanHTML }} />; };This neutralizes inline event handlers (e.g.,
onclick
,onerror
) and<script>
tags[6][7].
3. Convert HTML to React Elements
Use parsing libraries to transform sanitized HTML into React components:
- react-html-parser:
import ReactHtmlParser from 'react-html-parser'; const App = ({ html }) => <div>{ReactHtmlParser(DOMPurify.sanitize(html))}</div>;- html-react-parser:
import parse from 'html-react-parser'; const App = ({ html }) => <div>{parse(DOMPurify.sanitize(html))}</div>;These tools parse tags into React elements while preserving attributes like
class
andstyle
[3][4].
4. Content Security Policy (CSP)
Add a CSP header to block inline scripts and unauthorized sources:
<meta http-equiv="Content-Security-Policy" content="script-src 'self'">This mitigates risks even if malicious content bypasses sanitization[6][10].
When to Use
dangerouslySetInnerHTML
Reserve it only for:
- Trusted sources (e.g., internal CMS)[4][8].
- Fully sanitized content validated via automated tests[5][7].
- Legacy integrations where alternatives aren’t feasible[4].
Always combine sanitization with other safeguards like CSP for defense-in-depth[6][10]. By prioritizing these methods, you reduce exposure to XSS while maintaining dynamic content rendering capabilities.
Citations:
- 1: https://blog.logrocket.com/using-dangerouslysetinnerhtml-react-application/
- 2: https://dev.to/shehzadhussain/navigating-the-risks-of-dangerouslysetinnerhtml-in-react-5c19
- 3: https://www.dhiwise.com/post/how-to-use-react-dangerouslysetinnerhtml
- 4: https://deadsimplechat.com/blog/how-to-safely-use-dangerouslysetinnerhtml-in-react/
- 5: https://codeparrot.ai/blogs/understanding-dangerouslysetinnerhtml-in-react-a-complete-guide
- 6: https://macarthur.me/posts/safer-dangerouslysetinnerhtml
- 7: https://dev.to/alakkadshaw/how-to-safely-use-dangerouslysetinnerhtml-in-react-57l1
- 8: alternatives to dangerouslySetInnerHTML methods facebook/react#7462
- 9: https://pragmaticwebsecurity.com/articles/spasecurity/react-xss-part2
- 10: https://refine.dev/blog/use-react-dangerouslysetinnerhtml/
Revised Security Consideration on HTML Rendering
While it's great that you're sanitizing with DOMPurify, using
dangerouslySetInnerHTML
still carries risks. Consider the following:
Alternative Approaches:
- For static or largely predictable content, build the UI with JSX rather than injecting raw HTML.
- For dynamic or user-generated content, consider converting sanitized HTML into React elements using libraries like
html-react-parser
orreact-html-parser
to leverage React’s built-in protections.If Continuing with
dangerouslySetInnerHTML
:
- Ensure that your DOMPurify configuration is as restrictive as possible.
- Confirm that the HTML source is from a trusted origin.
- Consider implementing additional validation or a Content Security Policy (CSP) as a defense-in-depth measure.
Please review these considerations to decide if a safer rendering approach can be adopted, or if robust safeguards are in place when 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/partners/page.tsx (3)
1-9
: Good implementation of Next.js metadata for SEO.The metadata implementation follows Next.js best practices by providing descriptive title and description fields that will improve search engine visibility for the partners page.
3-3
: Import path is correctly cased.The import path properly uses the camelCase convention for the directory structure (
cleanAirForum
), which is consistent with the project's naming patterns.
11-15
: Clean component implementation.This follows the pattern of keeping page components lightweight by delegating rendering to specialized view components. This separation of concerns improves maintainability.
src/website2/src/views/cleanairforum/resources/ResourcesPage.tsx (1)
9-16
: Good utility function with proper error handling.The
getFileNameFromUrl
function includes appropriate null checks and error handling. However, consider adding a unit test to verify its behavior with various URL formats.src/website2/src/views/cleanairforum/sessions-programs/ProgramsPage.tsx (1)
29-36
: Good error handling for time formatting.The
formatTime
function properly handles potential errors when parsing invalid time formats, which prevents runtime crashes.src/website2/src/app/clean-air-forum/program-committee/page.tsx (2)
1-9
: Well-structured metadata for SEO.The metadata implementation follows Next.js best practices with descriptive title and description that will improve search engine rankings.
11-15
: Clean component implementation.This follows the project's architectural pattern of keeping page components lightweight and delegating rendering logic to specialized view components. This separation of concerns improves maintainability.
src/website2/src/views/cleanairforum/glossary/GlossaryPage.tsx (1)
1-115
: Overall implementation looks good.The component is well-structured with a clear separation of concerns. The use of DOMPurify for sanitizing HTML is a good security practice, and the conditional rendering is handled appropriately. The responsive design with different column widths for mobile and desktop is also well-implemented.
🧰 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)
[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)
|
||
const page = () => { | ||
import ResourcePage from '@/views/cleanAirNetwork/resources/ResourcePage'; |
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.
💡 Verification agent
🧩 Analysis chain
Import path needs correction.
The component is imported as ResourcePage
but used as ResourcesPage
in the component rendering. This inconsistency will cause the application to break.
Apply this diff to fix the inconsistency:
-import ResourcePage from '@/views/cleanAirNetwork/resources/ResourcePage';
+import ResourcesPage from '@/views/cleanAirNetwork/resources/ResourcesPage';
Or alternatively, update the component usage to match the import:
- <ResourcePage />
+ <ResourcesPage />
Verify the correct component name exists in the directory:
🏁 Script executed:
#!/bin/bash
# Check which component file actually exists
fd "Resource(s)?Page.tsx" src/website2/src/views/cleanAirNetwork/resources/
Length of output: 143
Component Usage Correction: Align with the Existing File
The file structure confirms that only ResourcePage.tsx
exists in the src/website2/src/views/cleanAirNetwork/resources/
directory. Therefore, the import statement is correct, and the JSX usage should be updated accordingly to avoid breaking the application.
- Action Required: Change the component usage from
<ResourcesPage />
to<ResourcePage />
.
Diff snippet for clarity:
- <ResourcesPage />
+ <ResourcePage />
Committable suggestion skipped: line range outside the PR's diff.
dangerouslySetInnerHTML={{ | ||
__html: DOMPurify.sanitize(mainPartnersHTML), | ||
}} | ||
/> |
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.
Security concern with dangerouslySetInnerHTML usage.
While you are using DOMPurify to sanitize the HTML content, which is good practice, the use of dangerouslySetInnerHTML still presents potential XSS vulnerabilities if the sanitization isn't completely effective.
Consider implementing a more restrictive sanitization configuration with DOMPurify to limit allowed HTML elements and attributes.
- <div
- dangerouslySetInnerHTML={{
- __html: DOMPurify.sanitize(mainPartnersHTML),
- }}
- />
+ <div
+ dangerouslySetInnerHTML={{
+ __html: DOMPurify.sanitize(mainPartnersHTML, {
+ ALLOWED_TAGS: ['p', 'b', 'i', 'em', 'strong', 'a', 'ul', 'ol', 'li', 'br'],
+ ALLOWED_ATTR: ['href', 'target', 'rel']
+ }),
+ }}
+ />
📝 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: DOMPurify.sanitize(mainPartnersHTML), | |
}} | |
/> | |
<div | |
dangerouslySetInnerHTML={{ | |
__html: DOMPurify.sanitize(mainPartnersHTML, { | |
ALLOWED_TAGS: ['p', 'b', 'i', 'em', 'strong', 'a', 'ul', 'ol', 'li', 'br'], | |
ALLOWED_ATTR: ['href', 'target', 'rel'] | |
}), | |
}} | |
/> |
🧰 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)
{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
Define interfaces for forum resources data.
Using proper TypeScript interfaces for the forum data structure would significantly improve code quality and maintainability.
+interface ResourceSession {
+ id: string;
+ session_title: string;
+ resource_files: SessionResourceFile[];
+}
+
+interface ForumResource {
+ id: string;
+ resource_title: string;
+ resource_sessions: ResourceSession[];
+}
// Then in the JSX:
-{selectedEvent.forum_resources?.map(
- (resource: any, resourceIndex: number) => (
+{selectedEvent.forum_resources?.map(
+ (resource: ForumResource, resourceIndex: number) => (
// And similarly:
-{resource.resource_sessions?.map(
- (session: any, sessionIndex: number) => (
+{resource.resource_sessions?.map(
+ (session: ResourceSession, sessionIndex: number) => (
📝 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> | |
), | |
)} | |
// Add these interface definitions at the top of your file (after imports) | |
interface ResourceSession { | |
id: string; | |
session_title: string; | |
resource_files: SessionResourceFile[]; | |
} | |
interface ForumResource { | |
id: string; | |
resource_title: string; | |
resource_sessions: ResourceSession[]; | |
} | |
// ...other code and imports | |
// The JSX snippet from within your component: | |
{ | |
selectedEvent.forum_resources?.map( | |
(resource: ForumResource, 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: ResourceSession, 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> | |
) | |
) | |
} |
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.
The AccordionItem component uses 'any' for props typing, which decreases type safety and code maintainability.
-const AccordionItem = ({ session, isOpen, toggleAccordion }: any) => {
+interface SessionResourceFile {
+ id: string;
+ resource_summary: string;
+ file_url: string;
+}
+
+interface Session {
+ id: string;
+ session_title: string;
+ resource_files: SessionResourceFile[];
+}
+
+interface AccordionItemProps {
+ session: Session;
+ isOpen: boolean;
+ toggleAccordion: () => void;
+}
+
+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 SessionResourceFile { | |
id: string; | |
resource_summary: string; | |
file_url: string; | |
} | |
interface Session { | |
id: string; | |
session_title: string; | |
resource_files: SessionResourceFile[]; | |
} | |
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> | |
); | |
}; |
className="text-sm text-gray-600" | ||
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.
Security risk: Unsanitized content rendering.
Using dangerouslySetInnerHTML
with renderContent(subText)
without sanitization creates an XSS vulnerability.
- <div
- className="text-sm text-gray-600"
- dangerouslySetInnerHTML={{ __html: renderContent(subText) }}
- />
+ <div
+ className="text-sm text-gray-600"
+ 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.
className="text-sm text-gray-600" | |
dangerouslySetInnerHTML={{ __html: renderContent(subText) }} | |
/> | |
<div | |
className="text-sm text-gray-600" | |
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)
<Link | ||
href={href} | ||
target="_blank" | ||
className="text-blue-600 hover:underline" | ||
> | ||
{event.title} | ||
</Link> |
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 when using target="_blank".
When opening links in a new tab, it's recommended to add rel="noopener noreferrer"
to prevent potential security vulnerabilities.
<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> | |
<Link | |
href={href} | |
target="_blank" | |
rel="noopener noreferrer" | |
className="text-blue-600 hover:underline" | |
> | |
{event.title} | |
</Link> |
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
Refactor
Style