-
Couldn't load subscription status.
- Fork 14
add sort and hub filter dropdowns #385
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
base: main
Are you sure you want to change the base?
Conversation
|
@chillenberger is attempting to deploy a commit to the ResearchHub Team on Vercel. A member of the Team first needs to authorize it. |
hooks/useFeed.ts
Outdated
| } | ||
| }, [status, activeTab]); | ||
|
|
||
| const arraysEqual = (a?: (string | number)[], b?: (string | number)[]) => { |
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.
We use lodash, can use _.isEqual
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.
👍
hooks/useFeed.ts
Outdated
| }; | ||
|
|
||
| // Check if options have changed | ||
| useEffect(() => { |
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.
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
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.
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) { |
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.
Nice pattern
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.
🙏
services/feed.service.ts
Outdated
|
|
||
| try { | ||
| const response = await ApiClient.get<any[]>(path); | ||
| // Use transformTopic to normalize |
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.
Can we use transformTopic in topic.ts ?
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.
👍
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.
Nice refactoring
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.
🙏 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
|



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