-
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 clean air Forum Updates #2467
Conversation
📝 WalkthroughWalkthroughThis pull request introduces styling and configuration updates across multiple parts of the project. The Changes
Suggested reviewers
Poem
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
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: 0
🧹 Nitpick comments (6)
website2/src/views/cleanairforum/PaginatedSection.tsx (1)
67-67
: Consider using responsive width for better mobile experience.While the current styling works well, consider making the
min-width
responsive:-className="object-contain w-full h-full max-h-[144px] min-w-[271px] p-3 max-w-[271px] mix-blend-multiply transition-transform duration-500 ease-in-out transform hover:scale-110" +className="object-contain w-full h-full max-h-[144px] min-w-[200px] sm:min-w-[271px] p-3 max-w-[271px] mix-blend-multiply transition-transform duration-500 ease-in-out transform hover:scale-110"website2/src/app/clean-air-forum/logistics/page.tsx (1)
50-52
: Consider extracting HTML rendering logic into a reusable component.Multiple instances of
dangerouslySetInnerHTML
with DOMPurify could be consolidated into a shared component for better maintainability and consistency.+// components/SafeHTML.tsx +import DOMPurify from 'dompurify'; + +interface SafeHTMLProps { + content: string; + className?: string; +} + +export const SafeHTML: React.FC<SafeHTMLProps> = ({ content, className }) => ( + <div + className={className} + dangerouslySetInnerHTML={{ + __html: DOMPurify.sanitize(content), + }} + /> +);Also applies to: 72-74
🧰 Tools
🪛 Biome (1.9.4)
[error] 50-50: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
website2/src/app/clean-air-forum/program-committee/page.tsx (1)
19-24
: Consider adding type safety for person category filtering.Using string literals for category filtering could lead to maintenance issues.
+type PersonCategory = 'Committee Member' | 'Committee Member and Key Note Speaker' | 'Speaker and Committee Member'; + +interface Person { + id: string; + category: PersonCategory; + // ... other properties +} + const committeeMembers = useMemo( () => - data?.persons?.filter( - (person: any) => + data?.persons?.filter((person: Person) => person.category === 'Committee Member' || person.category === 'Committee Member and Key Note Speaker' || person.category === 'Speaker and Committee Member', ) || [], [data?.persons], );website2/src/app/clean-air-forum/glossary/page.tsx (1)
56-69
: Enhance accessibility for external links.Links opening in new tabs should indicate this behavior to users.
<Link href={href} target="_blank" + rel="noopener noreferrer" onClick={(e) => { e.preventDefault(); window.open(href, '_blank'); }} className={`text-blue-600 hover:underline ${ selectedEventIndex === index ? 'font-bold' : '' }`} + aria-label={`Open ${event.title} in new tab`} > {event.title} </Link>website2/src/app/clean-air-forum/resources/page.tsx (1)
114-114
: Consider adding responsive padding for better mobile experience.While the
prose
class enhances typography, consider adding responsive padding classes for improved mobile readability.- <div className="px-4 prose max-w-none lg:px-0"> + <div className="px-4 md:px-6 prose max-w-none lg:px-8">website2/src/components/ui/MemberCard.tsx (1)
62-70
: Enhance image loading with priority prop.Consider adding the
priority
prop for above-the-fold images to improve LCP (Largest Contentful Paint).<Image src={member.picture_url || member.picture || PlaceholderImage} alt={member.name} fill sizes="(max-width: 640px) 100vw, 300px" + priority={true} className="object-cover transition-transform duration-300 ease-in-out hover:scale-105" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
website2/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (17)
website2/package.json
(1 hunks)website2/src/app/clean-air-forum/glossary/page.tsx
(3 hunks)website2/src/app/clean-air-forum/logistics/page.tsx
(2 hunks)website2/src/app/clean-air-forum/partners/page.tsx
(1 hunks)website2/src/app/clean-air-forum/program-committee/page.tsx
(1 hunks)website2/src/app/clean-air-forum/resources/page.tsx
(2 hunks)website2/src/app/clean-air-forum/sessions/page.tsx
(2 hunks)website2/src/app/clean-air-forum/speakers/page.tsx
(1 hunks)website2/src/app/clean-air-forum/sponsorships/page.tsx
(1 hunks)website2/src/components/ui/MemberCard.tsx
(5 hunks)website2/src/views/Forum/AboutPage.tsx
(4 hunks)website2/src/views/Forum/SectionDisplay.tsx
(1 hunks)website2/src/views/Forum/TabNavigation.tsx
(1 hunks)website2/src/views/cleanairforum/PaginatedSection.tsx
(1 hunks)website2/src/views/cleanairforum/events/SingleEvent.tsx
(1 hunks)website2/src/views/events/SingleEvent.tsx
(1 hunks)website2/tailwind.config.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- website2/src/views/events/SingleEvent.tsx
- website2/src/views/Forum/TabNavigation.tsx
- website2/src/app/clean-air-forum/speakers/page.tsx
- website2/src/views/cleanairforum/events/SingleEvent.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
website2/src/views/Forum/SectionDisplay.tsx
[error] 33-33: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[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] 72-72: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
website2/src/app/clean-air-forum/program-committee/page.tsx
[error] 72-72: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
website2/src/components/ui/MemberCard.tsx
[error] 188-188: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
website2/src/app/clean-air-forum/sponsorships/page.tsx
[error] 49-49: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-push-website2-deploy-preview
🔇 Additional comments (16)
website2/tailwind.config.ts (1)
80-80
:❓ Verification inconclusive
LGTM! Typography plugin addition looks good.
The addition of
@tailwindcss/typography
plugin aligns well with the styling updates needed for the HTML insert sections.Let's verify that the plugin is properly installed:
🏁 Script executed:
#!/bin/bash # Check if @tailwindcss/typography is installed npm list @tailwindcss/typographyLength of output: 62
Below is an updated review comment that flags the need for further manual verification of the plugin’s installation:
Typography Plugin Installation Verification
The updated configuration for
@tailwindcss/typography
inwebsite2/tailwind.config.ts
looks great and works well with the HTML insert styling. However, the previousnpm list @tailwindcss/typography
check didn’t display any output, so it isn’t clear whether the package is properly installed.
- Action Item: Please verify that
@tailwindcss/typography
is declared in yourpackage.json
(orpackage-lock.json
) and properly installed as a dependency.- Use a command like the following to check for its entry:
grep -E '"@tailwindcss/typography":' package.json grep -E '"@tailwindcss/typography":' package-lock.jsonOnce confirmed, the configuration can confidently be approved.
website2/src/views/Forum/SectionDisplay.tsx (2)
31-33
: LGTM! Class name changes and comments look good.The addition of
prose max-w-none
classes aligns with the typography plugin, and the comments clearly explain the styling decisions.🧰 Tools
🪛 Biome (1.9.4)
[error] 33-33: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
33-33
:❓ Verification inconclusive
Verify XSS protection measures.
While
dangerouslySetInnerHTML
is being used withDOMPurify.sanitize
, let's ensure we're using the latest version with all security patches:Also applies to: 60-60, 72-72
🌐 Web query:
What is the latest version of DOMPurify and are there any known security vulnerabilities?
💡 Result:
The latest version of DOMPurify is 3.2.4, released on January 30, 2025[1][6]. This version includes critical security fixes for bypass vulnerabilities and improvements to prevent XSS attacks[1].
Known Security Vulnerabilities:
CVE-2024-45801 (Prototype Pollution)
- Impact: Allows XSS attacks via malicious HTML nesting and prototype pollution to bypass depth checks[2][3].
- Fixed in:
- v2.x branch: 2.5.4
- v3.x branch: 3.1.3[2][3].
CVE-2020-26870 (Mutation XSS)
- Impact: Serialize-parse roundtrip flaws enabling namespace changes (HTML → MathML) for XSS[8].
- Fixed in: v2.0.17[8].
Key Updates in Recent Releases:
- v3.x Branch:
- Added TypeScript support (v3.2)[1].
- Enhanced sanitizer bypass protections (v3.1–v3.2)[1][7].
- v2.x Branch:
- Backported critical fixes for older environments (e.g., IE support)[6].
Recommendations:
- Upgrade to v3.x for modern environments or v2.x if legacy browser support is required[6].
- Use
npm install dompurify@latest
oryarn add dompurify
to ensure protection against known exploits[6][10].For projects needing server-side sanitization, consider
isomorphic-dompurify
(latest: v2.20 as of January 2025)[4].Citations:
- 1: https://github.com/cure53/DOMPurify/releases
- 2: GHSA-mmhx-hmjr-r674
- 3: https://vulert.com/vuln-db/CVE-2024-45801
- 4: https://classic.yarnpkg.com/en/package/isomorphic-dompurify
- 5: https://www.cvedetails.com/product/60458/Cure53-Dompurify.html?vendor_id=20669
- 6: https://www.npmjs.com/package/dompurify
- 7: https://security.snyk.io/package/npm/dompurify
- 8: https://www.acunetix.com/vulnerabilities/web/dompurify-improper-neutralization-of-input-during-web-page-generation-cross-site-scripting-vulnerability-cve-2020-26870/
- 9: https://security.snyk.io/package/npm/dompurify/2.4.0
- 10: https://www.invicti.com/web-vulnerability-scanner/vulnerabilities/out-of-date-version-dompurify/
XSS Protection: Confirm and Upgrade DOMPurify Version
Please verify that your project is using an up-to-date DOMPurify version. According to the latest information, DOMPurify v3.2.4 (released January 30, 2025) includes critical fixes—such as the mitigation for CVE-2024-45801—so ensure that your dependency is at least on v3.1.3 or, ideally, v3.2.4. This is particularly important given the use of
dangerouslySetInnerHTML
in this file (lines 33, 60, and 72 inwebsite2/src/views/Forum/SectionDisplay.tsx
).🧰 Tools
🪛 Biome (1.9.4)
[error] 33-33: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
website2/src/app/clean-air-forum/sponsorships/page.tsx (1)
41-41
: LGTM! Improved structure and styling.The changes enhance readability with better structure and consistent typography styling using the new plugin.
Also applies to: 44-54
website2/src/app/clean-air-forum/logistics/page.tsx (1)
34-35
: LGTM! Styling update enhances readability.The addition of
prose max-w-none
class improves text styling while maintaining full width layout.website2/src/views/Forum/AboutPage.tsx (1)
58-58
: LGTM! Consistent styling improvements.The addition of
max-w-none
to prose classes maintains consistent styling across sections while preventing unwanted width constraints.Also applies to: 73-73, 87-87
website2/src/app/clean-air-forum/program-committee/page.tsx (1)
63-63
: LGTM! Styling update aligns with project standards.The addition of
prose max-w-none
class maintains consistency with other pages.website2/src/app/clean-air-forum/glossary/page.tsx (1)
39-39
: LGTM! Styling update enhances readability.The addition of
prose max-w-none
class improves text styling while maintaining layout flexibility.website2/src/app/clean-air-forum/resources/page.tsx (1)
22-22
: LGTM! Padding adjustment improves visual balance.The change from
p-4
topy-2 px-4
provides better vertical spacing while maintaining horizontal padding.website2/src/app/clean-air-forum/sessions/page.tsx (2)
38-38
: LGTM! Consistent styling with resources page.The padding adjustments maintain visual consistency across the forum pages.
107-107
: LGTM! Typography enhancement with proper sanitization.The use of
prose
class enhances readability while maintaining proper HTML sanitization with DOMPurify.website2/src/app/clean-air-forum/partners/page.tsx (1)
60-60
: LGTM! Consistent styling with other forum pages.The container styling maintains consistency while properly accommodating the partner logo grid layout.
website2/src/components/ui/MemberCard.tsx (2)
1-1
: LGTM! Proper client-side rendering directive.The 'use client' directive correctly marks this interactive component for client-side rendering.
188-190
:❓ Verification inconclusive
Verify HTML sanitization for bio content.
While
renderContent
processes the content, ensure proper sanitization before using dangerouslySetInnerHTML.
🏁 Script executed:
#!/bin/bash # Search for sanitization in renderContent implementation rg -A 5 "renderContent.*=.*" | grep -i "sanitize\|purify\|clean"Length of output: 66
Action Required: Verify HTML Sanitization in
renderContent
The code in
website2/src/components/ui/MemberCard.tsx
usesdangerouslySetInnerHTML
to display the bio content provided bymember.bio
. Our initial automated search did not find any sanitization calls (e.g., to functions or libraries likesanitize
,purify
, orclean
) within or near therenderContent
implementation.
- Next Step: Please manually verify that the
renderContent
function properly sanitizes the HTML before rendering. Check if it employs a robust sanitization mechanism (such as DOMPurify, sanitize-html, etc.) to mitigate XSS risks. If no sanitization is implemented, you should consider adding one.🧰 Tools
🪛 Biome (1.9.4)
[error] 188-188: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
website2/package.json (2)
26-26
: LGTM! Good choice adding @tailwindcss/typography.The addition of @tailwindcss/typography aligns well with the PR's objective to address styling issues in HTML insert sections. The typography plugin provides robust styling defaults that will help maintain consistent text presentation.
26-26
:✅ Verification successful
Verify Tailwind typography plugin configuration.
Please ensure the plugin is properly configured in your Tailwind config file.
Let's verify the plugin configuration:
Also, verify the latest version and any potential issues:
🏁 Script executed:
#!/bin/bash # Check if the typography plugin is properly configured in the Tailwind config rg -l "@tailwindcss/typography" website2/tailwind.config.*Length of output: 87
🌐 Web query:
What is the latest version of @tailwindcss/typography and are there any known issues?
💡 Result:
The latest version of
@tailwindcss/typography
as of February 2025 is v0.5.x, designed for compatibility with Tailwind CSS v3.x[1][6]. Here's an overview of its features and known issues:
Key Features
- Dark mode support via
prose-invert
for automatic color inversion[1]- Customizable gray scales matching Tailwind's default palettes (e.g.,
prose-slate
,prose-gray
)[1]- Improved customization API for typography elements like headings and blockquotes[1]
Known Issues
1. Font Size Customization Errors
Using
theme('fontSize.base')
directly in configuration can output[object Object]
in CSS due to array-based theme values[6].
Workaround: Access specific array indices:fontSize: theme('fontSize.base')[0]2. Build Tool Integration Problems
- Incorrect font paths when using local fonts with some build systems[2]
- Font Manager conflicts in CMS integrations like Cwicly[3]
3. CSS Specificity Conflicts
- Requires
!important
modifiers to override default styles in nested elements[10]- Text size overrides sometimes need inline styles instead of Tailwind classes[10]
4. Upgrade Challenges
- Breaking changes when migrating between major Tailwind versions (v2 → v3 → v4)[4][7]
- Deprecated utilities require manual replacement (e.g., gap utilities)[7]
The Tailwind team has acknowledged these pain points and plans to revamp the plugin's customization approach in future versions[10]. For now, developers often combine
prose
withnot-prose
classes for targeted overrides[1][10].Citations:
- 1: https://tailwindcss.com/blog/tailwindcss-typography-v0-5
- 2: https://forum.pinegrow.com/t/tailwind-local-font-with-wrong-path/7634
- 3: https://discourse.cwicly.com/t/tailwind-html-component-and-custom-fonts/5231
- 4: https://tailwindcss.com/docs/upgrading-to-v2
- 5: https://tailwindcss-typography.vercel.app
- 6: Bad DX: [object Object] appearing in CSS when customizing typography tailwindlabs/tailwindcss-typography#330
- 7: https://tailwindcss.com/docs/upgrade-guide
- 8: https://www.npmjs.com/package/tailwindcss-typography
- 9: https://dev.to/phuctm97/misunderstanding-tailwindcss-typography-costs-me-2-hours-mj7
- 10: Any plans to revamp tailwindcss-typography in 4.0 or beyond? tailwindlabs/tailwindcss#15305
Typography Plugin Configuration Verified
- The typography plugin is indeed configured in website2/tailwind.config.ts.
- The installed version (^0.5.16) aligns with the latest v0.5.x release compatible with Tailwind CSS v3.x.
- No configuration issues or known version-related concerns were detected.
New Website2 changes available for preview here |
Summary of Changes (What does this PR do?)
Status of maturity (all need to be checked before merging):
Screenshots (optional)
Summary by CodeRabbit
New Dependencies
Style
Refactor