Conversation
GorlemZ
force-pushed
the
filters
branch
2 times, most recently
from
671106b to
4cd98ac
Compare
federico-ercoles
left a comment
federico-ercoles
left a comment
There was a problem hiding this comment.
A couple of questions before reviewing the code:
- I see you have skipped the step of implementing the API call, is there a specific reason why?
- I also see you're not using
formoas suggested for form management. I recommend having a look at using it, since it's our default stack choice.
|
Answ:
|
federico-ercoles
force-pushed
the
filters
branch
from
eb304b1 to
25b64b4
Compare
|
Another day another api call |
federico-ercoles
left a comment
federico-ercoles
left a comment
There was a problem hiding this comment.
There are a few areas for improvement, see the specific comments for details.
A general feedback: here you have implemented both the filters and the detail page, where normally each one should be their own PR. You should try to have your PRs have as small a scope as possible, so that their purpose is more easily identifiable and the code is quicker to review and fix, if needed.
|
I think I've managed all your observations :) I agree, I should have splitted this into more PRs for readability and speed on both of our sides. I'll learn from this, thanks! |
src/api.tsx
Outdated
| const radiusParamsString: string = | ||
| radius == 0 ? "900" : radius.toString() + "000"; | ||
| const uri = `/api/search?sort_by=best_match&location=${location}&radius=${radiusParamsString}&${priceParamsString}`; | ||
| console.log(uri); |
2 participants
No description provided.