Skip to content
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

Inspiration board Katherine #39

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

KatherineJF
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. set state of newFormCard class to watch "text" and "emoji" for changes. Create functions to map those values to fields. Create a function with a callBack to the addCard method in Board it updates the card array when the form is submitted. Render the form and index through available emojis to create a drop-down list in the form.
How did you learn how to use the API? Followed examples in Ada Pets.
What function did you use to place the GET request from the API to get the list of cards? Why use that function? componentDidMount is the best place to put calls to fetch data, using DidMount makes it clear that data won’t be loaded until after the initial render. So you don’t end up with undefined state that causes errors.
Explain the purpose of a Snapshot test. Snapshot testing is regression testing it ensure stability after a change is made.
What purpose does Enzyme serve in testing a React app? Enzyme makes testing easier by adding additional utility methods for rendering a component (or multiple components), finding elements, and interacting with elements.

@KatherineJF KatherineJF changed the title Inspiration board Inspiration board Katherine Dec 18, 2018
@CheezItMan
Copy link

Inspiration Board

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Only two commits???
Comprehension questions Check
General
Card Component renders the data provided as props Check
Board Component takes a URL and renders the list of Cards and passes in callback functions Check
NewCardform Component is a controlled form and uses a callback function to return entered data to the parent component Check
API
GET request made in componentDidMount Check
DELETE request made in callback function Check
POST request made in callback function passed to NewCardForm component. Check, but with a bug because you're not putting the ID assigned by the API into the card. This means that recently added cards can't be deleted.
Snapshot testing Check
Styling Check
Overall Well done, you hit the major learning goals of the project. You do have a bug with your add-delete actions, see my inline notes.


this.state = {
cards: [],
};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should also set the state of the error messages here, otherwise the push will fail.

})
.catch((error) => {
let { errors } = this.state
errors.push(error.message)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fail, unless errors is initialized prior, or you use the |= operator.

// What should we do when we know the post request worked?
console.log('we definitely have a new card!', cardData);

const updatedBoard = [ ...this.state.cards, cardData]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're missing the id for cardData here.

You should have:

cardData.id = response.data.card.id;

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