-
-
Notifications
You must be signed in to change notification settings - Fork 34
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
quadratic funding toast on donate success page #4820
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes in this pull request involve updates to language files for Catalan, English, and Spanish, enhancing the clarity and effectiveness of user interface strings related to donation processes and eligibility notifications. Additionally, modifications were made to several components, including the addition of new fields in GraphQL queries, adjustments to modal logic, and refinements to state management in donation handling. These changes aim to improve user experience by ensuring accurate and engaging communication throughout the Giveth platform. Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🔇 Additional comments (10)
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: 4
🧹 Outside diff range and nitpick comments (4)
src/components/views/donate/QFToast.tsx (1)
82-93
: LGTM with a suggestion: Updated button rendering for eligible users.The addition of a "Back to Project" button for eligible users improves navigation. However, consider the following suggestion:
- The
IconExternalLink16
icon might be misleading as it suggests an external link, while the navigation is internal. Consider using a more appropriate icon or removing it altogether.lang/ca.json (3)
1712-1713
: Great addition of QF donor eligibility translations!The new translations under the "profile.qf_donor_eligibility" key provide clear information about donor eligibility for Quadratic Funding, including Gitcoin Passport integration. This will help Catalan-speaking users understand their eligibility status.
Consider adding a translation for "Increase your score" in line 1712. It currently appears in English:
"profile.qf_donor_eligibility.incease_your_score": "Increase your score",Suggested translation:
"profile.qf_donor_eligibility.incease_your_score": "Augmenta la teva puntuació",
Line range hint
1815-1824
: Valuable additions to toast and tooltip translations!The new translations under the "toast" and "tooltip" keys provide important information about stream balance operations, donation matching, and other relevant features. These clear and concise messages will help Catalan-speaking users better understand the platform's functionality.
In the tooltip translation for "tooltip.flowrate", consider replacing "prenen" with "es prenen" for better clarity:
"tooltip.flowrate": "Les donacions recurrents es prenen del teu Saldo de Transmissió. Diposita tokens i mantén el teu saldo per habilitar les donacions recurrents.",Suggested change:
"tooltip.flowrate": "Les donacions recurrents es prenen del teu Saldo de Transmissió. Diposita tokens i mantén el teu saldo per habilitar les donacions recurrents.",
Line range hint
1-1825
: Overall excellent improvements to Catalan translations!The changes in this file significantly enhance the Catalan language support for the platform. The new translations cover a wide range of features and user interactions, including:
- Donation-related messages and UI elements
- Quadratic Funding (QF) donor eligibility information
- Detailed project status and GIVbacks-related messages
- Toast notifications and tooltips for various platform features
These additions will greatly improve the user experience for Catalan-speaking users, making the platform more accessible and user-friendly.
To ensure consistency and completeness of the Catalan translations:
- Consider reviewing the entire file for any remaining untranslated strings or inconsistencies in terminology.
- If possible, have a native Catalan speaker or professional translator review the translations to ensure natural language flow and accuracy.
Great job on expanding the language support for the platform!
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
- lang/ca.json (2 hunks)
- lang/en.json (2 hunks)
- lang/es.json (2 hunks)
- src/apollo/gql/gqlQF.ts (1 hunks)
- src/components/modals/PassportModal.tsx (1 hunks)
- src/components/views/donate/DonateIndex.tsx (1 hunks)
- src/components/views/donate/OneTime/DonateModal.tsx (1 hunks)
- src/components/views/donate/QFToast.tsx (4 hunks)
🧰 Additional context used
🪛 GitHub Check: build
src/components/modals/PassportModal.tsx
[failure] 299-299:
ReplacepassportScore·??·'--'
with(passportScore·??·'--')
🔇 Additional comments (15)
src/components/views/donate/QFToast.tsx (5)
13-13
: LGTM: New import for routing functionality.The addition of
useRouter
from Next.js is appropriate for implementing client-side navigation in this component.
23-23
: LGTM: Router initialization.The
useRouter
hook is correctly used to initialize the router, following Next.js best practices.
51-58
: LGTM: Updated description logic for eligible users.The changes improve the clarity of the message by including the specific minimum valid USD value for the current round. The addition of a period at the end enhances readability.
62-69
: LGTM: Improved description logic for non-eligible users.The changes enhance code readability and localization flexibility:
- Use of template literals improves code clarity.
- Passing
minimumValidUsdValue
as a parameter to the translation function allows for more flexible localization.
Line range hint
1-24
: Overall assessment: Improvements to QFToast component.The changes in this file enhance the QFToast component by:
- Adding client-side routing capabilities.
- Improving the clarity of messages for both eligible and non-eligible users.
- Providing better navigation options for eligible users.
These modifications align well with the PR objectives of updating the quadratic funding toast on the donate success page.
Also applies to: 51-58, 62-69, 82-93
lang/en.json (5)
1355-1356
: Update to eligibility message for QF donationsThe message has been updated to provide more specific information about QF eligibility. This change improves clarity for users.
1357-1357
: Clarification on non-eligible donationsThe message now includes a specific dollar amount for eligible donations and instructions for verifying QF Eligibility. This change provides more actionable information to users.
1360-1360
: Simplified title for non-eligible donationsThe title has been changed from "Quadratic Funding" to "Don't miss out!" This change makes the message more engaging and action-oriented.
1713-1713
: New navigation option addedA new "Back to projects" link has been added. This improves navigation and user experience by providing an easy way to return to the projects list.
Line range hint
1355-1713
: Overall improvements to QF-related messagesThe changes in this file focus on improving the clarity and actionability of messages related to quadratic funding (QF) and eligibility. The updates provide more specific information, clearer instructions, and better navigation options for users. These changes should enhance the user experience and understanding of the QF process.
lang/es.json (3)
1355-1357
: Updated translations for passport toast messagesThe translations for passport toast messages have been updated. The changes appear to be improvements in clarity and accuracy.
1713-1713
: New translation added for QF donor eligibility bannerA new translation has been added for the "Back to projects" link in the QF donor eligibility banner. This addition improves the user experience for Spanish-speaking users.
Line range hint
1-1716
: Overall assessment of changesThe modifications to this Spanish translation file are minor but beneficial. They improve the clarity of some messages, particularly in the passport toast section, and add a missing translation for the QF donor eligibility banner. These changes will enhance the user experience for Spanish-speaking users of the Giveth platform.
lang/ca.json (2)
1355-1360
: Excellent additions to the donation-related translations!The new translations under the "page.donate" key provide clear and informative messages about donation eligibility, matching funds, and Quadratic Funding (QF). These additions will improve the user experience for Catalan-speaking users interacting with the donation features.
Line range hint
1714-1749
: Comprehensive additions to project-related translations!The new translations under the "project.givback_toast" key provide detailed and clear messages for various project states and GIVbacks-related information. These additions will greatly improve the user experience for Catalan-speaking project owners and donors, helping them understand project status, eligibility, and actions they can take.
The translations cover a wide range of scenarios, including:
- Non-verified project states (draft, incomplete, submitted, rejected)
- Verified project states
- GIVbacks eligibility information
- Project boosting suggestions
This comprehensive set of translations will ensure that Catalan users have access to all important project-related information.
@@ -18,6 +18,7 @@ export const QF_ROUNDS_QUERY = ` | |||
allocatedFundUSDPreferred | |||
allocatedTokenSymbol | |||
minMBDScore | |||
minimumValidUsdValue |
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.
💡 Codebase verification
Issue Detected: Server-Side Implementation Missing
The minimumValidUsdValue
field is added to the QF_ROUNDS_QUERY
on the client side but does not appear to be defined in the GraphQL schema or resolvers. This discrepancy can lead to runtime errors and data inconsistencies.
Actions Needed:
- Define
minimumValidUsdValue
in the GraphQL schema. - Implement the corresponding resolver to handle this field.
🔗 Analysis chain
LGTM. Please provide additional context.
The addition of the minimumValidUsdValue
field to the QF_ROUNDS_QUERY
looks good. This change appears to be related to implementing a minimum donation amount in USD for Quadratic Funding rounds.
To ensure completeness:
- Could you confirm if corresponding changes have been made to the GraphQL schema and resolver?
- How does this new field relate to the "quadratic funding toast on donate success page" mentioned in the PR title?
- Are there any client-side changes that utilize this new field?
Let's verify if there are any client-side changes related to this new field:
[/run_scripts]
[/run_scripts]
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for client-side usage of the new minimumValidUsdValue field
# Test: Search for minimumValidUsdValue in TypeScript and JavaScript files
rg --type-add 'frontend:*.{ts,tsx,js,jsx}' -t frontend 'minimumValidUsdValue'
Length of output: 941
Script:
#!/bin/bash
# Description: Verify the addition of `minimumValidUsdValue` in the GraphQL schema and resolvers
# Search for the definition of `minimumValidUsdValue` in GraphQL schema files
rg --type graphql 'minimumValidUsdValue'
# Search for resolver implementations related to `minimumValidUsdValue`
rg --type ts 'resolve.*minimumValidUsdValue'
Length of output: 119
Script:
#!/bin/bash
# Description: Verify the addition of `minimumValidUsdValue` in the GraphQL schema and resolvers
# Search for the definition of `minimumValidUsdValue` in GraphQL schema files with .graphql and .gql extensions
rg 'minimumValidUsdValue' --glob '*.graphql' --glob '*.gql'
# Search for resolver implementations related to `minimumValidUsdValue` in TypeScript files
rg 'resolve.*minimumValidUsdValue' --type ts
Length of output: 255
activeStartedRound && | ||
isOnEligibleNetworks && | ||
donationUsdValue >= (activeStartedRound.minimumValidUsdValue || 0); | ||
const includeInQF = activeStartedRound && isOnEligibleNetworks; |
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
Simplified QF inclusion logic - Please clarify the criteria
The condition for including a donation in the Quadratic Funding (QF) round has been simplified, which improves code readability. However, this change raises a few points:
- The removal of the
donationUsdValue
check means that all donations on eligible networks will be included in QF, regardless of their USD value. Is this intentional? - If there's a minimum threshold for QF inclusion, it should be implemented to avoid including very small donations.
Consider adding a comment explaining the criteria for including donations in QF. If a minimum threshold is needed, you could modify the line as follows:
const includeInQF = activeStartedRound && isOnEligibleNetworks && donationUsdValue >= MINIMUM_QF_THRESHOLD;
Where MINIMUM_QF_THRESHOLD
is a constant defined elsewhere in the codebase.
@@ -296,7 +296,7 @@ | |||
size={10} | |||
/> | |||
) : ( | |||
(passportScore ?? '--') | |||
passportScore ?? '--' |
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 parentheses around nullish coalescing operation to fix syntax error
At line 299, the expression passportScore ?? '--'
should be enclosed in parentheses to resolve a syntax issue due to operator precedence in JSX.
Apply this diff to fix the syntax error:
- passportScore ?? '--'
+ (passportScore ?? '--')
📝 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.
passportScore ?? '--' | |
(passportScore ?? '--') |
🧰 Tools
🪛 GitHub Check: build
[failure] 299-299:
ReplacepassportScore·??·'--'
with(passportScore·??·'--')
activeStartedRound && !!getDonationById.valueUsd; | ||
setSuccessDonation({ |
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.
Potential Issue: Inclusion Criteria for Quadratic Funding Eligibility May Have Changed
The updated condition for includeInQF
now only checks for the existence of getDonationById.valueUsd
:
const includeInQF =
activeStartedRound && !!getDonationById.valueUsd;
Previously, the condition might have included a check against a minimum donation amount to qualify for Quadratic Funding (QF). By only verifying the presence of valueUsd
, donations with minimal amounts could unintentionally become eligible for QF, which may not be desired.
If the intention is to include only donations above a certain minimum value, consider updating the condition accordingly. For example:
const includeInQF =
activeStartedRound && !!getDonationById.valueUsd
+ && getDonationById.valueUsd >= GIVBACKS_DONATION_QUALIFICATION_VALUE_USD;
This ensures that only donations meeting the minimum USD value qualify for QF inclusion.
…te_success_page_quadratic_funding Update Branch
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.
Thanks @HrithikSampson
relates to #4539
Summary by CodeRabbit
New Features
Bug Fixes
Improvements