Skip to content

Conversation

@steffnay
Copy link

Inspiration Board

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Explain the steps in creating a new Card from the form. When the form is submitted it calls the onFormSubmit method, which then calls the addCardCallback from the form's props. This method is a callback to the parent component, Board, and calls its addCard method. This method does post request to the API using the form's information.
How did you learn how to use the API? I used the documentation and Postman.
What function did you use to place the GET request from the API to get the list of cards? Why use that function? I used componentDidMount because it is automatically called when the component is rendered, which ensures that the component has the list of cards before it is rendered.
Explain the purpose of a Snapshot test. It checks to see if any html has changed in the view.
What purpose does Enzyme serve in testing a React app? It renders components for testing.

@tildeee
Copy link

tildeee commented Jul 2, 2018

Inspiration Board

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene i would encourage smaller commits with smaller changes
Comprehension questions x
General
Card Component renders the data provided as props x
Board Component takes a URL and renders the list of Cards and passes in callback functions x
NewCardform Component is a controlled form and uses a callback function to return entered data to the parent component x
API
GET request made in componentDidMount x
DELETE request made in callback function x
POST request made in callback function passed to NewCardForm component. x, should not call componentDidMount directly
Snapshot testing not updated, failing
Styling x
Overall

The code looks great! It is mostly all best practice.

My biggest piece of feedback is a HUGE RED ALERT WARNING RED FLAG: You should not call this.componentDidMount in the Board component directly as part of the addCard callback. Remember.. in React, if you update State, it will update the UI automatically. You should update cards on state instead.

Good job otherwise!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants