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

Hayden - Edges (C10) - Inspo Board #44

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

Conversation

haydenwalls
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. the controlled NewCardForm stores any input into the form elements of the component in its own state. when the user submits the form, a callback func that had previously been passed to NCF via prop from Board is triggered, allowing Board to make an API call to backend adding the new card to the DB, and also updating its own state to include the new card :)
How did you learn how to use the API? dan taught me (jk i read the docs of course!)
What function did you use to place the GET request from the API to get the list of cards? Why use that function? componentDidMount. we use that function because it runs once and only once at the beginning of a component's lifecycle.
Explain the purpose of a Snapshot test. similar to a VHS cartridge that lets you record an API call, it allows you to take a snapshot of the most important details of a page and test the visual aspects of your app against a snapshot that you know is valid
What purpose does Enzyme serve in testing a React app? honestly, i'm not sure because we learned about it for all of 2 minutes. the lecture notes say it can select / manipulate the DOM similar to jQuery. i know you can tell it to take either a shallow or deep snapshot when 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 integrating a React app with an API have been met. Keep up the hard work!

delCard = (id) => {
const url = 'https://inspiration-board.herokuapp.com/cards/' + id.toString();

axios.delete(url)

Choose a reason for hiding this comment

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

You should use complete words for variable and function names. Your jargon may feel obvious to you, but it will often be less so to your coworkers.


axios.post(url)
.then((res) => {
let newState = this.state;

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 place - 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.

<select className="new-card-form__form-select" name="emoji" onChange={this.onFormChange}>
<option value=""></option>
<option value="heart_eyes">😍</option>
<option value="beer">🍺</option>

Choose a reason for hiding this comment

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

Could you generate these <option> tags dynamically from EMOJI_LIST?

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