basic functionality working, stretch goals still to come. Styling is lacking in areas.#3
basic functionality working, stretch goals still to come. Styling is lacking in areas.#3hamzah-kurdi wants to merge 2 commits intoconstructorlabs:masterfrom
Conversation
| <meta charset="UTF-8" /> | ||
|
|
||
| <link rel="stylesheet" href="./stylesheet.css"> | ||
| <title>Hamzah Cracks React</title> |
| import Results from "./Results"; | ||
| import MoreInfo from "./MoreInfo"; | ||
|
|
||
| const config = { |
| imdbLink: "https://www.imdb.com/title/", | ||
| imdbID: "", | ||
| details: {}, | ||
| errorPic: "./assets/woops.jpg" |
There was a problem hiding this comment.
if error pic does not change, it could be better off being specified in a config or variable. Reserve state for things that will be changed by your app. Same applies to imdbLink
| recieveFilmInfo(results) { | ||
| this.setState({ | ||
| films: results.Search, | ||
| imdbID: results.Search.imdbID |
There was a problem hiding this comment.
I am not sure the Search property has an imdbID field as it's an array
| } | ||
|
|
||
| closeDetails(showDetailedInfo) { | ||
| this.setState({ |
| import React from "react"; | ||
|
|
||
| function MoreInfo({ details, closeDetails }) { | ||
| console.log(details); |
There was a problem hiding this comment.
Avoid committing console.logs
| return ( | ||
| <div> | ||
| <ul> | ||
| {films.map(film => { |
There was a problem hiding this comment.
good use of naming conventions
|
|
||
| export default Results; | ||
|
|
||
| // {film.Poster.value != "N/A" ? ( |
There was a problem hiding this comment.
Avoid committing commented out code
| addPage(event) { | ||
| event.preventDefault(); | ||
| this.setState({ | ||
| page: page + 1 |
There was a problem hiding this comment.
page is likely to be undefined here. You probably meant this.state.page
| event.preventDefault(); | ||
| if (this.state.page > 0) { | ||
| this.setState({ | ||
| page: page-- |
There was a problem hiding this comment.
page is likely to be undefined here. You probably meant this.state.page
| page: page-- | ||
| }); | ||
| this.paginationFetch(); | ||
| } else { |
There was a problem hiding this comment.
If you are on page 0, the else can be omitted as we probably don't want to do anything in this case
| import App from './App'; | ||
| import React from "react"; | ||
| import ReactDOM from "react-dom"; | ||
| import App from "./Components/App"; |
There was a problem hiding this comment.
use lower case for folder names. it will give you fewer things to worry about
| width: 300px; | ||
| } | ||
|
|
||
| #More_Info { |
There was a problem hiding this comment.
use lower case for css id and class names
2 participants
No description provided.