website: UI updates in relation to backend changes #2304
Conversation
📝 WalkthroughWalkthroughThe pull request introduces several updates across multiple components in the website application. Key changes include enhancements to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
website2/src/app/clean-air-forum/layout.tsx (1)
15-52: Consider removing the commented-out metadata to keep the code cleanThe large block of commented-out code can make the file less readable. If the metadata is no longer needed, it's better to remove it entirely to maintain clarity.
website2/src/app/clean-air-forum/resources/page.tsx (1)
9-13: Consider refining error handling in 'getFileNameFromUrl'While the added type checks enhance robustness, logging errors to the console might not be ideal in production environments. Consider using a logging framework or handling the error differently.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
website2/src/app/(about)/events/[id]/SingleEvent.tsx(2 hunks)website2/src/app/clean-air-forum/layout.tsx(3 hunks)website2/src/app/clean-air-forum/partners/page.tsx(1 hunks)website2/src/app/clean-air-forum/resources/page.tsx(1 hunks)website2/src/app/clean-air-network/events/[id]/SingleEvent.tsx(2 hunks)website2/src/components/sections/Forum/BannerSection.tsx(1 hunks)website2/src/components/ui/MemberCard.tsx(1 hunks)
🔇 Additional comments (6)
website2/src/components/sections/Forum/BannerSection.tsx (1)
43-43: Update aligns with consistent naming conventions
Changing background_image to background_image_url improves clarity and consistency in how image URLs are referenced across the codebase.
website2/src/app/clean-air-forum/partners/page.tsx (1)
21-21: Consistent use of 'partner_logo_url' enhances clarity
Updating partner.partner_logo to partner.partner_logo_url ensures consistency in naming conventions for URL properties, improving code readability.
Also applies to: 29-29, 36-36, 44-44
website2/src/components/ui/MemberCard.tsx (2)
Line range hint 24-25: Optional props 'btnText' and 'cardClassName' enhance flexibility
Adding btnText and cardClassName as optional properties makes the MemberCard component more customizable and adaptable to different use cases.
59-59: Ensure 'member.picture_url' is reliably populated
With the removal of member.picture as a fallback, make sure that member.picture_url is consistently provided to avoid missing images.
Run the following script to check for any remaining usages of member.picture:
website2/src/app/clean-air-network/events/[id]/SingleEvent.tsx (1)
7-12: LGTM: Clean icon imports
The new icon imports are well-organized and properly grouped.
website2/src/app/(about)/events/[id]/SingleEvent.tsx (1)
350-376: Apply the same security enhancements as the other SingleEvent component
The changes in this file are identical to those in website2/src/app/clean-air-network/events/[id]/SingleEvent.tsx. Please apply the same security enhancements suggested in the previous review.
| }: CleanAirLayoutProps) { | ||
| let data = null; | ||
| const CleanAirLayout: React.FC<CleanAirLayoutProps> = ({ children }) => { | ||
| const [data, setData] = useState<any>(null); // Replace `any` with your actual data type |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Specify the exact data type instead of using 'any'
Using any reduces the benefits of TypeScript's type safety. Replacing it with the actual data type of data will enhance reliability and maintainability.
| className="flex justify-between items-center p-4 bg-gray-50 rounded-lg shadow-md hover:shadow-lg transition-shadow duration-200" | ||
| > | ||
| <div> | ||
| <h3 className="text-lg font-medium">{resource.title}</h3> | ||
| {resource.link && ( | ||
| <a | ||
| href={resource.link} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="text-blue-500 hover:underline" | ||
| > | ||
| View Link | ||
| </a> | ||
| )} | ||
| {resource.resource && ( | ||
| <a | ||
| href={resource.resource} | ||
| download | ||
| className="text-blue-500 hover:underline ml-4" | ||
| > | ||
| Download | ||
| </a> | ||
| )} | ||
| <h3 className="text-xl font-semibold mb-2 text-gray-800"> | ||
| {resource.title} | ||
| </h3> | ||
| <div className="flex space-x-4"> | ||
| {resource.link && ( | ||
| <a | ||
| href={resource.link} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="inline-flex items-center text-blue-600 hover:underline font-medium" | ||
| > | ||
| <FiExternalLink className="mr-2" /> View Link | ||
| </a> | ||
| )} | ||
| {resource.resource_url && ( | ||
| <a | ||
| href={resource.resource_url} | ||
| download | ||
| className="inline-flex items-center text-blue-600 hover:underline font-medium" | ||
| > | ||
| <FiDownload className="mr-2" /> Download | ||
| </a> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
Consider enhancing security for external links and downloads
While the UI improvements look great, there are some security considerations:
- External links should include additional security attributes
- Download links should validate file types
Apply this diff to enhance security:
- target="_blank"
- rel="noopener noreferrer"
+ target="_blank"
+ rel="noopener noreferrer nofollow"
+ className="inline-flex items-center text-blue-600 hover:underline font-medium"- download
+ download
+ onClick={(e) => {
+ const fileExtension = resource.resource_url.split('.').pop()?.toLowerCase();
+ const allowedTypes = ['pdf', 'doc', 'docx', 'xls', 'xlsx'];
+ if (!allowedTypes.includes(fileExtension)) {
+ e.preventDefault();
+ alert('Invalid file type');
+ }
+ }}Committable suggestion skipped: line range outside the PR's diff.
|
New Website2 changes available for preview here |
Summary of Changes (What does this PR do?)
ISSUE: [WEB-458]
Status of maturity (all need to be checked before merging):
Screenshots (optional)
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Chores