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

Caroline Nardi - Inspiration Board - Octos #35

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

Conversation

cmn53
Copy link

@cmn53 cmn53 commented Jun 18, 2018

Inspiration Board

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Explain the steps in creating a new Card from the form. Initially the form used to create a new card has state values set equal to empty strings. When the input/select values are changed, the onInputChange function is called and the form state is updated to match the input. When the form is submitted, the onFormSubmit function is called, which then invokes the addCard function in the Board component. The addCard function uses the form state to make a post request to the API and updates the board to include the new card. Finally, the form state values are reset to empty strings.
How did you learn how to use the API? I looked at the documentation that was provided and tested it 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 componentDidMount to make sure that the component had rendered before making the API call.
Explain the purpose of a Snapshot test. Snapshot tests let us compare the html from a previous point in time, or snapshot, with the html at a later point (after refactoring, fixing something, adding a new feature, etc.) to see if the page has changed in the way that we expect.
What purpose does Enzyme serve in testing a React app? I think it either takes the snapshots, or just makes them more legible.

@Hamled
Copy link

Hamled commented Jun 22, 2018

Inspiration Board

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene
Comprehension questions
General
Card Component renders the data provided as props
Board Component takes a URL and renders the list of Cards and passes in callback functions
NewCardform Component is a controlled form and uses a callback function to return entered data to the parent component
API
GET request made in componentDidMount
DELETE request made in callback function
POST request made in callback function passed to NewCardForm component.
Snapshot testing
Styling
Overall

}

onClickButton = () => {
this.props.deleteCardCallback(this);
Copy link

Choose a reason for hiding this comment

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

I'm not sure if there's a specific rule about this in React, but I think it's generally not considered best practice to pass a component instance itself to a callback.

In the deleteCard function from the Board component we're only using the id and index props, so we could just pass those directly instead.

import { shallow } from 'enzyme';

describe('Card', () => {
test('shallow mount', () => {
Copy link

Choose a reason for hiding this comment

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

It could also be helpful to have additional tests for Card which check the snapshot if the component is rendered with a specific set of props.

Right now we're providing the id, index, and deleteCardCallback props, but none of these impact on the rendering. I think we could have further tests which provide the text and emoji props (one for each, and a third for when both are provided), as those props do impact the visual result.

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