search

AlexsLemonade / refinebio-web

Refinebio Web
https://staging.web.refine.bio
BSD 3-Clause "New" or "Revised" License
1 stars 0 forks source link

122 - Implement SearchManagerContext and useSearchManager #126

Closed nozomione closed 1 year ago

nozomione commented 1 year ago

Issue Number

122

This PR is the latest codebase for the search results (relevant closed PRs 98(note), 115(note), 116(note), 118(note)).

Purpose/Implementation Notes

API Integration for the refinebio search endpoint.

Using the context API and the refinebio API search endpoints, implemented the SearchManagerContext and its hook useSearchManager which includes necessary methods to perform searches.

Please review the latest UI here.

NOTE: Currently using the api.search.get method in /api directory, but it will be swapped with the refinebio-js helper's api.search.get method later on.

Types of changes

The files that need to be reviewed in this PR (the initial commit of this PR starts at 6bae79b and ends at 7eb1bdb - those commits includes the UI adjustments/typos and I added the detailed descriptions per commit for your easy review 🔎):

[ Newly added ] Context: SearchManagerContext Hook: useSearchManager Helpers: formatURLString (this file is also included in PR 125), fetchSearch

[ Modified ] Pages: _app, search, experiment (commit c619a37) Config: options.js Components:

Other: next.config.js (commit a6cca83, c5b9266)

NOTE I merged the dev branch.

Functional tests

List out the functional tests you've completed to verify your changes work locally.

Checklist

vercel[bot] commented 1 year ago

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

Name Status Preview Comments Updated (UTC)
refinebio-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 20, 2023 10:09pm
nozomione commented 1 year ago

@davidsmejia as requested today's 1:1, I closed the outdated PRs and prepared the 3 PRs for you to review tomorrow during the time you allocated time for PR review.

3 of 3

This is the latest search result page and the search filter related files are one of the common causes for merge conflicts in the new PRs (since currently the dev includes the outdated implementation of the search results page), so merging this should reduce those conflicts caused by the old/deleted files.

I also updated my initial PR comment and included a list of files for you to review. Let me know if you have any questions!

NOTE:

nozomione commented 1 year ago

@davidsmejia, thank you for the review!

p should not exist on the client side in the new version, we should only have offset and limit

I used p(the currently selected page number) and size (the number of results that are currently showing on the page) parameters not only to match the refinebio-frontend implementation, but also to create a user-friendly URL. For instance, the interpretation of p would be easier for users to understand than offset since it's commonly used (e.g., when a user want to go to page 5 by simply altering the URL). What do you think 💬 ?

I also do not understand what's going on in src/helpers/getSearchQueryForAPI.js. If this is to support previous existing urls we should make that clearer in the code.

This helper is used to filter out any parameters(e.g., empty, sortby) that are only used on the client side when making the API requests to the server. In the helper file, I'll include a comment describing what it does and improve it as needed 👍

Other than that looks good!

🥳 🎉