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.

Sign up for GitHub

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

Jump to bottom

Feat/add best match sort item #4761

Merged
merged 5 commits into from
Sep 23, 2024
Merged

Feat/add best match sort item #4761

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4d8e054
update translations
MohammadPCh Sep 23, 2024
a719ae6
add best match to EProjectsSortBy
MohammadPCh Sep 23, 2024
8e9d519
add best match to sortMap
MohammadPCh Sep 23, 2024
dca6c01
add best match to sort if there is sort term
MohammadPCh Sep 23, 2024
02e572a
handle sort on search
MohammadPCh Sep 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
188 changes: 98 additions & 90 deletions lang/ca.json
View file
Open in desktop

Large diffs are not rendered by default.

196 changes: 98 additions & 98 deletions lang/en.json
View file
Open in desktop

Large diffs are not rendered by default.

193 changes: 98 additions & 95 deletions lang/es.json
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/apollo/types/gqlEnums.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export enum EProjectsSortBy {
INSTANT_BOOSTING = 'InstantBoosting',
ActiveQfRoundRaisedFunds = 'ActiveQfRoundRaisedFunds',
EstimatedMatching = 'EstimatedMatching',
BestMatch = 'BestMatch',
}

export enum EQFRoundsSortBy {
Expand Down
4 changes: 4 additions & 0 deletions src/components/views/projects/ProjectsSearchDesktop.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import Input from '@/components/Input';
import IconEnter from '../../../../public/images/icons/enter.svg';
import { useProjectsContext } from '@/context/projects.context';
import useFocus from '@/hooks/useFocus';
import { EProjectsSortBy } from '@/apollo/types/gqlEnums';

const ProjectsSearchDesktop = () => {
const { variables } = useProjectsContext();
Expand All @@ -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,

};
router.push({
pathname: router.pathname,
Expand All @@ -38,6 +40,8 @@ const ProjectsSearchDesktop = () => {
...router.query,
};
delete updatedQuery.searchTerm;
if (router.query.sort === EProjectsSortBy.BestMatch)
delete updatedQuery.sort;
MohammadPCh marked this conversation as resolved.
Show resolved Hide resolved
router.push({
pathname: router.pathname,
query: updatedQuery,
Expand Down
4 changes: 4 additions & 0 deletions src/components/views/projects/ProjectsSearchTablet.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import styled from 'styled-components';
import { useRouter } from 'next/router';
import Input from '@/components/Input';
import { useProjectsContext } from '@/context/projects.context';
import { EProjectsSortBy } from '@/apollo/types/gqlEnums';

const ProjectsSearchTablet = () => {
const { variables } = useProjectsContext();
Expand All @@ -14,6 +15,7 @@ const ProjectsSearchTablet = () => {
const updatedQuery = {
...router.query,
searchTerm,
sort: EProjectsSortBy.BestMatch,
};
router.push({
pathname: router.pathname,
Expand All @@ -27,6 +29,8 @@ const ProjectsSearchTablet = () => {
...router.query,
};
delete updatedQuery.searchTerm;
if (router.query.sort === EProjectsSortBy.BestMatch)
delete updatedQuery.sort;
MohammadPCh marked this conversation as resolved.
Show resolved Hide resolved
router.push({
pathname: router.pathname,
query: updatedQuery,
Expand Down
76 changes: 55 additions & 21 deletions src/components/views/projects/sort/ProjectsSortSelect.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
IconPublish16,
IconEstimated16,
IconGIVBack16,
IconSpark16,
} from '@giveth/ui-design-system';
import Select, {
components,
Expand Down Expand Up @@ -49,8 +50,10 @@ export const DropdownIndicator: ComponentType<
const ProjectsSortSelect = () => {
const { formatMessage } = useIntl();
const { isQF } = useProjectsContext();
const router = useRouter();

let sortByOptions: ISelectedSort[] = [
// Default sortByOptions without Best Match
const initialSortByOptions: ISelectedSort[] = [
{
label: formatMessage({ id: 'label.givpower' }),
value: EProjectsSortBy.INSTANT_BOOSTING,
Expand Down Expand Up @@ -87,27 +90,58 @@ const ProjectsSortSelect = () => {
},
];

isQF &&
sortByOptions.splice(
sortByOptions.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],
},
);

const [sortByOptions, setSortByOptions] =
useState<ISelectedSort[]>(initialSortByOptions);
const [value, setValue] = useState(sortByOptions[0]);
const { isMobile } = useDetectDevice();
const router = useRouter();

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


setSortByOptions(updatedOptions);
}, [router.query.searchTerm, isQF]);
Comment on lines +98 to +144
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]);


useEffect(() => {
if (router.query.sort) {
Expand All @@ -120,7 +154,7 @@ const ProjectsSortSelect = () => {
} else {
setValue(sortByOptions[0]);
}
}, [router.query.sort]);
}, [router.query.sort, sortByOptions]);

return (
<Flex
Expand Down
1 change: 1 addition & 0 deletions src/helpers/projects.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export const sortMap = {
EProjectsSortBy.ActiveQfRoundRaisedFunds,
[EProjectsSortBy.EstimatedMatching.toLowerCase()]:
EProjectsSortBy.EstimatedMatching,
[EProjectsSortBy.BestMatch.toLowerCase()]: EProjectsSortBy.BestMatch,
};

/**
Expand Down
Loading