ChickenKyiv / recipe-search-react

We're replicating advanced search for recipe based projects
https://modest-shannon-513067.netlify.com/
1 stars 3 forks source link

move from predefined arrays to ajax calls to api #35

Open atherdon opened 6 years ago

atherdon commented 6 years ago

also similar to react native app recipe logic

nickisaacs commented 6 years ago

Can I pick this up? Please provide me details if you are okay with it

atherdon commented 6 years ago

it's a not an easy task - do you think you can handle it? @nickisaacs Btw, just yesterday i finished a first part of documentation, related to our search API server, so for a first part of this task - i got you covered. basically, @chauhannishith make some code, related to API calls, so you'll not need to start from scratch. hope this information will help you

nickisaacs commented 6 years ago

I do this for a living, so yes :) Would love to help you out

atherdon commented 6 years ago

Ok, @nickisaacs good to know about it! I'll be happy to help with any of your questions. and will try to speed up your development process, by giving as much information as i can. So here is the current version of documentation, related to our search api server. it's not finished, so if you find something strange - just avoid it :) https://chickenkyiv.gitbook.io/documentation/search-api-tests

nickisaacs commented 6 years ago

@atherdon I can see you are currently reading from the JS files and populating the data inside the render method of each component which is not okay. Ideally, the data should be populated inside componentDidMount(). You will face problems when you switch to ajax calls. I plan to refactor this. Is that fine?

I went through your documentation quickly. I could see this: https://chickenkyiv.gitbook.io/documentation/search-api-tests/simple-queries/ingredients-by-name You have mentioned the URLs dont work. So If I were to begin right now, where should I start?

atherdon commented 6 years ago

perfect - thank you!

atherdon commented 6 years ago

you can start from fetching data for selects: https://chickenkyiv.gitbook.io/documentation/search-api-tests/attribute

atherdon commented 6 years ago

but keep in mind that requests should be separated from components some how. soon or later we plan to switch from REST to graphql - because of crazy generated API endpoints of loopback :)

nickisaacs commented 6 years ago

@atherdon I've implemented it for all selects except for ingredients. I can't find an endpoint/query that gives me a list of all the ingredients. BTW, im fetching the data from inside the component now. Do you want me to fetch it from the parent and then pass it down as props to the children? Or maybe we can start using redux for this?

atherdon commented 6 years ago

I think you should check this page for ingredient endpoints: https://chickenkyiv.gitbook.io/documentation/search-api-tests/ingredients

In short, you should check this URL for the list of ingredients: https://loopback-recipe-search.herokuapp.com/api/ingredient

Maybe we should use React Content API instead of Redux? If fetching data at parent will reduce logic in our form fields and will keep them more clean - i'm ok with that.

Q: Can you add id attribute to each option, so when we'll send a search request to a server -> we'll pass an array of ids?

atherdon commented 6 years ago

please follow this link: https://github.com/ChickenKyiv/recipe-search-react/invitations

nickisaacs commented 6 years ago

@atherdon Ok, will use the new context API for this. Yes, fetching data before rendering the selects should be better in my opinion, especially since you have components like cuisines which are shown two times, and call the same API twice currently. Sure, I will add the id as a prop to the Option component, currently I was using it as the key.

Would it be possible to have an API that returns data for all the selects in one network call? Does that sound like a good idea to you? Something like {allergies: [...], cuisines: [...], ... }

atherdon commented 6 years ago

yes, i can do this. will post an update when it'll be done

atherdon commented 6 years ago

your question is just +1 for moving from REST to graphQL :)

nickisaacs commented 6 years ago

@atherdon I have completed this activity, but it seems another commit was already made to fetch data. I have merge conflicts right now. Should I proceed with this?

chauhannishith commented 6 years ago

@nickisaacs you can undo the commit and make it available on a different branch. That way he will have both options to compare and choose

nickisaacs commented 6 years ago

@chauhannishith Ok done :) @atherdon Here is the PR: https://github.com/ChickenKyiv/recipe-search-react/pull/73 My branch is here: https://github.com/nickisaacs/recipe-search-react/tree/feature/async-calls-select

PS - My eslint autoformatted all the files I edited. I hope that's not going to be a problem

atherdon commented 6 years ago

@nickisaacs i'm working on updates, related to form right now. will review your changes when finish my part.