-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Amy Cash - Pipes #50
base: master
Are you sure you want to change the base?
Amy Cash - Pipes #50
Conversation
BackTREKWhat We're Looking For
|
|
||
// import Backbone from 'backbone'; | ||
// | ||
// const Reservation = Backbone.Model.extend({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like you were on a good track!
$('th.sort').removeClass('current-sort-field'); | ||
$(`th.sort.${ tripList.comparator }` ).addClass('current-sort-field'); | ||
|
||
tripList.on('update', renderAll); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should be in $(document(.ready
this will eventually cause lots of rendering when a new model is added or if it's been sorted multiple times.
Only add the listener once, not every time it renders.
const bookTripID = $(event.target).attr('data-id'); | ||
let reservationFormData = | ||
$('#reservation-form').serialize(); | ||
makeReservation(reservationFormData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you're calling makeReservation
with the wrong paramater. The 1st parameter should be the tripID
} | ||
|
||
if (!CONTINENTS.includes(attributes.continent)) { | ||
errors.continent = [' is not a valid continent.']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would overwrite the blank continent message. Instead you'll need to do:
if (errors.continent) {
errors.continent.push('is not a valid continent');
} else {
errors.continent = [' is not a valid continent.'];
}
|
||
$('#individual-trip').hide(); | ||
const newTrip = new Trip(readAddTripForm); | ||
tripList.add(newTrip); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no checking for validations?
Also the trip doesn't have an ID yet (gets the from the API), so you should wait until the save is successful and then execute tripList.fetch()
to pull down the updated Trip from the API (with an ID).
validate: function(attributes) { | ||
const errors = {}; | ||
const CONTINENTS = ['Asia', 'Africa', 'Australasia', 'Europe', 'South America', 'North America', 'asia', 'africa', 'australasia', 'europe', 'south america', 'north america'] | ||
if (!attributes.name) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you'll need this
in front of attributes
like:
if (! this.attributes.name) {
BackTREK
Congratulations! You're submitting your assignment!
Comprehension Questions