Skip to content

First Delevery#73

Open
alfruiz wants to merge 38 commits into
wizelineacademy:masterfrom
alfruiz:master
Open

First Delevery#73
alfruiz wants to merge 38 commits into
wizelineacademy:masterfrom
alfruiz:master

Conversation

@alfruiz

@alfruiz alfruiz commented Jul 9, 2018

Copy link
Copy Markdown

DONE !!

  • Redux ( actions / reducers / middleware / selectors / store )
  • Redux-saga (localStorage & axios)
  • React Router ( BrowserHistory / Route / Switch / Redirect )
  • JEST ( Enzyme / redux-mock-store / Snaptshots / redux "actions & reducers" / redux-saga )
  • Styled Component ( with classes / Own styled / jest / snaptshops / injectGlobal )
  • React 16 ( Error boundary / Fragments)

Final .... Here is the Coverage -- at least 70%

screen shot 2018-07-12 at 5 24 15 pm

@timgivois @Robfz @bgilm
Thanks for the comments all the teaching , support & and all =)

Comment thread .env.example Outdated


###### IMPORTANT KEYS ######
YOUR_API_KEY=BtpjCc5CrNS0nxVvRcC8PDz2u3KeW8Bq

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I guess this is not your actual key, right?

@alfruiz alfruiz Jul 10, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yep, I didnt know if you need it so I just copy on .env
Should I removed?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yeah remove it, with the name variable is enough to know we should use the key.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

Comment thread src/App.js Outdated
<Switch>
<Route path="/keeps" component={Keeps} />
<Route path="/" component={Home} />
<Redirect to="/" />

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 is a good practice to send the object on the redirect so you can let the user know what happened before the redirect.

Comment thread src/App.js Outdated
To get started, edit <code>src/App.js</code> and save to reload.
</p>
</div>
<React.Fragment>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

Comment thread src/App.js
<Switch>
<Route path="/home" component={Home} />
<Route path="/keeps" component={Favorites} />
<Redirect to={{

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

DONE & THX

import './footer.css';

const Footer = () => (
<footer className="footer">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

use styled components not css

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

DONE

@@ -0,0 +1,11 @@
import React from "react";

import './footer.css';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

use styled-components

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

DONE

Comment thread src/App.js

const App = ({ ...props }) => {
return (
<React.Fragment>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

thx

@@ -0,0 +1,42 @@
import React, { Component } from "react";

import './giphys.css';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Styled components

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

DOME

const Navegation = () => (
<header>
<ul>
<li><NavLink to="/home" activeclass="active">HOME</NavLink></li>

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 can style any component with styledcomponents, avoid using classname.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

DONE

Comment thread src/redux/sagas/fetch.js Outdated
let url = `${baseUrl}${word.length > 0 ? 'search' : 'trending'}?api_key=${process.env.YOUR_API_KEY}&limit=10&rating=G${query}`;
const data = yield call(fetchData, url);

yield put({ type: actions.FETCH_DATA, payload: { data } });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use action creators for this.


return newState;
}
case actions.LOCALSTORAGE_MODIFY: {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would suggest you to invert the data flow regarding the store and localStorage. You should first store the favorites in the Store and have a subscription/saga/middleware hearing the proper actions to just dump the state to localStorage when needed.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Actually if you check after LOCALSTORAGE_MODIFY redux saga runs localModifyStorage
whichs inject that in the window.localStorage

check inside sagas folder: local.js
localModifyStorage generator

const fetch_word = state => state.fetch.word;

// LOCAL
const local = state => state.local;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would suggest you to have more descriptive names.

@alfruiz alfruiz Jul 12, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Got IT @Robfz

Comment thread package.json
"lint": "eslint ./",
"test": "yarn lint && yarn jest",
"jest": "react-scripts test --env=jsdom --coverage"
"test": "yarn lint && yarn jest --watch",

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 change this things

@alfruiz alfruiz Jul 12, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I only add watch to avoid strating this all the time @timgivois

@timgivois

Copy link
Copy Markdown

You changed all the jest configuration. Had to change it again to make it work.

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.

4 participants