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

Anibel - Edges Inspiration Board #19

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

Conversation

anibelamerica
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. A user enters text or selects an emoji from a dropdown list. This event will update the text and emoji states in the NewCardForm component. Upon submission, these states are sent up to a newCard method in the Board component via a callback. The method posts a requests to the API and a new card is either successfully created or errors are stored in state. If the card is successfully created, the Board's card state is updated with the new card.
How did you learn how to use the API? I practiced API calls in 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 a componentDidMount() to get the list of cards from the API so that I can set the state and rerender the component within that lifecycle.
Explain the purpose of a Snapshot test. A snapshot tests whether our component is outputting the expected HTML.
What purpose does Enzyme serve in testing a React app? Enzyme has methods to create shallow and deep rendering of a component (or component and nested components for deep mounting) for testing.

@droberts-sea
Copy link

Inspiration Board

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene yes
Comprehension questions yes
General
Card Component renders the data provided as props yes
Board Component takes a URL and renders the list of Cards and passes in callback functions yes
NewCardform Component is a controlled form and uses a callback function to return entered data to the parent component yes
API
GET request made in componentDidMount yes
DELETE request made in callback function yes
POST request made in callback function passed to NewCardForm component. yes
Snapshot testing yes
Styling yes
Overall Good work overall. I've left a couple of inline comments below, but in general it's clear to me that the learning goals around building a complete interactive react app and integrating it with an API were met. Keep up the hard work!


this.state = {
cards: [],
url: this.props.url,
boardName: this.props.boardName,

Choose a reason for hiding this comment

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

If the URL and board name aren't going to be changed within this component, you don't need to keep them in state. If you read them from props throughout the app, it will be more clear that they're read-only.

const POST_INSPO_CARDS_URL = this.props.url + '/' + this.props.boardName + '/cards';
axios.post(POST_INSPO_CARDS_URL, cardData)
.then((response) => {
cardData.id = response.data.card.id;

Choose a reason for hiding this comment

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

I like that you've kept all the API interaction logic in one component - the callbacks are a little more complex, but I would say it makes the app as a whole much easier to comprehend. Whether or not you intended it, this is a great example of the container component pattern well-applied.

resetState = () => {
this.setState({
text: '',
emoji: ''

Choose a reason for hiding this comment

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

Good use of a helper method here. Could you call this from your constructor instead of repeating this work?

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