-
-
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
Feat/add best match sort item #4761
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces significant updates across multiple language JSON files, enhancing user interaction and feedback related to donation processes. New labels and error messages are added for clarity, particularly around donation amounts and project management. Additionally, a new sorting option, "BestMatch," is introduced in the project search functionality, affecting various components and helper functions. Changes
Possibly related PRs
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 (
|
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 @MohammadPCh LGTM
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: 5
Outside diff range and nitpick comments (5)
src/apollo/types/gqlEnums.ts (1)
24-24
: Consider if additional changes are needed for full implementation.While adding the 'BestMatch' enum value is a good start, it's worth considering if this change alone is sufficient to fully implement the "best match" sorting functionality. Depending on the implementation details, you might need to:
- Update the GraphQL schema to include this new sorting option.
- Implement the sorting logic on the backend (likely in the impact-graph repository).
- Update any UI components that use the EProjectsSortBy enum to include this new option.
- Add or update tests to cover the new sorting functionality.
Could you please confirm if these additional changes are planned or if they exist in separate PRs? If not, it might be beneficial to include them in this PR or create follow-up tasks to ensure complete implementation of the feature.
src/components/views/projects/ProjectsSearchTablet.tsx (2)
18-18
: LGTM: Added Best Match sorting for searchThe addition of
sort: EProjectsSortBy.BestMatch
to the query when performing a search is correct and aligns with the PR objective.Consider adding a brief comment explaining why we're setting the sort to "Best Match" when searching. This could improve code readability:
const updatedQuery = { ...router.query, searchTerm, + // Set sort to Best Match when searching for optimal results sort: EProjectsSortBy.BestMatch, };
Line range hint
1-95
: Summary: Implementation successfully adds "Best Match" sortingThe changes in this file successfully implement the "Best Match" sorting functionality for project searches, aligning well with the PR objectives. The code is well-structured and follows React best practices.
Key points:
- Correct import of
EProjectsSortBy
.- Proper implementation of "Best Match" sorting during search.
- Appropriate handling of sort removal when clearing the search.
Minor suggestions have been made for improved clarity and performance. Overall, the implementation looks good and achieves the intended functionality.
If you'd like any further improvements or have any questions about the suggestions, please let me know. I'd be happy to assist with implementing the suggested changes or exploring any other enhancements to this feature.
Tools
Biome
[error] 31-31: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 33-33: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
src/components/views/projects/sort/ProjectsSortSelect.tsx (2)
93-94
: Simplify the useState type inferenceThe generic type
<ISelectedSort[]>
inuseState
is redundant because TypeScript can infer the type from theinitialSortByOptions
. Removing the explicit type makes the code cleaner.You can simplify the code as follows:
- const [sortByOptions, setSortByOptions] = - useState<ISelectedSort[]>(initialSortByOptions); + const [sortByOptions, setSortByOptions] = + useState(initialSortByOptions);
100-121
: Ensure 'Best Match' option is added appropriatelyWhen adding the "Best Match" option, it's currently placed at the end of the list. For better user experience, consider placing it at the top of the sorting options when a search term is present, as it directly relates to the search results.
Modify the code to add the "Best Match" option at the beginning:
... - // Add the "Best Match" option at the end - updatedOptions.push(bestMatchOption); + // Add the "Best Match" option at the beginning + updatedOptions.unshift(bestMatchOption);
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- lang/ca.json (39 hunks)
- lang/en.json (39 hunks)
- lang/es.json (39 hunks)
- src/apollo/types/gqlEnums.ts (1 hunks)
- src/components/views/projects/ProjectsSearchDesktop.tsx (3 hunks)
- src/components/views/projects/ProjectsSearchTablet.tsx (3 hunks)
- src/components/views/projects/sort/ProjectsSortSelect.tsx (4 hunks)
- src/helpers/projects.ts (1 hunks)
Additional context used
Biome
src/components/views/projects/ProjectsSearchDesktop.tsx
[error] 44-44: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
src/components/views/projects/ProjectsSearchTablet.tsx
[error] 33-33: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
Additional comments not posted (68)
src/apollo/types/gqlEnums.ts (1)
24-24
: LGTM! New enum value added correctly.The new 'BestMatch' enum value has been added to the EProjectsSortBy enum, which aligns with the PR objective of introducing a "best match" sorting option. The naming convention is consistent with other enum values (PascalCase).
To ensure this change is properly integrated, let's verify its usage in the codebase:
This script will help us verify if the new enum value is being imported and used correctly in other parts of the codebase.
Verification successful
Verified! The new
BestMatch
enum value is correctly imported and utilized across the codebase, aligning with the PR objectives and maintaining consistent naming conventions.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usage of the new 'BestMatch' enum value # Search for imports of EProjectsSortBy echo "Checking imports of EProjectsSortBy:" rg --type typescript "import.*EProjectsSortBy.*from.*gqlEnums" # Search for usage of BestMatch echo "\nChecking usage of BestMatch:" rg --type typescript "EProjectsSortBy\.BestMatch" # Search for string literal 'BestMatch' in case it's used without the enum echo "\nChecking for string literal 'BestMatch':" rg --type typescript "'BestMatch'"Length of output: 519
Script:
#!/bin/bash # Description: Check for usage of the new 'BestMatch' enum value # Search for imports of EProjectsSortBy echo "Checking imports of EProjectsSortBy:" rg "import.*EProjectsSortBy.*from.*gqlEnums" --glob "*.ts" --glob "*.tsx" # Search for usage of BestMatch echo "\nChecking usage of BestMatch:" rg "EProjectsSortBy\.BestMatch" --glob "*.ts" --glob "*.tsx" # Search for string literal 'BestMatch' in case it's used without the enum echo "\nChecking for string literal 'BestMatch':" rg "'BestMatch'" --glob "*.ts" --glob "*.tsx"Length of output: 2336
src/components/views/projects/ProjectsSearchTablet.tsx (1)
7-7
: LGTM: Import statement for EProjectsSortByThe import of
EProjectsSortBy
from the appropriate module is correct and necessary for the new sorting functionality.src/components/views/projects/ProjectsSearchDesktop.tsx (2)
15-15
: LGTM: Import statement for sorting enum added.The import of
EProjectsSortBy
is correctly placed and aligns with the PR objective of adding a "best match" sort feature.
Line range hint
1-124
: Overall implementation looks good with minor suggestions for improvement.The changes successfully implement the "best match" sorting feature as intended, aligning well with the PR objectives. The code is well-structured and follows React best practices. Consider the suggested improvements for preserving user preferences and addressing the performance hint.
Tools
Biome
[error] 42-42: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 44-44: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
src/helpers/projects.ts (1)
23-23
: LGTM! The addition ofEProjectsSortBy.BestMatch
is consistent and aligns with PR objectives.The new entry in the
sortMap
object forEProjectsSortBy.BestMatch
is correctly implemented and follows the existing pattern. This change supports the PR objective of adding a "best match" sorting option for items.To ensure complete implementation of the "best match" sorting feature, please verify that the corresponding sorting logic is implemented elsewhere in the codebase. Run the following script to check for related changes:
Verification successful
Verification Passed: 'BestMatch' Sorting Implementation Confirmed
The usage of
EProjectsSortBy.BestMatch
is consistently found in multiple components, indicating that the "best match" sorting feature is fully implemented across the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for implementations or usages of the new BestMatch sorting option # Test 1: Search for BestMatch enum usage echo "Searching for BestMatch enum usage:" rg --type typescript "EProjectsSortBy\.BestMatch" # Test 2: Search for potential implementation of best match sorting logic echo "Searching for potential best match sorting implementation:" rg --type typescript "bestMatch|sort.*match"Length of output: 376
Script:
#!/bin/bash # Description: Search for implementations or usages of the new BestMatch sorting option # Test 1: Search for BestMatch enum usage in .ts and .tsx files echo "Searching for BestMatch enum usage in TypeScript files:" fd --extension ts --extension tsx | xargs rg "EProjectsSortBy\.BestMatch" # Test 2: Search for potential implementation of best match sorting logic in .ts and .tsx files echo "Searching for potential best match sorting implementation in TypeScript files:" fd --extension ts --extension tsx | xargs rg "bestMatch|sort.*match"Length of output: 1523
lang/en.json (50)
110-110
: New error message addedA new error message for amount validation has been added. This is a good addition for user feedback.
136-136
: New label added for additional informationA new label "Add more info" has been added. This is a clear and concise label that can be used in various contexts.
201-201
: New label for sorting optionA new label "Best Match" has been added, likely for a sorting option. This is a common and user-friendly sorting choice.
203-203
: New label for communication improvementA new label for better communication with the community has been added. This encourages user engagement.
238-239
: New labels for viewing donationsTwo new labels have been added for viewing donations and donation receipts. These provide clear actions for users to check their donation details.
286-286
: New label for copying memoA new label for copying memo to use in wallet apps has been added. This is a helpful instruction for users.
311-314
: New labels for DeVouch functionalitySeveral new labels related to DeVouch functionality have been added. These provide information and actions for the DeVouch feature.
316-316
: New label for unconfirmed donationsA new label for addressing unconfirmed donations has been added. This helps users understand what to do in case of donation issues.
353-353
: New label for donation statusA new label "Donation Status" has been added. This is a clear and informative label for users to check their donation progress.
391-391
: New label for entering memoA new label "Enter the Memo" has been added. This provides clear instruction for users when a memo is required.
503-503
: New label for modifying donationA new label for going back to modify donation has been added. This gives users the option to adjust their donation before finalizing.
555-555
: New labels for increasing passport scoreTwo new labels related to increasing passport score have been added. These encourage users to improve their standing in the system.
Also applies to: 559-559
564-564
: New label for GIVbacks eligibilityA new label "GIVbacks Eligible" has been added. This clearly indicates the eligibility status for the GIVbacks program.
567-567
: New label for process durationA new label "It won't take long!" has been added. This reassures users about the process duration.
640-640
: New label for memoA new label "Memo" has been added. This is a clear and concise label for memo fields.
668-668
: New label for QR code renewalA new label for requesting a new QR code has been added. This provides a clear action for users when they need a fresh QR code.
673-673
: New label for QR code expirationA new label explaining the need for a new QR code when the previous one expires has been added. This helps users understand the process.
694-694
: New label for unnecessary memoA new label indicating that no memo is needed for a specific address has been added. This clarifies the process for users.
696-696
: New label for number of donationsA new label "# of Donations" has been added. This provides a clear metric for donation counts.
737-737
: New labels for Passport connection and statusNew labels for Passport connection and pending status have been added. These provide clear information about the Passport feature status.
Also applies to: 739-739
763-765
: New labels for waiting and processingNew labels for asking users to wait and explaining the processing of donations have been added. These help manage user expectations during transactions.
774-774
: New label for processingA new label "Processing..." has been added. This is a standard indicator for ongoing operations.
782-782
: New label for project addressA new label "Project Address" has been added. This clearly identifies the address associated with a project.
785-786
: New labels for Endaoment projectsNew labels explaining Endaoment project delivery and associated fees have been added. These provide important information for users about project funding and fees.
794-795
: New labels for project owner address detectionNew labels for detecting project owner addresses and explaining donation restrictions have been added. These help prevent potential conflicts of interest in donations.
806-806
: Updated label for minimum character requirementThe label for minimum character requirement in project descriptions has been updated to use a variable. This allows for flexibility in setting the minimum character count.
812-814
: New labels for QF donor eligibilityNew labels for checking QF donor eligibility and providing more information have been added. These guide users through the eligibility verification process.
818-819
: New labels for QR code errorsNew labels for QR code generation errors and expiration have been added. These provide clear information about QR code issues and necessary actions.
826-826
: New label for raising a ticketA new label "Raise a ticket" has been added. This provides a clear action for users to report issues or seek support.
837-837
: New label for recipient addressA new label "Recipient address" has been added. This clearly identifies the address where funds will be sent.
855-855
: New label for remaining timeA new label "Remaining time" has been added. This provides a clear indicator for time-sensitive operations.
882-884
: New labels for sanctioned wallet detectionNew labels for detecting sanctioned wallet addresses and explaining the implications have been added. These provide important information about compliance and restrictions.
889-889
: New label for QR code scanningA new label explaining how to use the QR code for donations has been added. This provides clear instructions for users making donations.
1127-1128
: New labels for transaction detailsNew labels for transaction details and links have been added. These provide clear access to transaction information.
1131-1131
: New label for Stellar donationsA new label encouraging donations with Stellar has been added. This promotes the use of an additional cryptocurrency for donations.
1138-1138
: New label for uncompleted multisig transactionsA new label for uncompleted multisig transactions has been added. This helps users understand the status of complex transactions.
1144-1144
: New label for updating QR codeA new label "Update QR code" has been added. This provides a clear action for refreshing QR codes.
1164-1164
: New label for validity periodA new label "Valid for" has been added. This indicates the duration of validity for certain items or actions.
1178-1179
: New labels for viewing projects and detailsNew labels for viewing all projects and viewing details have been added. These provide clear navigation options for users.
1188-1190
: New labels for voting, vouching, and waitingNew labels for voting on proposals, vouched status, and waiting have been added. These provide clear indicators for various statuses and actions in the system.
1192-1192
: New label for waiting for donationA new label "Waiting for your donation" has been added. This clearly indicates the system is expecting a user's donation.
1207-1207
: New label for additional information requestA new label indicating the need for more information has been added. This prompts users to provide additional details when necessary.
1278-1278
: New label for pending donationsA new label explaining the handling of pending donations has been added. This helps users understand how to proceed with multiple donation attempts.
1281-1281
: New label for donation amountA new label "You are donating" has been added. This clearly indicates the amount a user is about to donate.
1319-1319
: New label for executing pending multisig transactionsA new label explaining the need to execute pending multisig transactions has been added. This guides users through the process of completing complex transactions.
1321-1321
: New label for submitting donations before timer expirationA new label reminding users to submit donations before a timer runs out has been added. This creates urgency and helps prevent missed donation opportunities.
1626-1637
: New labels for QF donor eligibilitySeveral new labels related to QF donor eligibility have been added. These provide information about eligibility status, Gitcoin Passport integration, and actions to improve eligibility.
1649-1649
: New labels for project verification and boostingNew labels for project verification, vouching, and boosting have been added. These provide information about project status and encourage user engagement through boosting.
Also applies to: 1662-1662, 1665-1665
1695-1696
: New labels for checking QF eligibilityNew labels for checking and rechecking QF eligibility have been added. These provide clear actions for users to verify their eligibility status.
Line range hint
1-1718
: Overall assessment: Excellent additions to the language fileThe new entries in this language file are well-crafted and consistent with the existing content. They provide clear, user-friendly labels and messages that enhance the overall user experience. The additions cover various aspects of the application, including donation processes, project verification, and new features like DeVouch and QF donor eligibility. These changes will contribute to better user understanding and engagement with the platform.
lang/es.json (6)
110-110
: New error message looks goodThe new error message for entering an amount is correctly translated and consistent with the existing style.
238-239
: New labels for donation verification featureThe new labels for checking donations and donation status are correctly translated and consistent with the existing style. These appear to be part of a new feature for verifying donations.
Also applies to: 353-353
391-391
: New labels for memos and transaction detailsThe new labels for memos and transaction details are correctly translated and consistent with the existing style. These appear to be part of new features for handling memos and displaying more transaction information.
Also applies to: 640-640, 1127-1128
739-739
: New labels for status and time informationThe new labels for pending status, remaining time, and success are correctly translated and consistent with the existing style. These will enhance the user interface with more status information.
Also applies to: 855-855, 1004-1004
794-795
: Important new labels for project ownership and donation restrictionsThese new labels provide crucial information for project owners and donors:
- Detection of project owner's address
- Restriction on donating to one's own project
- Handling of pending donations
- Time limit for submitting donations
The translations are correct and consistent with the existing style. These messages will help prevent misuse and improve the donation process.
Also applies to: 1278-1278, 1321-1321
Line range hint
1-1718
: Overall improvements to Spanish translationsThis update to the Spanish language file includes numerous new entries that support enhanced functionality in the application. Key improvements include:
- New error messages for amount validation
- Labels for donation verification and status checking
- Support for memos and detailed transaction information
- Additional status and time-related labels
- Important messages for project owners and donors to prevent misuse
All new translations are correct, consistent with the existing style, and well-formatted. These changes will significantly improve the user experience for Spanish-speaking users by providing more detailed and helpful information throughout the application.
lang/ca.json (6)
110-110
: New error message looks good!The added error message for entering an amount is clear and concise in Catalan.
238-238
: New label for checking donation is correctThe added label for checking a donation is accurately translated to Catalan.
136-136
: New label for adding more information is accurateThe added label for adding more information is correctly translated to Catalan.
1127-1128
: New transaction-related labels are correctly translatedThe added labels for transaction detail and transaction link are accurately translated to Catalan.
201-201
: Additional new labels are correctly translatedThe following new labels have been accurately translated to Catalan:
- "label.best_match": "Millor coincidència"
- "label.go_back_to_modify_your_donation": "Torna a modificar la teva donació"
- "label.memo": "Memo"
These translations are appropriate for their respective contexts.
Also applies to: 503-503, 640-640
Line range hint
1-1718
: Overall, excellent additions to the Catalan translationsThe new translations added to this file are accurate, consistent, and appropriate for their respective contexts. All changes, including those mentioned in the AI-generated summary and the additional ones found during the review, have been approved. The translations maintain a good quality and should improve the user experience for Catalan-speaking users.
src/components/views/projects/sort/ProjectsSortSelect.tsx (1)
157-157
: Update useEffect dependencies to include all relevant variablesThe
useEffect
depends onrouter.query.sort
andsortByOptions
. IfsortByOptions
changes due torouter.query.searchTerm
orisQF
, the effect should re-run to update the selected value accordingly.Ensure that all relevant dependencies are included:
- }, [router.query.sort, sortByOptions]); + }, [router.query.sort, sortByOptions, router.query.searchTerm, isQF]);Alternatively, since
sortByOptions
is now computed withuseMemo
, ensure that its dependencies are accurately reflected.Confirm that the dependency array covers all variables that, when changed, should trigger the effect to update the selected value.
@@ -23,6 +24,7 @@ const ProjectsSearchDesktop = () => { | |||
const updatedQuery = { | |||
...router.query, | |||
searchTerm: searchTerm, | |||
sort: EProjectsSortBy.BestMatch, |
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.
Consider preserving user's existing sort preference.
The addition of sort: EProjectsSortBy.BestMatch
aligns with the PR objective. However, it might override any existing sort preference the user had set.
Consider preserving the user's existing sort preference if one exists:
const updatedQuery = {
...router.query,
searchTerm: searchTerm,
- sort: EProjectsSortBy.BestMatch,
+ sort: router.query.sort || EProjectsSortBy.BestMatch,
};
This change would only set the sort to "Best Match" if no sort preference was previously set.
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.
sort: EProjectsSortBy.BestMatch, | |
sort: router.query.sort || EProjectsSortBy.BestMatch, |
if (isQF) { | ||
updatedOptions.splice( | ||
updatedOptions.length - 1, | ||
0, | ||
{ | ||
label: formatMessage({ id: 'label.amount_raised_in_qf' }), | ||
value: EProjectsSortBy.ActiveQfRoundRaisedFunds, | ||
icon: <IconIncrease16 />, | ||
color: semanticColors.jade[500], | ||
}, | ||
{ | ||
label: formatMessage({ id: 'label.estimated_matching' }), | ||
value: EProjectsSortBy.EstimatedMatching, | ||
icon: <IconEstimated16 />, | ||
color: semanticColors.jade[500], | ||
}, | ||
); | ||
} |
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.
Review splice usage for inserting QF-specific options
The use of splice
to insert QF-specific options assumes that there is at least one element in updatedOptions
. If the options list changes in the future, this insertion point might become incorrect.
For safer insertion, identify the exact position or append the QF-specific options at the desired location based on explicit conditions.
Consider this modification:
...
- updatedOptions.splice(
- updatedOptions.length - 1,
- 0,
+ // Insert QF options before 'Recently Updated' option
+ const insertionIndex = updatedOptions.findIndex(
+ option => option.value === EProjectsSortBy.RECENTLY_UPDATED
+ );
+ updatedOptions.splice(
+ insertionIndex,
+ 0,
{
label: formatMessage({ id: 'label.amount_raised_in_qf' }),
value: EProjectsSortBy.ActiveQfRoundRaisedFunds,
icon: <IconIncrease16 />,
color: semanticColors.jade[500],
},
{
label: formatMessage({ id: 'label.estimated_matching' }),
value: EProjectsSortBy.EstimatedMatching,
icon: <IconEstimated16 />,
color: semanticColors.jade[500],
},
);
This ensures that QF-specific options are inserted before the "Recently Updated" option regardless of the array length.
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.
if (isQF) { | |
updatedOptions.splice( | |
updatedOptions.length - 1, | |
0, | |
{ | |
label: formatMessage({ id: 'label.amount_raised_in_qf' }), | |
value: EProjectsSortBy.ActiveQfRoundRaisedFunds, | |
icon: <IconIncrease16 />, | |
color: semanticColors.jade[500], | |
}, | |
{ | |
label: formatMessage({ id: 'label.estimated_matching' }), | |
value: EProjectsSortBy.EstimatedMatching, | |
icon: <IconEstimated16 />, | |
color: semanticColors.jade[500], | |
}, | |
); | |
} | |
if (isQF) { | |
// Insert QF options before 'Recently Updated' option | |
const insertionIndex = updatedOptions.findIndex( | |
option => option.value === EProjectsSortBy.RECENTLY_UPDATED | |
); | |
updatedOptions.splice( | |
insertionIndex, | |
0, | |
{ | |
label: formatMessage({ id: 'label.amount_raised_in_qf' }), | |
value: EProjectsSortBy.ActiveQfRoundRaisedFunds, | |
icon: <IconIncrease16 />, | |
color: semanticColors.jade[500], | |
}, | |
{ | |
label: formatMessage({ id: 'label.estimated_matching' }), | |
value: EProjectsSortBy.EstimatedMatching, | |
icon: <IconEstimated16 />, | |
color: semanticColors.jade[500], | |
}, | |
); | |
} |
// Update sortByOptions based on the existence of searchTerm | ||
useEffect(() => { | ||
const hasSearchTerm = !!router.query.searchTerm; | ||
|
||
let updatedOptions = [...initialSortByOptions]; | ||
|
||
// Conditionally add the "Best Match" option if searchTerm exists | ||
if (hasSearchTerm) { | ||
const bestMatchOption = { | ||
label: capitalizeFirstLetter( | ||
formatMessage({ id: 'label.best_match' }), | ||
), | ||
value: EProjectsSortBy.BestMatch, | ||
icon: <IconSpark16 />, | ||
}; | ||
// Check if the Best Match option already exists before adding | ||
const bestMatchExists = updatedOptions.some( | ||
option => option.value === EProjectsSortBy.BestMatch, | ||
); | ||
|
||
if (!bestMatchExists) { | ||
updatedOptions.push(bestMatchOption); | ||
} | ||
} | ||
|
||
// Add QF-specific options if isQF is true | ||
if (isQF) { | ||
updatedOptions.splice( | ||
updatedOptions.length - 1, | ||
0, | ||
{ | ||
label: formatMessage({ id: 'label.amount_raised_in_qf' }), | ||
value: EProjectsSortBy.ActiveQfRoundRaisedFunds, | ||
icon: <IconIncrease16 />, | ||
color: semanticColors.jade[500], | ||
}, | ||
{ | ||
label: formatMessage({ id: 'label.estimated_matching' }), | ||
value: EProjectsSortBy.EstimatedMatching, | ||
icon: <IconEstimated16 />, | ||
color: semanticColors.jade[500], | ||
}, | ||
); | ||
} | ||
|
||
setSortByOptions(updatedOptions); | ||
}, [router.query.searchTerm, isQF]); |
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.
Avoid unnecessary state and re-renders by using useMemo
The sortByOptions
state is derived from initialSortByOptions
, router.query.searchTerm
, and isQF
. Using useState
with useEffect
for derived state can lead to unnecessary re-renders and potential bugs due to state updates.
Use useMemo
to compute sortByOptions
based on dependencies without additional state:
+ import React, { ..., useMemo } from 'react';
...
- const [sortByOptions, setSortByOptions] =
- useState(initialSortByOptions);
...
- useEffect(() => {
+ const sortByOptions = useMemo(() => {
const hasSearchTerm = !!router.query.searchTerm;
let updatedOptions = [...initialSortByOptions];
// Conditionally add the "Best Match" option if searchTerm exists
if (hasSearchTerm) {
const bestMatchOption = {
label: capitalizeFirstLetter(
formatMessage({ id: 'label.best_match' }),
),
value: EProjectsSortBy.BestMatch,
icon: <IconSpark16 />,
};
// Add the "Best Match" option at the beginning
updatedOptions.unshift(bestMatchOption);
}
// Add QF-specific options if isQF is true
if (isQF) {
updatedOptions.splice(
updatedOptions.length - 1,
0,
{
label: formatMessage({ id: 'label.amount_raised_in_qf' }),
value: EProjectsSortBy.ActiveQfRoundRaisedFunds,
icon: <IconIncrease16 />,
color: semanticColors.jade[500],
},
{
label: formatMessage({ id: 'label.estimated_matching' }),
value: EProjectsSortBy.EstimatedMatching,
icon: <IconEstimated16 />,
color: semanticColors.jade[500],
},
);
}
return updatedOptions;
- setSortByOptions(updatedOptions);
- }, [router.query.searchTerm, isQF]);
+ }, [initialSortByOptions, router.query.searchTerm, isQF]);
This refactor eliminates the need for the useEffect
and sortByOptions
state, reducing complexity and preventing unnecessary re-renders.
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.
// Update sortByOptions based on the existence of searchTerm | |
useEffect(() => { | |
const hasSearchTerm = !!router.query.searchTerm; | |
let updatedOptions = [...initialSortByOptions]; | |
// Conditionally add the "Best Match" option if searchTerm exists | |
if (hasSearchTerm) { | |
const bestMatchOption = { | |
label: capitalizeFirstLetter( | |
formatMessage({ id: 'label.best_match' }), | |
), | |
value: EProjectsSortBy.BestMatch, | |
icon: <IconSpark16 />, | |
}; | |
// Check if the Best Match option already exists before adding | |
const bestMatchExists = updatedOptions.some( | |
option => option.value === EProjectsSortBy.BestMatch, | |
); | |
if (!bestMatchExists) { | |
updatedOptions.push(bestMatchOption); | |
} | |
} | |
// Add QF-specific options if isQF is true | |
if (isQF) { | |
updatedOptions.splice( | |
updatedOptions.length - 1, | |
0, | |
{ | |
label: formatMessage({ id: 'label.amount_raised_in_qf' }), | |
value: EProjectsSortBy.ActiveQfRoundRaisedFunds, | |
icon: <IconIncrease16 />, | |
color: semanticColors.jade[500], | |
}, | |
{ | |
label: formatMessage({ id: 'label.estimated_matching' }), | |
value: EProjectsSortBy.EstimatedMatching, | |
icon: <IconEstimated16 />, | |
color: semanticColors.jade[500], | |
}, | |
); | |
} | |
setSortByOptions(updatedOptions); | |
}, [router.query.searchTerm, isQF]); | |
import React, { ..., useMemo } from 'react'; | |
// Update sortByOptions based on the existence of searchTerm | |
const sortByOptions = useMemo(() => { | |
const hasSearchTerm = !!router.query.searchTerm; | |
let updatedOptions = [...initialSortByOptions]; | |
// Conditionally add the "Best Match" option if searchTerm exists | |
if (hasSearchTerm) { | |
const bestMatchOption = { | |
label: capitalizeFirstLetter( | |
formatMessage({ id: 'label.best_match' }), | |
), | |
value: EProjectsSortBy.BestMatch, | |
icon: <IconSpark16 />, | |
}; | |
// Add the "Best Match" option at the beginning | |
updatedOptions.unshift(bestMatchOption); | |
} | |
// Add QF-specific options if isQF is true | |
if (isQF) { | |
updatedOptions.splice( | |
updatedOptions.length - 1, | |
0, | |
{ | |
label: formatMessage({ id: 'label.amount_raised_in_qf' }), | |
value: EProjectsSortBy.ActiveQfRoundRaisedFunds, | |
icon: <IconIncrease16 />, | |
color: semanticColors.jade[500], | |
}, | |
{ | |
label: formatMessage({ id: 'label.estimated_matching' }), | |
value: EProjectsSortBy.EstimatedMatching, | |
icon: <IconEstimated16 />, | |
color: semanticColors.jade[500], | |
}, | |
); | |
} | |
return updatedOptions; | |
}, [initialSortByOptions, router.query.searchTerm, isQF]); |
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.
LGTM! Thanks @MohammadPCh
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.
LGTM ;) thx @MohammadPCh
Summary by CodeRabbit
New Features
Bug Fixes