-
-
Notifications
You must be signed in to change notification settings - Fork 27
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: New page to present tech stack #976
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughA new tech stack page has been added to the Social Income website, featuring a comprehensive overview of the technologies and tools used by the organization. The implementation includes a hero section, a tabbed tech list interface, and a JSON configuration file that defines the tech stack details. The page allows users to view all technologies and filter specifically for donated technologies. Changes
Sequence DiagramsequenceDiagram
participant User
participant TechStackPage
participant Hero
participant TechList
participant TechCard
User->>TechStackPage: Access Tech Stack Page
TechStackPage->>Hero: Render Hero Section
TechStackPage->>TechList: Render Tech List
TechList->>TechCard: Create Tech Cards
User->>TechList: Switch Tabs
TechList->>TechCard: Filter Tech Cards
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 (
|
Visit the preview URL for this PR (updated for commit 1cab6a4): https://si-admin-staging--pr976-lvonlanthen-techstac-hejgjn6x.web.app (expires Tue, 24 Dec 2024 21:00:17 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: b7b0969384059dce6ea8fad1ee1d1737e54e6676 |
It was necessary to replace `Translator.getInstance` with `useTranslator` and add `'use client'`, otherwise there were issues with client/server components and async/await. Not sure if this is the correct way to solve this, but it seems to work.
448dd78
to
a5b0e02
Compare
@coderabbitai review |
✅ Actions performedReview triggered.
|
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 (3)
website/src/app/[lang]/[region]/(website)/techstack/(sections)/techlist.tsx (1)
9-16
: Enhance type safety for tech entry interface.The current type definition could be more strict to prevent potential runtime errors.
Consider this enhanced type definition:
type TechEntryJSON = { - title: string; - description: string; - link: string; - logo: string; - donated: boolean; + readonly title: string; + readonly description: string; + readonly link: string; + readonly logo: string; + readonly donated: boolean; };website/src/app/[lang]/[region]/(website)/techstack/(sections)/techcard.tsx (2)
5-16
: Consider adding type constraints for URLs and image files.The type definitions could be more specific to prevent invalid inputs.
type TechCardProps = { title: string; description: string; - link: string; - logo: string; + link: `https://${string}` | `http://${string}`; + logo: `${string}.${'png' | 'jpg' | 'svg' | 'webp'}`; donated: boolean; translations: TechCardTranslations; };
39-39
: Simplify the className conditional.The double negation is unnecessary and can be simplified.
-<div className={'' + (!!logo ? 'w-fit basis-3/4' : '')}> +<div className={logo ? 'w-fit basis-3/4' : ''}>🧰 Tools
🪛 Biome (1.9.4)
[error] 39-39: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
website/public/assets/tech/google-nonprofit.png
is excluded by!**/*.png
📒 Files selected for processing (5)
shared/locales/en/website-techstack.json
(1 hunks)website/src/app/[lang]/[region]/(website)/techstack/(sections)/hero.tsx
(1 hunks)website/src/app/[lang]/[region]/(website)/techstack/(sections)/techcard.tsx
(1 hunks)website/src/app/[lang]/[region]/(website)/techstack/(sections)/techlist.tsx
(1 hunks)website/src/app/[lang]/[region]/(website)/techstack/page.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- shared/locales/en/website-techstack.json
🧰 Additional context used
🪛 Biome (1.9.4)
website/src/app/[lang]/[region]/(website)/techstack/(sections)/techcard.tsx
[error] 39-39: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
🔇 Additional comments (1)
website/src/app/[lang]/[region]/(website)/techstack/page.tsx (1)
5-12
: LGTM! Clean and well-structured page component.
The implementation follows Next.js app router conventions and properly handles internationalization parameters.
{techArray | ||
?.filter((t) => !isDonated || t.donated) | ||
.map((techEntry, index) => ( | ||
<TechCard | ||
{...techEntry} | ||
translations={{ badgeDonated: translator?.t('badges.donated') || '' }} | ||
key={index} | ||
/> | ||
))} | ||
</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.
Add null checks and loading state handling.
The code assumes techArray and translator are always available, which might not be true during initial render or translation loading.
Add proper checks and loading state:
+ if (!translator || !techArray) {
+ return <div>Loading...</div>;
+ }
<div className="grid grid-cols-1 gap-4 p-4 sm:grid-cols-2 lg:grid-cols-2">
{techArray
- ?.filter((t) => !isDonated || t.donated)
+ .filter((t) => !isDonated || t.donated)
.map((techEntry, index) => (
<TechCard
{...techEntry}
- translations={{ badgeDonated: translator?.t('badges.donated') || '' }}
+ translations={{ badgeDonated: translator.t('badges.donated') }}
key={index}
/>
))}
</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.
{techArray | |
?.filter((t) => !isDonated || t.donated) | |
.map((techEntry, index) => ( | |
<TechCard | |
{...techEntry} | |
translations={{ badgeDonated: translator?.t('badges.donated') || '' }} | |
key={index} | |
/> | |
))} | |
</div> | |
if (!translator || !techArray) { | |
return <div>Loading...</div>; | |
} | |
<div className="grid grid-cols-1 gap-4 p-4 sm:grid-cols-2 lg:grid-cols-2"> | |
{techArray | |
.filter((t) => !isDonated || t.donated) | |
.map((techEntry, index) => ( | |
<TechCard | |
{...techEntry} | |
translations={{ badgeDonated: translator.t('badges.donated') }} | |
key={index} | |
/> | |
))} | |
</div> |
<Image | ||
src={getLogoSrc(logo)} | ||
alt={title} | ||
className="mx-auto w-1/2 rounded-sm sm:w-full" | ||
width="48" | ||
height="48" | ||
unoptimized | ||
/> |
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
Enable image optimization for better performance.
The unoptimized
prop bypasses Next.js's built-in image optimization. Consider removing it to benefit from automatic image optimization.
<Image
src={getLogoSrc(logo)}
alt={title}
className="mx-auto w-1/2 rounded-sm sm:w-full"
width="48"
height="48"
- unoptimized
/>
📝 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.
<Image | |
src={getLogoSrc(logo)} | |
alt={title} | |
className="mx-auto w-1/2 rounded-sm sm:w-full" | |
width="48" | |
height="48" | |
unoptimized | |
/> | |
<Image | |
src={getLogoSrc(logo)} | |
alt={title} | |
className="mx-auto w-1/2 rounded-sm sm:w-full" | |
width="48" | |
height="48" | |
/> |
Summary by CodeRabbit
New Features
Hero
component to showcase titles and subtitles related to the tech stack.TechCard
component to display individual technology details.TechList
component with a tabbed interface to filter technologies.Hero
andTechList
components.Bug Fixes