Add more information to restaurant cards, including website, phone number, and reviews. #36
Conversation
| @@ -316,7 +356,7 @@ app.get( | |||
| ); | |||
|
|
|||
| app.get(`${PREFIX_API}/geocode`, async (request, response) => { | |||
| const address = encodeAddress(request.query.address); | |||
| const address = encodeForURL(request.query.address); | |||
There was a problem hiding this comment.
What is the difference between encodeAddress and encodeForURL? Is it just a function you created to format the address?
There was a problem hiding this comment.
I changed the name to be a bit broader. It encodes any string to be used in URLs, not just addresses. I use this function in more than one api now, so I made the name fit its purpose better.
static/absolute/js/searchPage.js
Outdated
| let messageText = | ||
| restaurantResponse.data === "ZERO_RESULTS" | ||
| ? "We could not find any restaurants. Check to make sure you are using the correct address." | ||
| : "Something went wrong when searching for restaurants."; | ||
| restaurantErrorMessage.appendChild(document.createTextNode(messageText)); |
There was a problem hiding this comment.
Ved made some changes to the places API that may affect this. I suggest you rebase and run through it when he merges avoid any future issues
There was a problem hiding this comment.
I saw. I already plan to wait for his changes and rebase.
| let image = document.createElement("img"); | ||
| image.src = | ||
| "https://maps.googleapis.com/maps/api/place/photo?photoreference=" + | ||
| restaurant.photos[0].photo_reference + |
There was a problem hiding this comment.
nit: making this in such a way that its easier to understand it's coming from the place search response. Maybe placing it in a descriptive variable name?
static/absolute/js/searchPage.js
Outdated
| for (restaurant of allRestaurants.results) { | ||
| //Get additional details for every restaurant using that restaurant's place id. | ||
| let placeDetailsResponse = await ( | ||
| await fetch( |
There was a problem hiding this comment.
2 notes:
-
This will wait for each place details response to finish sequentially, which will be quite slow. It would be nice to parallelize the place details requests. However, since we're so close to the deadline, we might not have the time.
-
I think it would make sense to do the place details requests on the server and pass the information back to the client. It would probably be easier to parallelize on the server too, since the ordering of the restaurants that aren't returned doesn't matter when you're on the server, but, on the client we need to know which place details data corresponds to which HTML element.
static/absolute/js/searchPage.js
Outdated
| ) | ||
| ).json(); | ||
|
|
||
| let additionalDetails = |
There was a problem hiding this comment.
General note: I think it makes sense to prefer const by default. So all variables should be const, and if, you need to re-assign something, use let. This seems better because const variables tell the user that "this value doesn't change", which makes it easier to read/debug code. When I see a let, I assume that the value is going to be re-assigned later.
I would go through this PR and change all lets to const, if the variable declared with let is not being re-assigned.
aivey65
force-pushed
the
AshaDevelopment
branch
from
e731769 to
180ebe3
Compare
Add more information to restaurant cards, including website, phone number, and reviews.
This is just a draft until #35 is merged. After that, it will be rebased.
Addresses issues #11 & #8
Changes
Attached