Feat/Allow project boost if project is vouched #4858
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces updates to localization files for Catalan, English, and Spanish languages, enhancing user engagement through new keys and improved translations related to the GIVbacks program. Additionally, it modifies the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
src/components/views/project/projectActionCard/ProjectCardNotification.tsx (4)
18-19: Rename condition for clarity.The variable name
isOwnerVerifiedNotEligiblesuggests checking ownership, but it's actually checking admin status. Consider renaming toisAdminVerifiedNotEligiblefor better clarity.- const isOwnerVerifiedNotEligible = + const isAdminVerifiedNotEligible = isVerified && isAdmin && !isGivbackEligible;
13-16: Add TypeScript types for project data.Consider adding proper TypeScript types for the project data to ensure type safety and better maintainability.
+interface ProjectData { + verified: boolean; + isGivbackEligible: boolean; +} - const { isAdmin, projectData } = useProjectContext(); + const { isAdmin, projectData }: { isAdmin: boolean; projectData?: ProjectData } = useProjectContext();
29-31: Consider using type-safe message IDs.The message ID is hardcoded as a string. Consider using a type-safe approach with a messages constant or enum to prevent typos and enable better refactoring support.
+const MESSAGES = { + COMPLETE_ELIGIBILITY: 'project.givback_toast.complete_eligibility' +} as const; {formatMessage({ - id: `project.givback_toast.complete_eligibility`, + id: MESSAGES.COMPLETE_ELIGIBILITY, })}
37-45: Consider adding responsive design support.The styling is clean, but consider adding responsive design support for different screen sizes, especially since this component is used in both desktop and mobile views (as mentioned in the AI summary).
const ProjectCardNotificationWrapper = styled(Flex)` align-items: center; padding-top: 12px; font-size: 14px; color: ${semanticColors.golden[600]}; + + @media (max-width: 768px) { + padding-top: 8px; + font-size: 12px; + } + span { margin-left: 8px; + + @media (max-width: 768px) { + margin-left: 6px; + } } `;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
lang/ca.json(1 hunks)lang/en.json(1 hunks)lang/es.json(1 hunks)src/components/views/project/ProjectGIVbackToast.tsx(2 hunks)src/components/views/project/projectActionCard/AdminActions.tsx(2 hunks)src/components/views/project/projectActionCard/ProjectCardNotification.tsx(1 hunks)
🔇 Additional comments (8)
src/components/views/project/projectActionCard/ProjectCardNotification.tsx (2)
1-9: LGTM! Well-organized imports.
The imports are properly structured, following the convention of external dependencies first, followed by internal imports.
11-35: Verify component integration in AdminActions.
The component implementation looks good, but let's verify its integration in the AdminActions component.
✅ Verification successful
Component is properly integrated in AdminActions
The verification shows that ProjectCardNotification is correctly:
- Imported in AdminActions.tsx
- Used in both mobile and desktop views of the component
- Properly handles GIVback eligibility notifications as designed
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify ProjectCardNotification usage in AdminActions
# Expected: Find import and usage of ProjectCardNotification in AdminActions
# Search for the component usage
rg -l "ProjectCardNotification" "src/components/views/project/projectActionCard/AdminActions"
Length of output: 191
Script:
#!/bin/bash
# Let's first find the correct path for AdminActions
fd -t f "AdminActions" src/
# Then search for ProjectCardNotification usage across the codebase
rg -l "ProjectCardNotification"
# Get the content of files that use ProjectCardNotification to understand the integration
rg -A 5 "ProjectCardNotification"
Length of output: 4120
src/components/views/project/projectActionCard/AdminActions.tsx (2)
34-34: LGTM: Clean import statement.
The import follows React conventions and maintains good code organization by importing from the same directory.
167-167: Verify props requirements for ProjectCardNotification.
The component is rendered without any props. Please verify if this is intentional and that the component can function correctly without any required props.
✅ Verification successful
ProjectCardNotification is correctly used without props
The component is properly implemented to work without props as it uses context hooks (useProjectContext) to access required data. It internally:
- Gets project data and admin status from
useProjectContext - Uses
useIntlfor internationalization - Has conditional rendering logic based on verification and eligibility status
- Returns null when conditions aren't met
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if ProjectCardNotification requires any props
# Test: Search for the component definition to verify prop requirements
ast-grep --pattern 'interface $_ {
$$$
}' src/components/views/project/projectActionCard/ProjectCardNotification.tsx
# Test: Search for prop usage within the component
ast-grep --pattern 'const ProjectCardNotification = ($props) => {
$$$
}' src/components/views/project/projectActionCard/ProjectCardNotification.tsx
Length of output: 272
Script:
#!/bin/bash
# Let's try different approaches to verify the component implementation
# First, let's check if the file exists and see its content
fd ProjectCardNotification.tsx | xargs cat
# If that doesn't work, let's try finding any file containing ProjectCardNotification
rg -l "ProjectCardNotification"
# Let's also search for any usage of this component to understand the patterns
rg "ProjectCardNotification" -A 3
Length of output: 3958
src/components/views/project/ProjectGIVbackToast.tsx (1)
56-56: LGTM: Variable declaration change is appropriate.
The change from const to let is necessary as the color value is reassigned in the new conditional block.
lang/en.json (1)
1682-1682: LGTM! New translation key added for GIVbacks eligibility form.
The new key follows proper naming conventions and provides a clear message for users to complete their GIVbacks eligibility form.
lang/es.json (1)
1682-1682: LGTM! New translation key added correctly
The Spanish translation for the new GIVbacks eligibility form message is grammatically correct and maintains consistency with the existing translations.
lang/ca.json (1)
1681-1681: LGTM! New translation key added correctly.
The new translation key "project.givback_toast.complete_eligibility" with Catalan translation "Completa el teu formulari d'elegibilitat per als GIVbacks" follows proper naming conventions, is well formatted, and maintains consistency with other GIVbacks related messages.
| <> | ||
| <MobileWrapper | ||
| gap='4px' | ||
| onClick={() => setShowMobileActionsModal(true)} | ||
| > | ||
| <div>Project Actions</div> | ||
| <IconChevronDown24 /> | ||
| {showMobileActionsModal && ( | ||
| <MobileActionsModal | ||
| setShowModal={setShowMobileActionsModal} | ||
| > | ||
| {options.map(option => ( | ||
| <MobileActionModalItem | ||
| key={option.label} | ||
| onClick={option.cb} | ||
| > | ||
| <Flex gap='8px'> | ||
| <Flex $alignItems='center'> | ||
| {option.icon} | ||
| </Flex> | ||
| <div>{option.label}</div> | ||
| </Flex> | ||
| </MobileActionModalItem> | ||
| ))} | ||
| {showVerificationModal && ( | ||
| <VerificationModal | ||
| onClose={() => setShowVerificationModal(false)} | ||
| /> | ||
| )} | ||
| {deactivateModal && ( | ||
| <DeactivateProjectModal | ||
| setShowModal={setDeactivateModal} | ||
| projectId={projectData?.id} | ||
| onSuccess={fetchProjectBySlug} | ||
| /> | ||
| )} | ||
| {showShareModal && ( | ||
| <ShareModal | ||
| contentType={EContentType.thisProject} | ||
| setShowModal={setShowShareModal} | ||
| projectHref={slug} | ||
| /> | ||
| )} | ||
| </MobileActionsModal> | ||
| )} | ||
| {showClaimModal && ( | ||
| <ClaimRecurringDonationModal | ||
| setShowModal={setShowClaimModal} | ||
| project={project} | ||
| /> | ||
| )} | ||
| </MobileWrapper> | ||
| <ProjectCardNotification /> | ||
| </> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve code structure and accessibility.
Several issues need to be addressed in the mobile view implementation:
- The click handler is attached to a div element which may cause accessibility issues.
- The modal components are duplicated between desktop and mobile views.
Consider these improvements:
- <MobileWrapper
- gap='4px'
- onClick={() => setShowMobileActionsModal(true)}
- >
- <div>Project Actions</div>
+ <MobileWrapper
+ gap='4px'
+ as="button"
+ onClick={() => setShowMobileActionsModal(true)}
+ role="button"
+ aria-label="Open project actions"
+ >
+ <span>Project Actions</span>Also, consider extracting the modal components to avoid duplication:
const ActionModals = () => (
<>
{showVerificationModal && (
<VerificationModal onClose={() => setShowVerificationModal(false)} />
)}
{deactivateModal && (
<DeactivateProjectModal
setShowModal={setDeactivateModal}
projectId={projectData?.id}
onSuccess={fetchProjectBySlug}
/>
)}
{/* ... other modals ... */}
</>
);| @@ -163,55 +164,63 @@ | |||
| project={project} | |||
| /> | |||
| )} | |||
| <ProjectCardNotification /> | |||
There was a problem hiding this comment.
Prevent duplicate notifications on desktop/tablet views.
The ProjectCardNotification component is rendered in both desktop and mobile views. This could lead to duplicate notifications being displayed when transitioning between viewport sizes.
Consider consolidating the notification rendering:
return !isMobile ? (
<Wrapper>
<Dropdown
style={dropdownStyle}
label='Project Actions'
options={options}
/>
{/* ... modals ... */}
- <ProjectCardNotification />
</Wrapper>
) : (
<>
<MobileWrapper>
{/* ... mobile content ... */}
</MobileWrapper>
<ProjectCardNotification />
</>
);And move it outside the conditional render:
return (
<>
{!isMobile ? (
<Wrapper>
<Dropdown
style={dropdownStyle}
label='Project Actions'
options={options}
/>
{/* ... modals ... */}
</Wrapper>
) : (
<MobileWrapper>
{/* ... mobile content ... */}
</MobileWrapper>
)}
<ProjectCardNotification />
</>
);Also applies to: 222-222
| } else if (isOwnerVerifiedNotEligible) { | ||
| title = formatMessage( | ||
| { | ||
| id: `${useIntlTitle}verified_owner`, | ||
| }, | ||
| { | ||
| percent: givbackFactorPercent, | ||
| value: GIVBACKS_DONATION_QUALIFICATION_VALUE_USD, | ||
| }, | ||
| ); | ||
| description = formatMessage({ | ||
| id: `${useIntlDescription}verified_owner_not_eligible`, | ||
| }); | ||
| color = semanticColors.golden[600]; | ||
| icon = <IconGIVBack24 color={semanticColors.golden[600]} />; | ||
| link = links.GIVPOWER_DOC; | ||
| Button = ( | ||
| <OutlineButton | ||
| onClick={handleBoostClick} | ||
| label='Boost' | ||
| icon={<IconRocketInSpace16 />} | ||
| /> | ||
| ); |
There was a problem hiding this comment.
Fix duplicate condition and improve code organization.
There are several issues with the new conditional block:
- This condition appears twice in the code (here and later in the file), making the second occurrence unreachable.
- The Button implementation is duplicated.
- The color is inconsistently assigned (both to a variable and directly to the icon).
Consider consolidating the conditions and removing duplicates:
- } else if (isOwnerVerifiedNotEligible) {
- title = formatMessage(
- {
- id: `${useIntlTitle}verified_owner`,
- },
- {
- percent: givbackFactorPercent,
- value: GIVBACKS_DONATION_QUALIFICATION_VALUE_USD,
- },
- );
- description = formatMessage({
- id: `${useIntlDescription}verified_owner_not_eligible`,
- });
- color = semanticColors.golden[600];
- icon = <IconGIVBack24 color={semanticColors.golden[600]} />;
- link = links.GIVPOWER_DOC;
- Button = (
- <OutlineButton
- onClick={handleBoostClick}
- label='Boost'
- icon={<IconRocketInSpace16 />}
- />
- );Move this logic to where the other isOwnerVerifiedNotEligible condition is handled and ensure consistent color usage:
} else if (isOwnerVerifiedNotEligible) {
color = semanticColors.golden[600];
title = formatMessage({
id: `${useIntlTitle}verified_owner_not_eligible`,
});
description = formatMessage(
{
id: `${useIntlDescription}verified_owner_not_eligible`,
},
{
stakeLock: (
<InnerLink href={Routes.GIVfarm} target='_blank'>
{formatMessage({ id: 'label.stake_and_lock' })}{' '}
</InnerLink>
),
},
);
icon = <IconGIVBack24 color={color} />;
link = links.GIVPOWER_DOC;
Button = (
<OutlineButton
onClick={handleBoostClick}
label='Boost'
icon={<IconRocketInSpace16 />}
/>
);
Summary by CodeRabbit
Release Notes
New Features
ProjectCardNotificationcomponent to inform users about project eligibility for GIVbacks.Bug Fixes
Refactor
AdminActionscomponent to include a new notification feature while maintaining existing functionalities.