Ampers: Nora Peters #20
Conversation
…request functionality for all trips
…t form data and successfully submit axios post request
…s and reserve-trip sections
… method to take in modified url. adds helper methods for making continent query urls. and adds a bunch of comments and failed attempts at making a toggle feature for read more text. adds successful click event for reading more about text
|
|
||
| $('#trip-list').on('click', 'li', function(event) { | ||
| let id = Number(event.target.id); | ||
| $("section").removeClass("hidden"); |
There was a problem hiding this comment.
this ends up working, but it might be more specific to use a more specific selector than 'section' since you have more than one section in your HTML
| axios.post((`${URL}${id}/reservations`), tripData) | ||
| .then((response) => { | ||
| clearForm(); | ||
| reportStatus(`Successfully created trip reservation with ID ${response.data.trip_id}!`); |
There was a problem hiding this comment.
response.data.trip_id refer to the ID of the trip, not the reservation itself. Funnily enough, the API doesn't give back a response with the reservation ID!
| axios.get(URL + id) | ||
| .then((response) => { | ||
| let data = response.data; | ||
| let name = $(`<h4><strong>Name:</strong> ${data.name}</h4>`).addClass(`${id}`); |
There was a problem hiding this comment.
It might make more sense to set the id attr on this element instead of adding a class, because this value represents a unique identifier rather than a styling class?
| let data = response.data; | ||
| let name = $(`<h4><strong>Name:</strong> ${data.name}</h4>`).addClass(`${id}`); | ||
| let about = $(`<span><strong>Description:</strong> ${data.about.slice(0, 150)}</span>`).addClass("teaser"); | ||
| let aboutFullText = $(`<span>${data.about.slice(150)}</span>`).addClass("moreinfo hidden").attr('id', 'info1'); |
| const sAmericaTrips = () => { | ||
| let url = (URL + '/continent?query=South%20America') | ||
| getTrips(url) | ||
| } |
There was a problem hiding this comment.
These functions all end up feeling very similar! It might make sense to refactor them such that...
const getTripsByContinent = (continentName) => {
getTrips( encodeURI( URL + '/continent?query=' + continentName ) );
}Then lines 167-173 could look maybe more like
$('#asia').click( (event) => getTripsByContinent('Asia') );
| //LOAD all/specific trips based on url params | ||
| const getTrips = (url) => { | ||
| const tripList = $('#trip-list'); | ||
| tripList.show(); |
There was a problem hiding this comment.
This is just a random jQuery thing that I'm not too concerned about but: Since you're calling .show() on this element, jQuery actually puts an inline style on this element for style={display: block;} ... which is fine, except that it's awkward since there's a class on this called hidden that never gets removed... so it ends up looking like this:
<ul class="trips hidden" id="trip-list" style="display: block;"> ... </ul>It's a little awkward to read class="hidden" and at the same time see it on the page because of the inline style.
Like I said, I'm not too worried about this... unless it introduces bugs later (what if you try to select on $('.hidden')?) It might make sense to conform to one style or the other (either removing the class hidden instead of using .show(), or in $(document).ready calling $('#trip-list').hide() right at the beginning
TREKWhat We're Looking For
Great work on this project! The project definitely works as expected; the code looks good and the site looks good. I love the interesting work you did on the "... Read more" part Also, good job on the optionals! I've left a couple of comments all on minor things; I have no red flags otherwise Overall, good work |
TREK
Congratulations! You're submitting your assignment!
Comprehension Questions