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

Selam - Ampers #34

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

Selam - Ampers #34

wants to merge 5 commits into from

Conversation

SelamawitA
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 form is styled in the render section where there is a label expecting a user to input a text and emoji. Helper functions are referenced so that when a form is submitted both fields would clear and is responsive to a keyboard. After a user has hit the submit button a callback function to Card is referenced passing in the fieldName & fieldValue which later on update the number of cards.
How did you learn how to use the API? I read through the documentation, looked at the JSON structure within values that had been previously posted, and played with requests 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 for the get request, calling to an API is asynchronous and can cause later lines of code to break if the content from the get request hasn't been loaded completely yet. Using componentDidMount with the get requests ensures that the cards are loaded after the constructor is instantiated.
Explain the purpose of a Snapshot test. To help a developer keep track of and ensure that small changes to the views created in React are being modified as expected.
What purpose does Enzyme serve in testing a React app? Enzyme provides shallow and mounted snapshot testing options.

@SelamawitA SelamawitA changed the title Selam - & Selam - Ampers Jun 18, 2018
import './Card.css';

function convertToemoji(anEmoji){
Copy link

Choose a reason for hiding this comment

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

proper camel case would be convertToEmoji

class Card extends Component {
findID = () => {
Copy link

Choose a reason for hiding this comment

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

not a very great function name, maybe something like just deleteCard would be better

@tildeee
Copy link

tildeee commented Jul 5, 2018

Inspiration Board

What We're Looking For

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

Overall, looks good -- my only comments would be on naming conventions.

Good job on this project!

import emoji from 'emoji-dictionary';
import './NewCardForm.css';

const EMOJI_LIST = ["", "heart_eyes", "beer", "clap", "sparkling_heart", "heart_eyes_cat", "dog"]


class NewCardForm extends Component {
Copy link

Choose a reason for hiding this comment

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

watch your indentation throughout this entire component!

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