Skip to content
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

Merged
merged 5 commits into from
Sep 23, 2024
Merged

Conversation

MohammadPCh
Copy link
Collaborator

@MohammadPCh MohammadPCh commented Sep 23, 2024

Summary by CodeRabbit

  • New Features

    • Added new sorting option "Best Match" for project searches.
    • Enhanced user feedback with new error messages and labels across multiple languages (Catalan, English, Spanish) related to donations and project management.
    • Improved user guidance with new prompts for donation processes.
  • Bug Fixes

    • Updated existing labels for clarity and relevance in donation and project management contexts.

Copy link

vercel bot commented Sep 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
giveth-dapps-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 23, 2024 2:33pm

Copy link
Contributor

coderabbitai bot commented Sep 23, 2024

Walkthrough

The 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

File Change Summary
lang/ca.json, lang/en.json, lang/es.json New labels and error messages added to improve user feedback during donation processes and project management.
src/apollo/types/gqlEnums.ts New enumeration value BestMatch added to EProjectsSortBy for sorting projects.
src/components/views/projects/ProjectsSearchDesktop.tsx, src/components/views/projects/ProjectsSearchTablet.tsx Updated to utilize EProjectsSortBy.BestMatch for sorting project searches based on user input.
src/components/views/projects/sort/ProjectsSortSelect.tsx Logic modified to conditionally include "Best Match" in sorting options based on search term presence.
src/helpers/projects.ts New entry added to sortMap for EProjectsSortBy.BestMatch, expanding sorting options.

Possibly related PRs

Suggested reviewers

  • RamRamez
  • mateodaza

🐰 In the garden of code, we hop with glee,
New labels and sorting, as clear as can be!
With donations in mind, we help users see,
A "Best Match" for projects, oh what a spree!
So let’s celebrate changes, both big and small,
For a smoother experience, we’ll answer the call! 🌼


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    -- I pushed a fix in commit <commit_id>, please review it.
    -- Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    -- @coderabbitai generate unit testing code for this file.
    -- @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    -- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    -- @coderabbitai read src/utils.ts and generate unit testing code.
    -- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    -- @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@mohammadranjbarz mohammadranjbarz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @MohammadPCh LGTM

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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:

  1. Update the GraphQL schema to include this new sorting option.
  2. Implement the sorting logic on the backend (likely in the impact-graph repository).
  3. Update any UI components that use the EProjectsSortBy enum to include this new option.
  4. 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 search

The 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" sorting

The 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:

  1. Correct import of EProjectsSortBy.
  2. Proper implementation of "Best Match" sorting during search.
  3. 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 inference

The generic type <ISelectedSort[]> in useState is redundant because TypeScript can infer the type from the initialSortByOptions. 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 appropriately

When 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

Commits

Files that changed from the base of the PR and between 751a03a and 02e572a.

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 EProjectsSortBy

The 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 of EProjectsSortBy.BestMatch is consistent and aligns with PR objectives.

The new entry in the sortMap object for EProjectsSortBy.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 added

A new error message for amount validation has been added. This is a good addition for user feedback.


136-136: New label added for additional information

A 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 option

A 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 improvement

A new label for better communication with the community has been added. This encourages user engagement.


238-239: New labels for viewing donations

Two 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 memo

A 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 functionality

Several new labels related to DeVouch functionality have been added. These provide information and actions for the DeVouch feature.


316-316: New label for unconfirmed donations

A 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 status

A 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 memo

A 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 donation

A 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 score

Two 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 eligibility

A new label "GIVbacks Eligible" has been added. This clearly indicates the eligibility status for the GIVbacks program.


567-567: New label for process duration

A new label "It won't take long!" has been added. This reassures users about the process duration.


640-640: New label for memo

A new label "Memo" has been added. This is a clear and concise label for memo fields.


668-668: New label for QR code renewal

A 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 expiration

A 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 memo

A 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 donations

A new label "# of Donations" has been added. This provides a clear metric for donation counts.


737-737: New labels for Passport connection and status

New 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 processing

New 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 processing

A new label "Processing..." has been added. This is a standard indicator for ongoing operations.


782-782: New label for project address

A new label "Project Address" has been added. This clearly identifies the address associated with a project.


785-786: New labels for Endaoment projects

New 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 detection

New 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 requirement

The 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 eligibility

New 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 errors

New 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 ticket

A 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 address

A new label "Recipient address" has been added. This clearly identifies the address where funds will be sent.


855-855: New label for remaining time

A new label "Remaining time" has been added. This provides a clear indicator for time-sensitive operations.


882-884: New labels for sanctioned wallet detection

New 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 scanning

A 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 details

New labels for transaction details and links have been added. These provide clear access to transaction information.


1131-1131: New label for Stellar donations

A 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 transactions

A 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 code

A new label "Update QR code" has been added. This provides a clear action for refreshing QR codes.


1164-1164: New label for validity period

A 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 details

New 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 waiting

New 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 donation

A 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 request

A 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 donations

A 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 amount

A 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 transactions

A 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 expiration

A 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 eligibility

Several 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 boosting

New 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 eligibility

New 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 file

The 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 good

The new error message for entering an amount is correctly translated and consistent with the existing style.


238-239: New labels for donation verification feature

The 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 details

The 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 information

The 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 restrictions

These new labels provide crucial information for project owners and donors:

  1. Detection of project owner's address
  2. Restriction on donating to one's own project
  3. Handling of pending donations
  4. 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 translations

This update to the Spanish language file includes numerous new entries that support enhanced functionality in the application. Key improvements include:

  1. New error messages for amount validation
  2. Labels for donation verification and status checking
  3. Support for memos and detailed transaction information
  4. Additional status and time-related labels
  5. 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 correct

The added label for checking a donation is accurately translated to Catalan.


136-136: New label for adding more information is accurate

The added label for adding more information is correctly translated to Catalan.


1127-1128: New transaction-related labels are correctly translated

The added labels for transaction detail and transaction link are accurately translated to Catalan.


201-201: Additional new labels are correctly translated

The 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 translations

The 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 variables

The useEffect depends on router.query.sort and sortByOptions. If sortByOptions changes due to router.query.searchTerm or isQF, 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 with useMemo, 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,
Copy link
Contributor

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.

Suggested change
sort: EProjectsSortBy.BestMatch,
sort: router.query.sort || EProjectsSortBy.BestMatch,

Comment on lines +124 to +141
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],
},
);
}
Copy link
Contributor

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.

Suggested change
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],
},
);
}

Comment on lines +98 to +144
// 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]);
Copy link
Contributor

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.

Suggested change
// 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]);

Copy link
Collaborator

@Meriem-BM Meriem-BM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks @MohammadPCh

Copy link
Collaborator

@kkatusic kkatusic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ;) thx @MohammadPCh

@MohammadPCh MohammadPCh merged commit 2691a57 into develop Sep 23, 2024
3 checks passed
@MohammadPCh MohammadPCh deleted the feat/add-best-match-sort-item branch September 23, 2024 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: QA
Development

Successfully merging this pull request may close these issues.

4 participants