From ed46bdcf5e3623e069f5c41eb8013cda5629f815 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Delattre?= Date: Tue, 18 Apr 2017 14:32:40 +0200 Subject: [PATCH 1/3] Double check props before exploring movies / shows --- .../js/components/list/explorerOptions.js | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/public/js/components/list/explorerOptions.js b/src/public/js/components/list/explorerOptions.js index d71e0ea..f4ed65b 100644 --- a/src/public/js/components/list/explorerOptions.js +++ b/src/public/js/components/list/explorerOptions.js @@ -11,9 +11,11 @@ export default class ExplorerOptions extends React.Component { // Check if the options are present if (Object.keys(this.props.options).length === 0) { // Fetch options - this.props.fetchOptions(); + props.fetchOptions(); // Explore - this.props.explore(this.props.params.source, this.props.params.category); + if (this.propsValid(props)) { + props.explore(props.params.source, props.params.category); + } } else { source = this.props.params.source; category = this.props.params.category; @@ -42,12 +44,19 @@ export default class ExplorerOptions extends React.Component { this.setState({ selectedCategory: event.target.value }); this.props.router.push(`/${this.props.type}/explore/${this.state.selectedSource}/${event.target.value}`); } + propsValid(props) { + if (!props.params + || !props.params.source + || !props.params.category + || (props.params.source === "") + || (props.params.category === "")) { + return false + } + return true; + } componentWillUpdate(nextProps, nextState) { // Check props - if (!nextProps.params.source - || !nextProps.params.category - || (nextProps.params.source === "") - || (nextProps.params.category === "")) { + if (!this.propsValid(nextProps)) { return } From fa73dc09c25b54b3a33c599c4257a59955655ac3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Delattre?= Date: Wed, 19 Apr 2017 13:41:34 +0200 Subject: [PATCH 2/3] Simplify the search module --- src/public/js/components/movies/list.js | 37 +++--------------------- src/public/js/components/navbar.js | 38 ++++++++++++++++++++++--- src/public/js/components/shows/list.js | 35 ++--------------------- 3 files changed, 40 insertions(+), 70 deletions(-) diff --git a/src/public/js/components/movies/list.js b/src/public/js/components/movies/list.js index c3421aa..2b43b6b 100644 --- a/src/public/js/components/movies/list.js +++ b/src/public/js/components/movies/list.js @@ -41,41 +41,12 @@ function MovieButtons(props) { } export default class MovieList extends React.Component { - handleParams(props = this.props) { - // Check if the URL to fetch is in the props - if (props.moviesUrl) { - this.props.fetchMovies(props.moviesUrl); - return - } - - // Check for URL parameters - if (!props.params) { - return - } - - // Search param - if (props.params.search && props.params.search !== "") { - props.searchMovies({ - key: props.params.search - }); - return - } + constructor(props) { + super(props); } componentWillMount() { - this.handleParams(); - } - componentWillUpdate(nextProps, nextState) { - // No params - if (!nextProps.params) { - return - } - - // Search field changed - if (this.props.params.search - && (this.props.params.search !== nextProps.params.search) - && (nextProps.params.search !== "")) { - this.handleParams(nextProps); - return + if (this.props.moviesUrl) { + this.props.fetchMovies(this.props.moviesUrl); } } render() { diff --git a/src/public/js/components/navbar.js b/src/public/js/components/navbar.js index 97cc8ac..51fbc1f 100644 --- a/src/public/js/components/navbar.js +++ b/src/public/js/components/navbar.js @@ -30,8 +30,10 @@ export default function NavBar(props) { placeholder="Search movies" router={props.router} search={props.movieStore.search} + searchFunc={props.searchMovies} path='/movies/search' pathMatch='movies' + params={props.params} /> @@ -52,14 +56,40 @@ class Search extends React.Component { constructor(props) { super(props); this.handleSearch = this.handleSearch.bind(this); + this.search(this.props); } handleSearch() { - this.props.router.push(`${this.props.path}/${encodeURI(this.props.search)}`) + if (this.props.search === "") { + return; + } + this.search(this.props); + this.props.router.push(`${this.props.path}/${encodeURI(this.props.search)}`); + } + isActive() { + const location = this.props.router.getCurrentLocation().pathname; + return (location.indexOf(this.props.pathMatch) !== -1) + } + search(props) { + if (!this.isActive()) { + return; + } + + // Search from the props if defined + if (props.search !== "") { + props.searchFunc({ key: props.search }); + return; + } + + // Search from the url params + if (props.params + && props.params.search + && props.params.search !== "") { + props.searchFunc({ key: props.params.search }); + return; + } } render() { - const location = this.props.router.getCurrentLocation().pathname; - if (location.indexOf(this.props.pathMatch) === -1) - { + if (!this.isActive()) { return null; } diff --git a/src/public/js/components/shows/list.js b/src/public/js/components/shows/list.js index e783c49..91a2e17 100644 --- a/src/public/js/components/shows/list.js +++ b/src/public/js/components/shows/list.js @@ -6,40 +6,9 @@ import Loader from '../loader/loader' import ShowButtons from './listButtons' export default class ShowList extends React.Component { - handleParams(props = this.props) { - // Check if the URL to fetch is in the props - if (props.showsUrl) { - this.props.fetchShows(props.showsUrl); - return - } - - // Check for URL parameters - if (!props.params) { - return - } - - // Search param - if (props.params.search && props.params.search !== "") { - this.props.searchShows({ - key: this.props.params.search - }); - return - } - } componentWillMount() { - this.handleParams(); - } - componentWillUpdate(nextProps, nextState) { - // No params - if (!nextProps.params) { - return - } - - // Search field changed - if (this.props.params.search - && (this.props.params.search !== nextProps.params.search) - && (nextProps.params.search !== "")) { - this.handleParams(nextProps); + if (this.props.showsUrl) { + this.props.fetchShows(this.props.showsUrl); return } } From a44cdd3a4275d47c8185606c2685e593bd6b5b55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Delattre?= Date: Wed, 19 Apr 2017 14:44:08 +0200 Subject: [PATCH 3/3] Update explore view to a more stable version * no more state * loading is in the posters module, not the whole page --- .../js/components/list/explorerOptions.js | 71 ++++++++----------- src/public/js/components/list/posters.js | 66 +++++++++++------ src/public/js/components/movies/list.js | 7 +- src/public/js/components/shows/list.js | 7 +- 4 files changed, 75 insertions(+), 76 deletions(-) diff --git a/src/public/js/components/list/explorerOptions.js b/src/public/js/components/list/explorerOptions.js index f4ed65b..bd8b83d 100644 --- a/src/public/js/components/list/explorerOptions.js +++ b/src/public/js/components/list/explorerOptions.js @@ -4,45 +4,29 @@ import { Form, FormGroup, FormControl, ControlLabel } from 'react-bootstrap' export default class ExplorerOptions extends React.Component { constructor(props) { super(props); - let source = null; - let category = null; - let categories = []; - - // Check if the options are present if (Object.keys(this.props.options).length === 0) { - // Fetch options - props.fetchOptions(); - // Explore - if (this.propsValid(props)) { - props.explore(props.params.source, props.params.category); - } - } else { - source = this.props.params.source; - category = this.props.params.category; - categories = this.props.options[this.props.params.source]; + this.props.fetchOptions(); + } + + // Initial explore + if (this.propsValid(props)) { + props.explore(props.params.source, props.params.category); } - this.state = { - selectedSource: source, - selectedCategory: category, - categories: categories, - }; this.handleSourceChange = this.handleSourceChange.bind(this); this.handleCategoryChange = this.handleCategoryChange.bind(this); } handleSourceChange(event) { let source = event.target.value; let category = this.props.options[event.target.value][0]; - this.setState({ - selectedSource: source, - selectedCategory: category, - categories: this.props.options[source], - }); + this.props.explore(source, category); this.props.router.push(`/${this.props.type}/explore/${source}/${category}`); } handleCategoryChange(event) { - this.setState({ selectedCategory: event.target.value }); - this.props.router.push(`/${this.props.type}/explore/${this.state.selectedSource}/${event.target.value}`); + let source = this.props.params.source; + let category = event.target.value; + this.props.explore(source, category); + this.props.router.push(`/${this.props.type}/explore/${source}/${category}`); } propsValid(props) { if (!props.params @@ -60,20 +44,17 @@ export default class ExplorerOptions extends React.Component { return } + // No previous params + if (!this.props.params.source && !this.props.params.category) { + this.props.explore(this.props.params.source, this.props.params.category); + return; + } + // Explore params changed if ((this.props.params.source !== nextProps.params.source) || (this.props.params.category !== nextProps.params.category)) { this.props.explore(nextProps.params.source, nextProps.params.category); - } - - // State must be updated - if ((this.state.selectedSource !== nextProps.params.source) - || (this.state.selectedCategory !== nextProps.params.category)) { - this.setState({ - selectedSource: nextProps.params.source, - selectedCategory: nextProps.params.category, - categories: nextProps.options[nextProps.params.source], - }); + return; } } prettyName(name) { @@ -93,11 +74,15 @@ export default class ExplorerOptions extends React.Component { return null; } - // State is not yet set - if (!this.state.selectedSource && !this.state.selectedCategory) { - return null; + // Invalid props + if (!this.propsValid(this.props)) { + return } + let source = this.props.params.source; + let category = this.props.params.category; + let categories = this.props.options[this.props.params.source]; + return (
@@ -110,7 +95,7 @@ export default class ExplorerOptions extends React.Component { bsClass="form-control input-sm" componentClass="select" onChange={this.handleSourceChange} - value={this.state.selectedSource} + value={source} > {Object.keys(this.props.options).map(function(source) { return () @@ -125,9 +110,9 @@ export default class ExplorerOptions extends React.Component { bsClass="form-control input-sm" componentClass="select" onChange={this.handleCategoryChange} - value={this.state.selectedCategory} + value={category} > - {this.state.categories.map(function(category) { + {categories.map(function(category) { return () }, this)} diff --git a/src/public/js/components/list/posters.js b/src/public/js/components/list/posters.js index 367bccb..3789dc4 100644 --- a/src/public/js/components/list/posters.js +++ b/src/public/js/components/list/posters.js @@ -7,6 +7,8 @@ import ListFilter from './filter' import ExplorerOptions from './explorerOptions' import ListPoster from './poster' +import Loader from '../loader/loader' + export default class ListPosters extends React.Component { constructor(props) { super(props); @@ -78,30 +80,52 @@ export default class ListPosters extends React.Component { options={this.props.exploreOptions} explore={this.props.explore} /> - {elmts.length === 0 && -
-

No result

-
- } - - {elmts.map(function(el, index) { - const selected = (el.imdb_id === this.props.selectedImdbId) ? true : false; - return ( - this.props.onClick(el.imdb_id)} - /> - )} - , this)} - + selectedImdbId={this.props.selectedImdbId} + onClick={this.props.onClick} + />
); } } + +function Posters(props) { + if (props.loading) { + return (); + } + + if (props.elmts.length === 0) { + return ( +
+

No result

+
+ ); + } + + return ( +
+ + {props.elmts.map(function(el, index) { + const selected = (el.imdb_id === props.selectedImdbId) ? true : false; + return ( + props.onClick(el.imdb_id)} + /> + )} + )} + +
+ ); +} diff --git a/src/public/js/components/movies/list.js b/src/public/js/components/movies/list.js index 2b43b6b..fb39b7d 100644 --- a/src/public/js/components/movies/list.js +++ b/src/public/js/components/movies/list.js @@ -5,7 +5,6 @@ import TorrentsButton from './torrents' import ActionsButton from './actions' import ListPosters from '../list/posters' import ListDetails from '../list/details' -import Loader from '../loader/loader' function MovieButtons(props) { const imdb_link = `http://www.imdb.com/title/${props.movie.imdb_id}`; @@ -58,11 +57,6 @@ export default class MovieList extends React.Component { } const selectedMovie = movies[index]; - // Loading - if (this.props.movieStore.loading) { - return (); - } - return (
{selectedMovie && diff --git a/src/public/js/components/shows/list.js b/src/public/js/components/shows/list.js index 91a2e17..7434765 100644 --- a/src/public/js/components/shows/list.js +++ b/src/public/js/components/shows/list.js @@ -2,7 +2,6 @@ import React from 'react' import ListDetails from '../list/details' import ListPosters from '../list/posters' -import Loader from '../loader/loader' import ShowButtons from './listButtons' export default class ShowList extends React.Component { @@ -21,11 +20,6 @@ export default class ShowList extends React.Component { } const selectedShow = shows[index]; - // Loading - if (this.props.showStore.loading) { - return (); - } - return (
{selectedShow &&