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.

Sign up for GitHub

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

Tahuana's assignment #6

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

Tahuana's assignment #6

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6bde0d6
React-promise assignment: task 1, including stretch goal
tahuana Feb 27, 2018
0b25393
React-promise assignment: task 2, NOT including stretch goals
tahuana Feb 27, 2018
f781dbb
.
tahuana Feb 27, 2018
80c8b28
refactoring based on TA comments
tahuana Mar 4, 2018
e577d59
implementing: strech goal 3 - reusable function for API request; and …
tahuana Mar 4, 2018
f8638ec
improving error handling
tahuana Mar 5, 2018
5c48f0e
adding a higher order component that conditionally show either an err…
tahuana Mar 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions src/App.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,37 @@ import logo from './logo.svg';
import './App.css';
import { Credentials } from './config/Config.js'
import Recipe from './components/Recipe.js'
import Error from './components/Error.js'

Copy link

Choose a reason for hiding this comment

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

It looks like you've got the concepts of a HOC down! Nice work 😄 - this is a really complicated concept!

One thing you could do to make this more generic is pass your props directly to your pass and fail components instead of manipulating the props here, similar to this:

const branch = (test, ComponentOnPass, ComponentOnFail) => props => 
test 
? < ComponentOnPass data={props} />
: <ComponentOnFail data={props} />

Then you can use the data however you'd like within the component itself, which means you could pass any components to your branch component and reuse your branch component multiple times.

One other thing to double check is that you're passing your parameters into your HOC in the order you expect them - when you call this on line 67 you're passing 1) your condition, 2) your success component (Recipe) and 3) your fail component (Error). Here, you're rendering your Error component as the success component (which I think is is what you actually want to render, since you're checking for hasError). It could be worth renaming your test boolean something like passCondition or similar so it's clear that the first returned component is the one you want rendered if your condition is true. And if you want your Recipe component to show with the success of your condition, you could change your boolean to !this.state.hasError.

Excellent work with Higher Order Components!

class App extends Component {
constructor(){
super();
this.state = {
recipeList: []
recipeList: [],
errorMessage: []
}
}

componentDidMount() {
const q = 'cakes'
const endpoint = `${Credentials.URL}?app_id=${Credentials.APP_ID}&app_key=${Credentials.APP_KEY}&q=${q}`
Copy link

Choose a reason for hiding this comment

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

nice!

//const endpoint = `${Credentials.URL}?app_id=${Credentials.APP_ID}&app_key=${Credentials.APP_KEY}`

const fetchRecipes = () =>
Copy link

Choose a reason for hiding this comment

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

Consider pulling this function outside of the scope of componentDidMount

Something like this

 componentDidMount() {
  this.fetchRescipe()
}
fetchRecipes = () => <your fancy code here>

fetch(endpoint)
.then(response => response.json())
.then(response => this.setState({recipeList: response.hits}))
.catch( (error) => console.log(error))
.catch( (error) => {
console.log("error: ", error)
this.setState({errorMessage: [error.message]})
})

fetchRecipes();

}

render() {

return (
<div className="App">
<header className="App-header">
Expand All @@ -35,11 +42,14 @@ class App extends Component {
</header>
<div className="recipe-list">
<p>Recipe List:</p>
{this.state.recipeList.map(recipe => <Recipe myRecipe = {recipe} />)}
{this.state.recipeList.map(recipe => <Recipe myRecipe = {recipe} />)}
{this.state.errorMessage.map(error => <Error message={error} />)}
</div>
</div>
);
}
}

export default App;


20 changes: 20 additions & 0 deletions src/components/Error.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import React, { Component } from 'react';

class Error extends Component {
constructor(props){
super(props)
}
Copy link

Choose a reason for hiding this comment

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

Same with this constructor! You don't need this :)


render() {
console.log("estou no erro")
return (
<div>
<p>Ops... we got an error: {this.props.message}</p>
</div>
)
}
}
Copy link

Choose a reason for hiding this comment

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

since you're not manipulating state or using any of the lifecycle hooks, you can make this a stateless component. Rather than extending the Component class, you can do something like this:

export const Error = (props) =>
    <div>
    {
        console.log("estou no erro")
    }

        <p>Ops... we got an error: {props.message}</p>
    </div>

export default Error;

You can try doing the same with your recipe component as well!



export default Error;