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

Anne Watson - Octos - Inspiration Board #25

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

Conversation

annehwatson
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. Enter input into the form fields. Changes to the field's values call the onInputChange function and updated the state. When the form is submitted by clicking the button, the onSubmit function passes the state into board via the addCardCallback.
How did you learn how to use the API? Read the documentation; used Postman
What function did you use to place the GET request from the API to get the list of cards? Why use that function? componentDidMount. Want to make sure that the component rendered before making the api request.
Explain the purpose of a Snapshot test. Snapshot tests ensure that you don't accidentally make a change once you take a "snapshot" of the desired output of your app components by notifying you when there is a diff.
What purpose does Enzyme serve in testing a React app? Enzyme provides different methods to test components (including shallow and mount). Shallow isolates the test assertions to a single component (just the parent, not the children). You would need to use mount if you wanted to look at children components.
Summary

@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 Environment not set up correctly - see inline
Styling yes - good use of BEM
Overall Good job overall! There are a few small issues which I've tried to point out inline, but for the most part this code is clean and well-organized, and it's obvious the learning goals were met. Keep up the hard work!

Copy link

@droberts-sea droberts-sea left a comment

Choose a reason for hiding this comment

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

Whoops, forgot to actually submit these.

exports[`Board shallow mount 1`] = `
ShallowWrapper {
"length": 1,
Symbol(enzyme.__root__): [Circular],

Choose a reason for hiding this comment

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

This snapshot doesn't look right - it should basically just be HTML. My guess is you're missing the enzyme JSON encoder, or don't have Jest set up to use it. See https://github.com/Ada-Developers-Academy/textbook-curriculum/blob/master/React/testing.md#installing-enzyme--react-test-renderer

Choose a reason for hiding this comment

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

This ended up making the tests fail for me, since some of these serialized Enzyme objects have absolute paths from your filesystem that contain your username.

<div>
<label className="new-card-form__form-label" htmlFor="emoji">Emoji</label>
<input className="new-card-form__form-select" type="text"
name="emoji"

Choose a reason for hiding this comment

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

The emoji input should be a dropdown menu - otherwise how will the user know what their options are?

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