Skip to content

1st delivery (Eliahu Cabasso)#54

Open
EC-AI wants to merge 19 commits into
wizelineacademy:masterfrom
EC-AI:master
Open

1st delivery (Eliahu Cabasso)#54
EC-AI wants to merge 19 commits into
wizelineacademy:masterfrom
EC-AI:master

Conversation

@EC-AI

@EC-AI EC-AI commented Jul 6, 2018

Copy link
Copy Markdown

1st delivery

@EC-AI EC-AI changed the title Added navbar with search field (Initial PR) 1st delivery Jul 9, 2018
@EC-AI EC-AI changed the title 1st delivery 1st delivery (Eliahu Cabasso) Jul 9, 2018
constructor(props){
super(props);
this.state = {
hasError: false

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 nice to have an info var to store the errorInfo

}

componentDidCatch() {
this.setState(() => ({hasError: true}));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍


class ExplorePanel extends Component {

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.

👍

return (
<div>
<SearchBar />
<ErrorBoundary message="Unable to load.">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The message attribute value Unable to load may have not the most accurate content as this component will show on the tree despite having an error thrown or not.

Comment thread src/components/MainPanel/GifView.js Outdated
import Star from './Star';

const GifView = ({ src, id, favorite, gif }) => (
<div style={{display: 'inline-block', padding: 10}} className="GifView">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Avoid inline styles

<Switch>
<Route path="/explore" component={ExplorePanel} />
<Route path="/favorites" component={FavoritesPanel} />
<Redirect to="/explore" />

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 nice feature to send an object to handle the redirect and let the user know what happened before the redirect.

Comment thread src/constants/index.js Outdated
@@ -0,0 +1,5 @@
export default {
API_KEY: 'pL67xABMUz8kJ7kAF9LIomqHGMueUK56',

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 an .env file to store your API key and dont version that file. You can provide an .ev.example file to express the usage of that value.

https://www.npmjs.com/package/dotenv


const NavBar = (props) => (
<StyledNavBar>
<StyledNavLink

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

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