The movie db #4
The movie db #4
Conversation
package.json
Outdated
| @@ -4,7 +4,10 @@ | |||
| "private": true, | |||
| "dependencies": { | |||
| "inspector": "^0.5.0", | |||
| "jquery": "^3.3.1", | |||
| "lodash.uniqueid": "^4.0.1", | |||
There was a problem hiding this comment.
install full version of lodash
src/components/AfficheList.js
Outdated
| </div> | ||
| ); | ||
| import $ from 'jquery'; | ||
| import uniqueId from 'lodash.uniqueid'; |
There was a problem hiding this comment.
After installation full version of lodash write:
import uniqueId from 'lodash/uniqueId';
src/components/AfficheList.js
Outdated
| super(props); | ||
| this.state = {} | ||
|
|
||
| this.performSearch('avengers'); |
There was a problem hiding this comment.
move it in componentDidMount. For more info read:
https://daveceddia.com/ajax-requests-in-react/,
reactjs/react.dev#302
src/components/AfficheList.js
Outdated
|
|
||
| constructor(props) { | ||
| super(props); | ||
| this.state = {} |
There was a problem hiding this comment.
If you just initialize state, then you don't need the constructor. Move state = {} outside of constructor and remove constructor at all. Before removing constructor read next comment. Read: https://www.robinwieruch.de/react-state-without-constructor/
| performSearch(searchWord) { | ||
| console.log('Perform search using moviedb'); | ||
| const urlString = 'https://api.themoviedb.org/3/search/movie?api_key=0db50d1e81184cc04e761a3e55b0ee62&query=' + searchWord; | ||
| $.ajax({ |
There was a problem hiding this comment.
Usage of jquery only for ajax request is redundant. You install big library and use only one feature from it. Instead of jquery look at axios(https://github.com/axios/axios) or use native fetch(https://scotch.io/tutorials/how-to-use-the-javascript-fetch-api-to-get-data) api. Axios is preferably.
| movieRows.push(movieRow); | ||
| }) | ||
| this.setState({ | ||
| rows: movieRows |
There was a problem hiding this comment.
movieRows: movieRows
|
|
||
| searchChangeHandler = e => { | ||
| const searchWord = e.target.value; | ||
| this.performSearch(searchWord) |
There was a problem hiding this comment.
Right now when the user enters one letter your app makes the request. For word 'hero' your app will make 4 requests. Look at lodash debounce to fix multiple requests issue.
| <h4>{ time }<sub>minutes</sub></h4> | ||
| <p>{ description }</p> | ||
| <div className="price-tag" key={movie.id}> | ||
| <img src={ movie.poster_src } alt="poster"/> |
There was a problem hiding this comment.
Better to use movie name and word poster for alt attribute. example: 'Superman poster'
| <h4>{ time }<sub>minutes</sub></h4> | ||
| <p>{ description }</p> | ||
| <div className="price-tag" key={movie.id}> | ||
| <img src={ movie.poster_src } alt="poster"/> |
There was a problem hiding this comment.
destructure movie properties.
| @@ -29,4 +30,8 @@ class Authorization extends Component { | |||
| } | |||
| } | |||
|
|
|||
| Authorization.propTypes = { | |||
| onTriggerModal: PropTypes.func | |||
There was a problem hiding this comment.
should be isRequired:
onTriggerModal: PropTypes.func.isRequired
There was a problem hiding this comment.
Check other places where you use PropTypes. If the property is required then add isRequired.
| <li><a href="#" className="footer-menu-link">Food and Drinks</a></li> | ||
| <li><a href="#" className="footer-menu-link">Info</a></li> | ||
| <li><a href="#" className="footer-sign-in sign-in-btn footer-menu-link" onClick={ onHandleTriggerModal }>Sign in</a></li> | ||
| <li><Link to="/" className="footer-top__menu__link">Home</Link></li> |
There was a problem hiding this comment.
Wrong BEM naming. Read https://css-tricks.com/bem-101/
| </ul> | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| FooterNavigation.propTypes = { | ||
| onHandleTriggerModal: PropTypes.func |
|
|
||
| return ( | ||
| <> | ||
| { isOpenMiniMenu && <MiniMenu onHandleTriggerModal={ onHandleTriggerModal } handleRemoveMiniMenu={ this.handleRemoveMiniMenu } onAnimatedClass={ animatedClass } handleTriggerSearch={ this.handleTriggerSearch } isOpenSearchInput={ isOpenSearchInput } /> } | ||
| { isOpenMiniMenu && <MiniMenu onHandleTriggerModal={ onHandleTriggerModal } handleRemoveMiniMenu={ this.handleRemoveMiniMenu } handleTriggerSearch={ this.handleTriggerSearch } isOpenSearchInput={ isOpenSearchInput } /> } |
There was a problem hiding this comment.
format to:
{ isOpenMiniMenu && ( <MiniMenu onHandleTriggerModal={ onHandleTriggerModal } handleRemoveMiniMenu={ this.handleRemoveMiniMenu } handleTriggerSearch={ this.handleTriggerSearch } isOpenSearchInput={ isOpenSearchInput } /> )}
| performSearch() { | ||
| console.log('Perform search using moviedb'); | ||
| const urlString = 'https://api.themoviedb.org/3/movie/popular?api_key=0db50d1e81184cc04e761a3e55b0ee62&language=en-US&page=1'; | ||
| $.ajax({ |
| transition: 0.5s; | ||
| } | ||
| .price-tag:hover { | ||
| cursor: default; | ||
| box-shadow: 0px 0px 60px 10px rgba(204, 204, 223, 0.71); | ||
| } | ||
| .price-tag img { |
There was a problem hiding this comment.
Add class for img, use classes for styling.
| } | ||
| .price-tag span { | ||
| font-size: 1.4rem; | ||
| .price-tag img:hover { |
| cursor: pointer; | ||
| opacity: 0.9; | ||
| } | ||
| .price-tag h2 { |
| text-transform: uppercase; | ||
| margin: 3rem auto 1rem; | ||
| margin: 0 auto; | ||
| color: #4f4f6f; | ||
| } | ||
| .price-tag h4 { |
| width: 50%; | ||
| } | ||
| .footer-menu ul { | ||
| .footer-top__menu ul { |
| display: flex; | ||
| justify-content: space-around; | ||
| list-style: none; | ||
| } | ||
| .footer-menu-link { | ||
| .footer-top__menu__link { |
There was a problem hiding this comment.
incorrect BEM naming
Adding themoviedb to the project and implementation of search