Skip to content

Challenge#74

Open
gabrielgs wants to merge 32 commits into
wizelineacademy:masterfrom
gabrielgs:develop
Open

Challenge#74
gabrielgs wants to merge 32 commits into
wizelineacademy:masterfrom
gabrielgs:develop

Conversation

@gabrielgs

Copy link
Copy Markdown

No description provided.

}
}

componentDidMount() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

you should do this with sagas not with state. Remember that you are required to create almost stateless components.

Comment thread src/components/Favorites/index.js Outdated
})

console.log(newFavoriteGif)
localStorage.setItem('favoriteGif', JSON.stringify(newFavoriteGif))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

you shouldn't be saving to local storage in the component, that's a sagas job

Comment thread src/components/Favorites/index.js Outdated
this.fetchTrending()
} else {
axios.get(
`https://api.giphy.com/v1/gifs/search?api_key=mjA6Ro3NCh6Q8yWu2qyxlIksx3ACBbAJ&q=${query}&limit=25&offset=0&rating=G&lang=en`

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

try to use .env for apiKeys at lear


import axios from 'axios'

class Home extends Component {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It's cool but use Redux :)

Comment thread src/components/Gifs/Gif.css Outdated
@@ -0,0 +1,33 @@
.GifContainer {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

don't use css

Comment thread src/components/Gifs/GifCard/GifCard.js Outdated

const GifCard = ({gif, addFavorites}) => {
return (
<article className="GifCard">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

don't use css and classnames

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