Skip to content

Conversation

@chillenberger
Copy link

@chillenberger chillenberger commented Aug 29, 2025

Add frontend to filter and sort Funding pages (RFP, Proposal, Previously Funded):

RFP:

  • filter on (Hub)
  • sort on (Created date, Amount, Expiring soon, Most applicants)

Proposals:

  • filter on Hub
  • sort on (Created date, Goal, Expiring soon, Amount Raised, Popular, Most reviewed)

Previously Funded:

  • filter on Hub
  • sort on (Created date, Goal, Amount Raised)

Other:

  • add sort icon to sort dropdown for clarity

@vercel
Copy link

vercel bot commented Aug 29, 2025

@chillenberger is attempting to deploy a commit to the ResearchHub Team on Vercel.

A member of the Team first needs to authorize it.

@chillenberger chillenberger marked this pull request as ready for review September 4, 2025 19:03
hooks/useFeed.ts Outdated
}
}, [status, activeTab]);

const arraysEqual = (a?: (string | number)[], b?: (string | number)[]) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

We use lodash, can use _.isEqual

Copy link
Author

Choose a reason for hiding this comment

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

👍

hooks/useFeed.ts Outdated
};

// Check if options have changed
useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we refactor this useEffect into a hook? I realize that this was here before you started but it's getting a bit too long right now

Copy link
Author

Choose a reason for hiding this comment

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

I agree, this whole thing is getting a bit long. I’m happy to move this effect into its own hook, but since it’s only used here, I first tried using omit and isEqual to simplify and compress it. Let me know what you think. I also combined loadFeed and loadMore into one function, since they were basically copy-pastes of each other. I think these two changes make the useFeed hook much easier to understand.

It looks like we could simplify it even further, since the currentTab state and initial activeTab parameter seem to be dead code. That would require touching more files, though, so I held back for now.

private static readonly GRANT_PATH = '/api/grant_feed';

// Determine which endpoint to use
private static getEndpointPath(endpoint: Endpoints) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice pattern

Copy link
Author

Choose a reason for hiding this comment

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

🙏


try {
const response = await ApiClient.get<any[]>(path);
// Use transformTopic to normalize
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use transformTopic in topic.ts ?

Copy link
Author

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice refactoring

Copy link
Author

Choose a reason for hiding this comment

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

🙏 Thank you, and again I think we can go further here since after finishing this I found another component that was a copy and paste of the one I refactored. I haven't tracked down exactly what is going on here but I believe they should probably be refactored to be one component. https://github.com/ResearchHub/web/blob/main/app/paper/create/components/HubsSelector.tsx

@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 9, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants