Sockets - Kate N by KateAnnNichols · Pull Request #33 · Ada-C11/trek

KateAnnNichols · 2019-06-03T06:46:44Z

TREK

Congratulations! You're submitting your assignment!

Comprehension Questions

Question	Answer
What does it mean for code to be asynchronous?	Many things can be done at once- here, you can still interact with the page even if actively waiting on a response.
Describe a piece of your code that executes asynchronously. How did this affect the way you structured it?	Loading details for a trip (or any of the API calls). I used jQuery to manage display properties after data had been retrieved from the API (ex- scrollTop).
What kind of errors might the API give you? How did you choose to handle them?	A not found error, incorrect access token, invalid data. If there was an error with the API, an error message would appear.
Suppose you needed to routinely find a specific `Trip` in the list by it's ID field. Would it be advantageous to keep the list in order by the id field? Explain why or why not.	Yes, so you could more easily find the trip with the ID listed first. The easiest way to do this would be to add a search function for finding a trip by ID.

…s the request from JS to trip API, .then loops through the response data, and displays the trip names (wave 1)

droberts-sea · 2019-06-04T18:40:52Z

TREK

What We're Looking For

Feature	Feedback
Core Requirements
Git hygiene	Good work on the commit messages, this is a big improvement over previous projects! You probably want to add the `node_modules` folder to your `.gitignore` though.
Comprehension questions	yes
Functionality
Click a button to load and view a list of trips	yes
Clicking the "load" button twice does not cause each trip to display twice	yes
Click a trip to load and view trip details	yes
Clicking a different trip loads different details	yes
Fill out a form to reserve a spot	yes
Errors are reported to the user	no - you have some code to display status messages, but I don't think it will work, and you don't use it everywhere.
Site is clearly laid out and easy to navigate	yes
Under the Hood
Callback functions are not nested more than 2 levels deep	yes
Callback functions are given descriptive names	some - you definitely have some big complex behavior defined inside of anonymous callbacks, that might be better broken out as separate named functions.
Code is generally well-organized and easy to read	some - you have some very long functions that are a little difficult to read, and it's not super easy to track how code flows through the site
All API calls have both success and error callbacks defined	yes, though your error callbacks don't correctly report errors
Optional but recommended: closures are used to keep track of which trip is selected	no - you attach the trip ID to the DOM and read that in the callback. This works fine, but it's not quite as graceful as doing it with a closure (plus that's good practice).
HTML is semantic	some - lots of `<div>`s that could be something more meaningful
CSS is DRY, uses CSS Grid, Flexbox, and/or Bootstrap	yes
Overall	Good work overall. This code solves the problem at hand, and you've managed to get all of those tricky callbacks working just fine. I do see some room for growth around organization and particularly with error handling, and if you have some time over interview week I think you might get a lot out of revisiting and cleaning up this code. I would be happy to sit down and spend some time on that this afternoon or tomorrow morning if you'll be at Ada. But as is, this is an acceptable submission. Keep up the hard work!

droberts-sea · 2019-06-04T18:11:13Z

+<style>
+    .flex-container {
+      display: none;
+      flex-wrap: nowrap;


Why is this CSS in your HTML document? It should live in a separate .css file.

It looks like you do this for the rest of your styles... I am confused.

droberts-sea · 2019-06-04T18:23:27Z

+<body>
+  <div id="background">
+    <img src="https://i.ibb.co/RQjSQ7w/Screen-Shot-2019-05-31-at-1-28-37-PM.png" class="stretch" alt="" />
+</div>


Watch your indentation (opt+shift+f to fix this automatically in VS Code)

droberts-sea · 2019-06-04T18:24:47Z

+
+const displayStatus = (message) => {
+    $('#status').html(message);
+  };


This function tries to put messages in some element on the page that has ID status, but you've commented that section out in the HTML, which means statuses don't appear.

droberts-sea · 2019-06-04T18:25:07Z

+    }
+  }
+  content += "</ul>";
+  reportStatus(content);


On line 15, do you mean displayStatus? reportStatus isn't defined in this file.

droberts-sea · 2019-06-04T18:25:36Z

+        <p>${trip.about}</p>
+        <ul>
+        <li><b>Continent</b>: ${trip.continent}</li>
+        <li><b>Category</b>: ${trip.category}</li>


I don't know that I would use a <ul> here - what you're displaying isn't quite a list.

droberts-sea · 2019-06-04T18:28:47Z

+  $("#trip-info").empty();
+  const id = $(event.target).attr("id");
+  console.log("loading " + id);
+  showTripDetails(id);


I like the organization here, that this callback is focused on handling the DOM event and figuring out what was clicked, and showTripDetails is focused on showing details for that trip.

droberts-sea · 2019-06-04T18:29:32Z

+$(document).on("click", "#load-trips", event => {
+  $('.trip-details').hide();
+  loadTrips();
+});


All of these event handler registrations should be inside of $(document).ready.

droberts-sea · 2019-06-04T18:31:00Z

+$(document).on('submit', 'form', event => {
+  event.preventDefault();
+  console.log("running submit");
+  const trip_id = $("#trip-id").text();


This code might be easier to read if you moved some of it to the reserveTrip function.

droberts-sea · 2019-06-04T18:31:41Z

+      console.log(response);
+    })
+    .catch(function(error) {
+      console.log(error);


You should do something more than just console.log here - give the user of the site some indication of what's going on.

droberts-sea · 2019-06-04T18:33:18Z

+    .then(function (response) {
+      response.data.forEach((trip) => {
+          tripList.append(`<li><a href="" class="trip-link" id="${trip.id}">${trip.name}</a></li>`);
+      });


You should be using arrow functions for all of these callbacks.

KateAnnNichols added 12 commits May 29, 2019 15:03

set up index.css/.html/.js files with design scaffolding

19ada00

trip list displays - created empty element with trip list, axios send…

9c4628d

…s the request from JS to trip API, .then loops through the response data, and displays the trip names (wave 1)

can click on individual trips

3370a83

can view trip details from trip name

1cec18f

changed reservation button to form with submit button

9f2dff3

added flexbox

c413aef

added box for reservation form

b48934d

added title for all trips

5ee9eac

flex-container only loads after view all trips button is clicked

2e3b4e4

added error reporting

346f5ee

booking request saves to api

7b0f8b5

corrected css typo

80dcf4c

droberts-sea reviewed Jun 4, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sockets - Kate N#33

Sockets - Kate N#33
KateAnnNichols wants to merge 12 commits intoAda-C11:masterfrom
KateAnnNichols:master

KateAnnNichols commented Jun 3, 2019

Uh oh!

droberts-sea commented Jun 4, 2019

Uh oh!

droberts-sea Jun 4, 2019

Uh oh!

droberts-sea Jun 4, 2019

Uh oh!

droberts-sea Jun 4, 2019

Uh oh!

droberts-sea Jun 4, 2019

Uh oh!

droberts-sea Jun 4, 2019

Uh oh!

droberts-sea Jun 4, 2019

Uh oh!

droberts-sea Jun 4, 2019

Uh oh!

droberts-sea Jun 4, 2019

Uh oh!

droberts-sea Jun 4, 2019

Uh oh!

droberts-sea Jun 4, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

KateAnnNichols commented Jun 3, 2019

TREK

Comprehension Questions

Uh oh!

droberts-sea commented Jun 4, 2019

TREK

What We're Looking For

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants