From fb7b020fde08497e089625870e346718997a129d Mon Sep 17 00:00:00 2001 From: Alex Wang Date: Fri, 7 Jun 2019 21:28:42 -0700 Subject: [PATCH 01/13] Update filters to use style hooks and memo --- src/components/Filters/FilterActions.js | 35 ++++---- src/components/Filters/FilterGroup.js | 6 +- src/components/Filters/FilterSelect.js | 24 ++--- src/components/Filters/FilterSort.js | 107 ++++++++++++----------- src/components/Filters/FilterTristate.js | 20 +++-- 5 files changed, 104 insertions(+), 88 deletions(-) diff --git a/src/components/Filters/FilterActions.js b/src/components/Filters/FilterActions.js index 4c5310d9..c006d7a6 100644 --- a/src/components/Filters/FilterActions.js +++ b/src/components/Filters/FilterActions.js @@ -1,10 +1,10 @@ // @flow -import React from "react"; -import { withStyles } from "@material-ui/core/styles"; +import React, { memo } from "react"; +import { makeStyles } from "@material-ui/styles"; import Button from "@material-ui/core/Button"; import Divider from "@material-ui/core/Divider"; -const styles = { +const useStyles = makeStyles({ // TODO: Position the controls div so that it's always at the top of the viewport // I tried with position sticky and absolute, but it didn't work as intended // Try again in the future @@ -21,24 +21,27 @@ const styles = { flexBasis: "40%" } } -}; +}); type Props = { - classes: Object, onResetClick: Function, onSearchClick: Function }; -const FilterActions = ({ classes, onResetClick, onSearchClick }: Props) => ( -
-
- - +const FilterActions = memo(({ onResetClick, onSearchClick }: Props) => { + const classes = useStyles(); + + return ( +
+
+ + +
+
- -
-); + ); +}); -export default withStyles(styles)(FilterActions); +export default FilterActions; diff --git a/src/components/Filters/FilterGroup.js b/src/components/Filters/FilterGroup.js index e9d52841..e99fd45e 100644 --- a/src/components/Filters/FilterGroup.js +++ b/src/components/Filters/FilterGroup.js @@ -1,5 +1,5 @@ // @flow -import React from "react"; +import React, { memo } from "react"; import ExpansionPanel from "@material-ui/core/ExpansionPanel"; import ExpansionPanelDetails from "@material-ui/core/ExpansionPanelDetails"; import ExpansionPanelSummary from "@material-ui/core/ExpansionPanelSummary"; @@ -18,7 +18,7 @@ type Props = { // NOTE: Assuming that GROUP will only contain TRISTATE children // NOTE: using name as the key, this shouldn't be a problem -const FilterGroup = ({ name, state, onChange }: Props) => ( +const FilterGroup = memo(({ name, state, onChange }: Props) => ( expand_more}> {name} @@ -36,6 +36,6 @@ const FilterGroup = ({ name, state, onChange }: Props) => ( -); +)); export default FilterGroup; diff --git a/src/components/Filters/FilterSelect.js b/src/components/Filters/FilterSelect.js index 4a206e09..f3124dd2 100644 --- a/src/components/Filters/FilterSelect.js +++ b/src/components/Filters/FilterSelect.js @@ -1,9 +1,9 @@ // @flow -import React from 'react'; -import FormControl from '@material-ui/core/FormControl'; -import InputLabel from '@material-ui/core/InputLabel'; -import Select from '@material-ui/core/Select'; -import MenuItem from '@material-ui/core/MenuItem'; +import React, { memo } from "react"; +import FormControl from "@material-ui/core/FormControl"; +import InputLabel from "@material-ui/core/InputLabel"; +import Select from "@material-ui/core/Select"; +import MenuItem from "@material-ui/core/MenuItem"; // NOTE: Odd obsevations about choosing the key (No errors though) // @@ -19,15 +19,17 @@ type Props = { values: Array, index: number, state: number, - onChange: Function, + onChange: Function }; -const FilterSelect = ({ - name, values, index, state, onChange, -}: Props) => ( +const FilterSelect = memo(({ name, values, index, state, onChange }: Props) => ( {name} - {values.map((text, valuesIndex) => ( {text} @@ -35,7 +37,7 @@ const FilterSelect = ({ ))} -); +)); // Helper function function generateId(index: number): string { diff --git a/src/components/Filters/FilterSort.js b/src/components/Filters/FilterSort.js index 44ddfa27..d2df1ff0 100644 --- a/src/components/Filters/FilterSort.js +++ b/src/components/Filters/FilterSort.js @@ -1,73 +1,78 @@ // @flow -import React from 'react'; -import ExpansionPanel from '@material-ui/core/ExpansionPanel'; -import ExpansionPanelDetails from '@material-ui/core/ExpansionPanelDetails'; -import ExpansionPanelSummary from '@material-ui/core/ExpansionPanelSummary'; -import Typography from '@material-ui/core/Typography'; -import Icon from '@material-ui/core/Icon'; -import FormGroup from '@material-ui/core/FormGroup'; -import ButtonBase from '@material-ui/core/ButtonBase'; -import { withStyles } from '@material-ui/core/styles'; +import React, { memo } from "react"; +import ExpansionPanel from "@material-ui/core/ExpansionPanel"; +import ExpansionPanelDetails from "@material-ui/core/ExpansionPanelDetails"; +import ExpansionPanelSummary from "@material-ui/core/ExpansionPanelSummary"; +import Typography from "@material-ui/core/Typography"; +import Icon from "@material-ui/core/Icon"; +import FormGroup from "@material-ui/core/FormGroup"; +import ButtonBase from "@material-ui/core/ButtonBase"; +import { makeStyles } from "@material-ui/styles"; -const styles = { +const useStyles = makeStyles({ item: { - width: '100%', - padding: '8px 16px', - display: 'flex', - alignItems: 'center', - justifyContent: 'flex-start', + width: "100%", + padding: "8px 16px", + display: "flex", + alignItems: "center", + justifyContent: "flex-start" }, formGroup: { - width: '100%', // lets the items be full width + width: "100%" // lets the items be full width }, icon: { - marginRight: 8, - }, -}; + marginRight: 8 + } +}); type Props = { - classes: Object, // injected styles values: Array, name: string, state: { index: number, - ascending: boolean, + ascending: boolean }, - onChange: Function, + onChange: Function }; -const FilterSort = ({ - classes, values, name, state, onChange, -}: Props) => ( - - expand_more}> - {name} - - - - {values.map((value, nestedIndex) => ( - - - {iconValue(state.index, state.ascending, nestedIndex)} - - {value} - - ))} - - - -); +const FilterSort = memo(({ values, name, state, onChange }: Props) => { + const classes = useStyles(); + + return ( + + expand_more}> + {name} + + + + {values.map((value, nestedIndex) => ( + + + {iconValue(state.index, state.ascending, nestedIndex)} + + {value} + + ))} + + + + ); +}); // Helper methods -function iconValue(index: number, ascending: boolean, nestedIndex: number): string { +function iconValue( + index: number, + ascending: boolean, + nestedIndex: number +): string { if (nestedIndex === index) { - return ascending ? 'arrow_upward' : 'arrow_downward'; + return ascending ? "arrow_upward" : "arrow_downward"; } - return ''; + return ""; } -export default withStyles(styles)(FilterSort); +export default FilterSort; diff --git a/src/components/Filters/FilterTristate.js b/src/components/Filters/FilterTristate.js index 65e188dd..823b61c1 100644 --- a/src/components/Filters/FilterTristate.js +++ b/src/components/Filters/FilterTristate.js @@ -1,12 +1,12 @@ // @flow -import React from 'react'; -import FormControlLabel from '@material-ui/core/FormControlLabel'; -import Checkbox from '@material-ui/core/Checkbox'; +import React, { memo } from "react"; +import FormControlLabel from "@material-ui/core/FormControlLabel"; +import Checkbox from "@material-ui/core/Checkbox"; type Props = { name: string, state: number, - onChange: Function, + onChange: Function }; // +-------+---------+ @@ -17,16 +17,22 @@ type Props = { // | 2 | EXCLUDE | // +-------+---------+ -const FilterTristate = ({ name, state, onChange }: Props) => { +const FilterTristate = memo(({ name, state, onChange }: Props) => { const checked: boolean = state === 1; const indeterminate: boolean = state === 0; return ( } + control={ + + } label={name} /> ); -}; +}); export default FilterTristate; From d07b9f1c821a1085499dfd7d4bc12d7dc029b024 Mon Sep 17 00:00:00 2001 From: Alex Wang Date: Sat, 8 Jun 2019 10:33:08 -0700 Subject: [PATCH 02/13] Refactoring filters WIP --- .../Filters/DynamicSourceFilters.js | 63 +++++++--- src/components/Filters/FilterGroup.js | 61 +++++----- src/components/Filters/FilterSelect.js | 56 +++++---- src/components/Filters/FilterSort.js | 35 +++--- src/components/Filters/FilterTextField.js | 31 +++++ src/components/Filters/FilterTristate.js | 37 ++---- src/components/Filters/TristateCheckbox.js | 38 ++++++ src/redux-ducks/filters.js | 113 +++++++++++++++++- src/types/filters.js | 46 ++++--- 9 files changed, 349 insertions(+), 131 deletions(-) create mode 100644 src/components/Filters/FilterTextField.js create mode 100644 src/components/Filters/TristateCheckbox.js diff --git a/src/components/Filters/DynamicSourceFilters.js b/src/components/Filters/DynamicSourceFilters.js index 26c8b9ec..0ad419bc 100644 --- a/src/components/Filters/DynamicSourceFilters.js +++ b/src/components/Filters/DynamicSourceFilters.js @@ -3,18 +3,20 @@ import React, { useState } from "react"; import Button from "@material-ui/core/Button"; import Drawer from "@material-ui/core/Drawer"; import FormGroup from "@material-ui/core/FormGroup"; -import type { FilterAnyType } from "types/filters"; import FilterActions from "components/Filters/FilterActions"; -import { filterElements } from "components/Filters/filterUtils"; import { makeStyles } from "@material-ui/styles"; import { fetchCatalogue } from "redux-ducks/catalogue"; import { selectCurrentFilters, resetFilters, - updateLastUsedFilters, - updateCurrentFilters + updateLastUsedFilters } from "redux-ducks/filters"; import { useSelector, useDispatch } from "react-redux"; +import FilterTextField from "components/Filters/FilterTextField"; +import FilterSelect from "components/Filters/FilterSelect"; +import FilterSort from "components/Filters/FilterSort"; +import FilterTristate from "components/Filters/FilterTristate"; +import FilterGroup from "components/Filters/FilterGroup"; // FIXME: Weird blue line when clicking the @@ -56,20 +58,28 @@ const DynamicSourceFilters = () => { dispatch(resetFilters()); }; - const handleUpdateFilters = (newFilters: Array) => { - dispatch(updateCurrentFilters(newFilters)); - }; - const handleSearchWithFilters = () => { dispatch(updateLastUsedFilters()); // Must come before fetchCatalogue. This is a synchronous function. dispatch(fetchCatalogue()); setDrawerOpen(false); }; + if (!filters.length) { + return ( + + ); + } + return (
diff --git a/src/components/Filters/FilterGroup.js b/src/components/Filters/FilterGroup.js index e99fd45e..d9ca6647 100644 --- a/src/components/Filters/FilterGroup.js +++ b/src/components/Filters/FilterGroup.js @@ -6,36 +6,43 @@ import ExpansionPanelSummary from "@material-ui/core/ExpansionPanelSummary"; import Typography from "@material-ui/core/Typography"; import Icon from "@material-ui/core/Icon"; import FormGroup from "@material-ui/core/FormGroup"; -import type { FilterTristate as FilterTristateType } from "types/filters"; -import FilterTristate from "components/Filters/FilterTristate"; - -type Props = { - name: string, - state: Array, - onChange: Function -}; +import { useSelector, useDispatch } from "react-redux"; +import { selectFilterAtIndex, updateFilterGroup } from "redux-ducks/filters"; +import TristateCheckbox from "components/Filters/TristateCheckbox"; // NOTE: Assuming that GROUP will only contain TRISTATE children // NOTE: using name as the key, this shouldn't be a problem -const FilterGroup = memo(({ name, state, onChange }: Props) => ( - - expand_more}> - {name} - - - - {state.map((tristate, nestedIndex) => ( - - ))} - - - -)); +type Props = { index: number }; + +const FilterGroup = memo(({ index }: Props) => { + const dispatch = useDispatch(); + + const filter = useSelector(state => selectFilterAtIndex(state, index)); + + const handleChange = (clickedIndex: number) => () => { + dispatch(updateFilterGroup(index, clickedIndex)); + }; + + return ( + + expand_more}> + {filter.name} + + + + {filter.state.map((tristate, nestedIndex) => ( + + ))} + + + + ); +}); export default FilterGroup; diff --git a/src/components/Filters/FilterSelect.js b/src/components/Filters/FilterSelect.js index f3124dd2..e27bd36b 100644 --- a/src/components/Filters/FilterSelect.js +++ b/src/components/Filters/FilterSelect.js @@ -4,6 +4,8 @@ import FormControl from "@material-ui/core/FormControl"; import InputLabel from "@material-ui/core/InputLabel"; import Select from "@material-ui/core/Select"; import MenuItem from "@material-ui/core/MenuItem"; +import { selectFilterAtIndex, updateFilterSelect } from "redux-ducks/filters"; +import { useSelector, useDispatch } from "react-redux"; // NOTE: Odd obsevations about choosing the key (No errors though) // @@ -14,30 +16,36 @@ import MenuItem from "@material-ui/core/MenuItem"; // I think because MenuItem is so deeply nested in other components (via material-ui) // it makes passing values behave oddly... -type Props = { - name: string, - values: Array, - index: number, - state: number, - onChange: Function -}; - -const FilterSelect = memo(({ name, values, index, state, onChange }: Props) => ( - - {name} - - -)); +type Props = { index: number }; + +const FilterSelect = memo(({ index }: Props) => { + const dispatch = useDispatch(); + + const filter = useSelector(state => selectFilterAtIndex(state, index)); + + const handleChange = (event: SyntheticEvent) => { + // NOTE: LIElement is actually within a select + const newValue = parseInt(event.currentTarget.dataset.value, 10); + dispatch(updateFilterSelect(index, newValue)); + }; + + return ( + + {filter.name} + + + ); +}); // Helper function function generateId(index: number): string { diff --git a/src/components/Filters/FilterSort.js b/src/components/Filters/FilterSort.js index d2df1ff0..43f90425 100644 --- a/src/components/Filters/FilterSort.js +++ b/src/components/Filters/FilterSort.js @@ -8,6 +8,8 @@ import Icon from "@material-ui/core/Icon"; import FormGroup from "@material-ui/core/FormGroup"; import ButtonBase from "@material-ui/core/ButtonBase"; import { makeStyles } from "@material-ui/styles"; +import { selectFilterAtIndex, updateFilterSort } from "redux-ducks/filters"; +import { useSelector, useDispatch } from "react-redux"; const useStyles = makeStyles({ item: { @@ -25,34 +27,37 @@ const useStyles = makeStyles({ } }); -type Props = { - values: Array, - name: string, - state: { - index: number, - ascending: boolean - }, - onChange: Function -}; +type Props = { index: number }; -const FilterSort = memo(({ values, name, state, onChange }: Props) => { +const FilterSort = memo(({ index }: Props) => { const classes = useStyles(); + const dispatch = useDispatch(); + + const filter = useSelector(state => selectFilterAtIndex(state, index)); + + const handleChange = (clickedIndex: number) => () => { + dispatch(updateFilterSort(index, clickedIndex)); + }; return ( expand_more}> - {name} + {filter.name} - {values.map((value, nestedIndex) => ( + {filter.values.map((value, nestedIndex) => ( - {iconValue(state.index, state.ascending, nestedIndex)} + {iconValue( + filter.state.index, + filter.state.ascending, + nestedIndex + )} {value} diff --git a/src/components/Filters/FilterTextField.js b/src/components/Filters/FilterTextField.js new file mode 100644 index 00000000..5110bd3f --- /dev/null +++ b/src/components/Filters/FilterTextField.js @@ -0,0 +1,31 @@ +// @flow +import React, { memo } from "react"; +import { + selectFilterAtIndex, + updateFilterTextField +} from "redux-ducks/filters"; +import { useSelector, useDispatch } from "react-redux"; +import TextField from "@material-ui/core/TextField"; + +type Props = { index: number }; + +const FilterTextField = memo(({ index }: Props) => { + const dispatch = useDispatch(); + + const filter = useSelector(state => selectFilterAtIndex(state, index)); + + const handleChange = (event: SyntheticEvent) => { + dispatch(updateFilterTextField(index, event.currentTarget.value)); + }; + + return ( + + ); +}); + +export default FilterTextField; diff --git a/src/components/Filters/FilterTristate.js b/src/components/Filters/FilterTristate.js index 823b61c1..4d9c7104 100644 --- a/src/components/Filters/FilterTristate.js +++ b/src/components/Filters/FilterTristate.js @@ -1,36 +1,21 @@ // @flow import React, { memo } from "react"; -import FormControlLabel from "@material-ui/core/FormControlLabel"; -import Checkbox from "@material-ui/core/Checkbox"; +import TristateCheckbox from "components/Filters/TristateCheckbox"; +import { useSelector, useDispatch } from "react-redux"; +import { selectFilterAtIndex, updateFilterTristate } from "redux-ducks/filters"; -type Props = { - name: string, - state: number, - onChange: Function -}; +type Props = { index: number }; -// +-------+---------+ -// | Index | State | -// +-------+---------+ -// | 0 | IGNORE | -// | 1 | INCLUDE | -// | 2 | EXCLUDE | -// +-------+---------+ +const FilterTristate = memo(({ index }: Props) => { + const dispatch = useDispatch(); -const FilterTristate = memo(({ name, state, onChange }: Props) => { - const checked: boolean = state === 1; - const indeterminate: boolean = state === 0; + const filter = useSelector(state => selectFilterAtIndex(state, index)); return ( - - } - label={name} + ); }); diff --git a/src/components/Filters/TristateCheckbox.js b/src/components/Filters/TristateCheckbox.js new file mode 100644 index 00000000..cd2e6de4 --- /dev/null +++ b/src/components/Filters/TristateCheckbox.js @@ -0,0 +1,38 @@ +// @flow +import React, { memo } from "react"; +import FormControlLabel from "@material-ui/core/FormControlLabel"; +import Checkbox from "@material-ui/core/Checkbox"; + +type Props = { + name: string, + state: number, + onChange: Function +}; + +// +-------+---------+ +// | Index | State | +// +-------+---------+ +// | 0 | IGNORE | +// | 1 | INCLUDE | +// | 2 | EXCLUDE | +// +-------+---------+ + +const TristateCheckbox = memo(({ name, state, onChange }: Props) => { + const checked: boolean = state === 1; + const indeterminate: boolean = state === 0; + + return ( + + } + label={name} + /> + ); +}); + +export default TristateCheckbox; diff --git a/src/redux-ducks/filters.js b/src/redux-ducks/filters.js index 277ede36..d34f5a66 100644 --- a/src/redux-ducks/filters.js +++ b/src/redux-ducks/filters.js @@ -1,6 +1,6 @@ // @flow import { Server } from "api"; -import type { FilterAnyType } from "types/filters"; +import type { FilterAnyType, FilterSelect } from "types/filters"; import { handleHTMLError } from "redux-ducks/utils"; import { RESET_STATE as RESET_CATALOGUE_STATE } from "redux-ducks/catalogue"; @@ -15,6 +15,8 @@ const RESET_FILTERS = "filters/RESET_FILTERS"; const UPDATE_LAST_USED_FILTERS = "filters/UPDATE_LAST_USED_FILTERS"; const UPDATE_CURRENT_FILTERS = "filters/UPDATE_CURRENT_FILTERS"; +const UPDATE_FILTER = "filters/UPDATE_FILTER"; + // ================================================================================ // Reducers // ================================================================================ @@ -61,6 +63,19 @@ export default function filtersReducer( case UPDATE_CURRENT_FILTERS: return { ...state, currentFilters: action.filters }; + case UPDATE_FILTER: { + const { filter, index } = action; + + const oldFilters = state.currentFilters; + const newFilters = [ + ...oldFilters.slice(0, index), + filter, + ...oldFilters.slice(index + 1) + ]; + + return { ...state, currentFilters: newFilters }; + } + default: return state; } @@ -77,6 +92,9 @@ export const selectLastUsedFilters = (state): Array => export const selectCurrentFilters = (state): Array => state.filters.currentFilters; +export const selectFilterAtIndex = (state, index): FilterAnyType => + state.filters.currentFilters[index]; + // ================================================================================ // Action Creators // ================================================================================ @@ -119,3 +137,96 @@ export function updateCurrentFilters(filters: Array) { return (dispatch: Function) => dispatch({ type: UPDATE_CURRENT_FILTERS, filters }); } + +// ================================================================================ +// Action Creators - Individual filter update actions +// ================================================================================ +export function updateFilterTextField(index: number, newState: string) { + return (dispatch: Function, getState: Function) => { + const currentFilter = selectFilterAtIndex(getState(), index); + + const updatedFilter: FilterSelect = { + ...currentFilter, + state: newState + }; + + dispatch({ type: UPDATE_FILTER, filter: updatedFilter, index }); + }; +} + +export function updateFilterSelect(index: number, newState: number) { + return (dispatch: Function, getState: Function) => { + const currentFilter = selectFilterAtIndex(getState(), index); + + const updatedFilter: FilterSelect = { + ...currentFilter, + state: newState + }; + + dispatch({ type: UPDATE_FILTER, filter: updatedFilter, index }); + }; +} + +export function updateFilterSort(index: number, selectedIndex: number) { + return (dispatch: Function, getState: Function) => { + const currentFilter = selectFilterAtIndex(getState(), index); + + const isAscending = currentFilter.state.ascending; + const currentIndex = currentFilter.state.index; + + const newAscendingState = + currentIndex === selectedIndex ? !isAscending : false; + + const updatedFilter = { + ...currentFilter, + state: { index: selectedIndex, ascending: newAscendingState } + }; + + dispatch({ type: UPDATE_FILTER, filter: updatedFilter, index }); + }; +} + +export function updateFilterTristate(index: number) { + return (dispatch: Function, getState: Function) => { + const currentFilter = selectFilterAtIndex(getState(), index); + + const updatedFilter = { + ...currentFilter, + state: newTristateState(currentFilter.state) + }; + + dispatch({ type: UPDATE_FILTER, filter: updatedFilter, index }); + }; +} + +export function updateFilterGroup(index: number, nestedIndex: number) { + return (dispatch: Function, getState: Function) => { + const currentFilter = selectFilterAtIndex(getState(), index); + + // Update the state of the nested item + const nestedTristate = currentFilter.state[nestedIndex]; + const updatedTristate: FilterTristateType = { + ...nestedTristate, + state: newTristateState(nestedTristate.state) + }; + + // Update the array of state with the updated item + const updatedState = [ + ...currentFilter.state.slice(0, nestedIndex), + updatedTristate, + ...currentFilter.state.slice(nestedIndex + 1) + ]; + + const updatedFilter = { ...currentFilter, state: updatedState }; + + dispatch({ type: UPDATE_FILTER, filter: updatedFilter, index }); + }; +} + +// helper function +function newTristateState(prevState: number): number { + if (prevState < 2) { + return prevState + 1; + } + return 0; +} diff --git a/src/types/filters.js b/src/types/filters.js index bd7bfad9..7cb83de2 100644 --- a/src/types/filters.js +++ b/src/types/filters.js @@ -1,83 +1,81 @@ // @flow export type FilterText = { _cmaps: { - name: 'java.lang.String', - state: 'java.lang.String', + name: "java.lang.String", + state: "java.lang.String" }, name: string, state: string, - _type: 'TEXT', + _type: "TEXT" }; export type FilterSelect = { values: Array, _cmaps: { - name: 'java.lang.String', - state: 'java.lang.Integer', + name: "java.lang.String", + state: "java.lang.Integer" }, name: string, state: number, - _type: 'SELECT', + _type: "SELECT" }; export type FilterTristate = { _cmaps: { - name: 'java.lang.String', - state: 'java.lang.Integer', + name: "java.lang.String", + state: "java.lang.Integer" }, name: string, state: number, - _type: 'TRISTATE', + _type: "TRISTATE" }; export type FilterGroup = { state: Array, _cmaps: { - name: 'java.lang.String', + name: "java.lang.String" }, name: string, - _type: 'GROUP', + _type: "GROUP" }; +export type SortState = { index: number, ascending: boolean }; export type FilterSort = { values: Array, - state: { - index: number, - ascending: boolean, - }, + state: SortState, _cmaps: { - name: 'java.lang.String', + name: "java.lang.String" }, name: string, - _type: 'SORT', + _type: "SORT" }; // UNUSED FILTER TYPES BELOW? export type FilterHeader = { _cmaps: { - name: 'java.lang.String', + name: "java.lang.String" }, name: string, - _type: 'HEADER', + _type: "HEADER" }; export type FilterSeparator = { _cmaps: { - name: 'java.lang.String', + name: "java.lang.String" }, name: string, - _type: 'SEPARATOR', + _type: "SEPARATOR" }; export type FilterCheckbox = { _cmaps: { - name: 'java.lang.String', - state: 'java.lang.Boolean', + name: "java.lang.String", + state: "java.lang.Boolean" }, name: string, state: boolean, - _type: 'CHECKBOX', + _type: "CHECKBOX" }; // Consolidated type From de6d8656d9b04f3e56bf113f57c7fc93526d9fd2 Mon Sep 17 00:00:00 2001 From: Alex Wang Date: Sat, 8 Jun 2019 10:52:01 -0700 Subject: [PATCH 03/13] More filters updates --- .../Filters/DynamicSourceFilters.js | 87 +++---- src/components/Filters/FilterActions.js | 27 ++- src/components/Filters/filterUtils.js | 223 ------------------ 3 files changed, 56 insertions(+), 281 deletions(-) delete mode 100644 src/components/Filters/filterUtils.js diff --git a/src/components/Filters/DynamicSourceFilters.js b/src/components/Filters/DynamicSourceFilters.js index 0ad419bc..b63ef807 100644 --- a/src/components/Filters/DynamicSourceFilters.js +++ b/src/components/Filters/DynamicSourceFilters.js @@ -5,13 +5,8 @@ import Drawer from "@material-ui/core/Drawer"; import FormGroup from "@material-ui/core/FormGroup"; import FilterActions from "components/Filters/FilterActions"; import { makeStyles } from "@material-ui/styles"; -import { fetchCatalogue } from "redux-ducks/catalogue"; -import { - selectCurrentFilters, - resetFilters, - updateLastUsedFilters -} from "redux-ducks/filters"; -import { useSelector, useDispatch } from "react-redux"; +import { selectCurrentFilters } from "redux-ducks/filters"; +import { useSelector } from "react-redux"; import FilterTextField from "components/Filters/FilterTextField"; import FilterSelect from "components/Filters/FilterSelect"; import FilterSort from "components/Filters/FilterSort"; @@ -20,14 +15,6 @@ import FilterGroup from "components/Filters/FilterGroup"; // FIXME: Weird blue line when clicking the -// FIXME: I think using cloneDeep here is getting really laggy. -// Even after switching to non-lodash, still laggy. -// May have to do actual object updates instead. - -// Choosing to use a deep copy instead of the standard setState method -// It would be a huge pain to try updating an array of objects (and be less readable) -// https://stackoverflow.com/questions/29537299/react-how-do-i-update-state-item1-on-setstate-with-jsfiddle - const useStyles = makeStyles({ openButton: { marginBottom: 24, @@ -48,19 +35,12 @@ const useStyles = makeStyles({ const DynamicSourceFilters = () => { const classes = useStyles(); - const dispatch = useDispatch(); const filters = useSelector(selectCurrentFilters); const [drawerOpen, setDrawerOpen] = useState(false); - const handleResetFilters = () => { - dispatch(resetFilters()); - }; - - const handleSearchWithFilters = () => { - dispatch(updateLastUsedFilters()); // Must come before fetchCatalogue. This is a synchronous function. - dispatch(fetchCatalogue()); + const handleSearchClick = () => { setDrawerOpen(false); }; @@ -95,44 +75,45 @@ const DynamicSourceFilters = () => { > {/* without this div, FilterGroup components screw up, not sure why though */}
- + - {filters.map((filter, index) => { - if (["HEADER", "SEPARATOR", "CHECKBOX"].includes(filter._type)) { - console.error( - `Catalogue filters - ${filter._type} is not implemented.` - ); - } + {filters.map((filter, index) => ( + + ))} + +
+ + + ); +}; - switch (filter._type) { - case "TEXT": - return ; +type Props = { type: string, index: number }; - case "SELECT": - return ; +const DynamicFilter = ({ type, index }: Props) => { + if (["HEADER", "SEPARATOR", "CHECKBOX"].includes(type)) { + console.error(`Catalogue filters - ${type} is not implemented.`); + } - case "SORT": - return ; + switch (type) { + case "TEXT": + return ; - case "TRISTATE": - return ; + case "SELECT": + return ; - case "GROUP": - return ; + case "SORT": + return ; - default: - return null; - } - })} - - - - - ); + case "TRISTATE": + return ; + + case "GROUP": + return ; + + default: + return null; + } }; export default DynamicSourceFilters; diff --git a/src/components/Filters/FilterActions.js b/src/components/Filters/FilterActions.js index c006d7a6..cf7d3d67 100644 --- a/src/components/Filters/FilterActions.js +++ b/src/components/Filters/FilterActions.js @@ -3,6 +3,9 @@ import React, { memo } from "react"; import { makeStyles } from "@material-ui/styles"; import Button from "@material-ui/core/Button"; import Divider from "@material-ui/core/Divider"; +import { useDispatch } from "react-redux"; +import { resetFilters, updateLastUsedFilters } from "redux-ducks/filters"; +import { fetchCatalogue } from "redux-ducks/catalogue"; const useStyles = makeStyles({ // TODO: Position the controls div so that it's always at the top of the viewport @@ -24,18 +27,32 @@ const useStyles = makeStyles({ }); type Props = { - onResetClick: Function, - onSearchClick: Function + onSearchClick: Function // for any additional actions that fire }; -const FilterActions = memo(({ onResetClick, onSearchClick }: Props) => { +const FilterActions = memo(({ onSearchClick }: Props) => { const classes = useStyles(); + const dispatch = useDispatch(); + + const handleResetClick = () => { + dispatch(resetFilters()); + }; + + const handleSearchWithFiltersClick = () => { + dispatch(updateLastUsedFilters()); // Must come before fetchCatalogue. This is a synchronous function. + dispatch(fetchCatalogue()); + onSearchClick(); + }; return (
- - +
diff --git a/src/components/Filters/filterUtils.js b/src/components/Filters/filterUtils.js deleted file mode 100644 index 8593442f..00000000 --- a/src/components/Filters/filterUtils.js +++ /dev/null @@ -1,223 +0,0 @@ -// @flow -import * as React from "react"; -import TextField from "@material-ui/core/TextField"; -import type { - FilterAnyType, - FilterText as FilterTextType, - FilterSelect as FilterSelectType, - FilterTristate as FilterTristateType, - FilterSort as FilterSortType, - FilterGroup as FilterGroupType -} from "types/filters"; -import FilterSelect from "components/Filters/FilterSelect"; -import FilterTristate from "components/Filters/FilterTristate"; -import FilterGroup from "components/Filters/FilterGroup"; -import FilterSort from "components/Filters/FilterSort"; - -// FIXME: Still feels a little laggy in dev mode. -// May partially be caused in dev by React DevTools -// https://stackoverflow.com/questions/32911519/react-slow-with-multiple-controlled-text-inputs -// -// Try a production build to see how bad it is. -// -// If it's still bad, try using immer - https://github.com/mweststrate/immer -// Other suggestions here - https://medium.freecodecamp.org/handling-state-in-react-four-immutable-approaches-to-consider-d1f5c00249d5 - -/* eslint-disable import/prefer-default-export, no-underscore-dangle */ -export function filterElements( - filters: Array, - onChange: Function -): Array { - return filters.map((filter: FilterAnyType, index: number) => { - // TODO: header, separator, checkbox - // not doing right now because none of the sources use it - - // NOTE: using filter.name as the key. I doubt it'll be a problem. - - if (filter._type === 'HEADER') { - console.error('DynamicSourcesFilters HEADER not implemented'); - return null; - } if (filter._type === 'SEPARATOR') { - console.error('DynamicSourcesFilters SEPARATOR not implemented'); - return null; - } if (filter._type === 'CHECKBOX') { - console.error('DynamicSourcesFilters CHECKBOX not implemented'); - return null; - } else if (filter._type === 'TEXT') { - return ( - - ); - } else if (filter._type === 'SELECT') { - return ( - - ); - } else if (filter._type === 'TRISTATE') { - return ( - - ); - } else if (filter._type === 'GROUP') { - // NOTE: Assuming that GROUP will only contain TRISTATE children - return ( - - ); - } else if (filter._type === 'SORT') { - return ( - - ); - } - - return null; - }); -} - -function handleTextChange( - index: number, - filter: FilterTextType, - filters: Array, - onChange: Function -) { - return (event: SyntheticEvent) => { - const updatedFilter: FilterTextType = { - ...filter, - state: event.currentTarget.value - }; - onChange(updateArray(index, updatedFilter, filters)); - }; -} - -function handleSelectChange( - index: number, - filter: FilterSelectType, - filters: Array, - onChange: Function -) { - // NOTE: LIElement is actually within a select - return (event: SyntheticEvent) => { - const newSelection = parseInt(event.currentTarget.dataset.value, 10); - const updatedFilter: FilterSelectType = { ...filter, state: newSelection }; - onChange(updateArray(index, updatedFilter, filters)); - }; -} - -function handleTristateChange( - index: number, - filter: FilterTristateType, - filters: Array, - onChange: Function -) { - return () => { - const updatedFilter: FilterTristateType = { - ...filter, - state: updateTristate(filter.state) - }; - onChange(updateArray(index, updatedFilter, filters)); - }; -} - -function handleGroupChange( - index: number, - filter: FilterGroupType, - filters: Array, - onChange: Function -) { - // NOTE: Assuming that GROUP will only contain TRISTATE children - return (clickedIndex: number) => () => { - // Nested filters, so it's a bit more complex to update - // First update the nested tristate's state - const tristate = filter.state[clickedIndex]; - const updatedTristate: FilterTristateType = { - ...tristate, - state: updateTristate(tristate.state) - }; - - // Then insert the updated tristate into the original array of tristates - const updatedTristateArray = updateArray( - clickedIndex, - updatedTristate, - filter.state - ); - - // Then update the group's state with the new array and update the whole state - const updatedFilter: FilterGroupType = { - ...filter, - state: updatedTristateArray - }; - onChange(updateArray(index, updatedFilter, filters)); - }; -} - -type SortState = { index: number, ascending: boolean }; -function handleSortChange( - index: number, - filter: FilterSortType, - filters: Array, - onChange: Function -) { - return (clickedIndex: number) => () => { - const isAscending = filter.state.ascending; - const currentIndex = filter.state.index; - - const newState: SortState = updateSort( - currentIndex, - clickedIndex, - isAscending - ); - const updatedFilter: FilterSortType = { ...filter, state: newState }; - onChange(updateArray(index, updatedFilter, filters)); - }; -} - -// Helper Functions -function updateTristate(oldState: number): number { - if (oldState < 2) { - return oldState + 1; - } - return 0; -} - -function updateSort( - index: number, - clickedIndex: number, - isAscending: boolean -): SortState { - return { - index: clickedIndex, - ascending: index === clickedIndex ? !isAscending : false - }; -} - -function updateArray( - index: number, - newElement: T, - array: Array -): Array { - return [...array.slice(0, index), newElement, ...array.slice(index + 1)]; -} From 9a0fc3d5646b7b8556859dc0d481673bce5e20c2 Mon Sep 17 00:00:00 2001 From: Alex Wang Date: Sat, 8 Jun 2019 11:11:58 -0700 Subject: [PATCH 04/13] Mostly finish optimizations --- .../Filters/DynamicSourceFilters.js | 20 ++++++++++++------- src/redux-ducks/filters.js | 18 +++++++++++++++++ 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/components/Filters/DynamicSourceFilters.js b/src/components/Filters/DynamicSourceFilters.js index b63ef807..534e65c3 100644 --- a/src/components/Filters/DynamicSourceFilters.js +++ b/src/components/Filters/DynamicSourceFilters.js @@ -5,13 +5,17 @@ import Drawer from "@material-ui/core/Drawer"; import FormGroup from "@material-ui/core/FormGroup"; import FilterActions from "components/Filters/FilterActions"; import { makeStyles } from "@material-ui/styles"; -import { selectCurrentFilters } from "redux-ducks/filters"; +import { + selectFiltersLength, + selectFilterTypeAtIndex +} from "redux-ducks/filters"; import { useSelector } from "react-redux"; import FilterTextField from "components/Filters/FilterTextField"; import FilterSelect from "components/Filters/FilterSelect"; import FilterSort from "components/Filters/FilterSort"; import FilterTristate from "components/Filters/FilterTristate"; import FilterGroup from "components/Filters/FilterGroup"; +import times from "lodash/times"; // FIXME: Weird blue line when clicking the @@ -36,7 +40,7 @@ const useStyles = makeStyles({ const DynamicSourceFilters = () => { const classes = useStyles(); - const filters = useSelector(selectCurrentFilters); + const filtersLength = useSelector(selectFiltersLength); const [drawerOpen, setDrawerOpen] = useState(false); @@ -44,7 +48,7 @@ const DynamicSourceFilters = () => { setDrawerOpen(false); }; - if (!filters.length) { + if (!filtersLength) { return (
@@ -88,9 +92,11 @@ const DynamicSourceFilters = () => { ); }; -type Props = { type: string, index: number }; +type Props = { index: number }; + +const DynamicFilter = ({ index }: Props) => { + const type = useSelector(state => selectFilterTypeAtIndex(state, index)); -const DynamicFilter = ({ type, index }: Props) => { if (["HEADER", "SEPARATOR", "CHECKBOX"].includes(type)) { console.error(`Catalogue filters - ${type} is not implemented.`); } diff --git a/src/redux-ducks/filters.js b/src/redux-ducks/filters.js index d34f5a66..10d56ec0 100644 --- a/src/redux-ducks/filters.js +++ b/src/redux-ducks/filters.js @@ -3,6 +3,8 @@ import { Server } from "api"; import type { FilterAnyType, FilterSelect } from "types/filters"; import { handleHTMLError } from "redux-ducks/utils"; import { RESET_STATE as RESET_CATALOGUE_STATE } from "redux-ducks/catalogue"; +import { createSelector } from "reselect"; +import createCachedSelector from "re-reselect"; // ================================================================================ // Actions @@ -95,6 +97,22 @@ export const selectCurrentFilters = (state): Array => export const selectFilterAtIndex = (state, index): FilterAnyType => state.filters.currentFilters[index]; +export const selectFiltersLength = createSelector( + // Optimization + // The length of filters is constant, so we can look outside of currentFilters + [selectInitialFilters], + (filters): Array => filters.length +); + +// selectFilterTypeAtIndex(state, index) +export const selectFilterTypeAtIndex = createCachedSelector( + // Optimization + // The type and position of a filter is constant, so we can look outside of currentFilters + [selectInitialFilters, (_, index) => index], + (filters, index) => filters[index]._type + // Cache Key +)((_, index) => index); + // ================================================================================ // Action Creators // ================================================================================ From 5cad69000abaf97ed3883edbbb67cdb9a015159a Mon Sep 17 00:00:00 2001 From: Alex Wang Date: Sat, 8 Jun 2019 11:13:49 -0700 Subject: [PATCH 05/13] Extract a DynamicFilter component --- src/components/Filters/DynamicFilter.js | 41 +++++++++++++++++++ .../Filters/DynamicSourceFilters.js | 41 +------------------ 2 files changed, 43 insertions(+), 39 deletions(-) create mode 100644 src/components/Filters/DynamicFilter.js diff --git a/src/components/Filters/DynamicFilter.js b/src/components/Filters/DynamicFilter.js new file mode 100644 index 00000000..f33c28e6 --- /dev/null +++ b/src/components/Filters/DynamicFilter.js @@ -0,0 +1,41 @@ +// @flow +import React from "react"; +import { selectFilterTypeAtIndex } from "redux-ducks/filters"; +import { useSelector } from "react-redux"; +import FilterTextField from "components/Filters/FilterTextField"; +import FilterSelect from "components/Filters/FilterSelect"; +import FilterSort from "components/Filters/FilterSort"; +import FilterTristate from "components/Filters/FilterTristate"; +import FilterGroup from "components/Filters/FilterGroup"; + +type Props = { index: number }; + +const DynamicFilter = ({ index }: Props) => { + const type = useSelector(state => selectFilterTypeAtIndex(state, index)); + + if (["HEADER", "SEPARATOR", "CHECKBOX"].includes(type)) { + console.error(`Catalogue filters - ${type} is not implemented.`); + } + + switch (type) { + case "TEXT": + return ; + + case "SELECT": + return ; + + case "SORT": + return ; + + case "TRISTATE": + return ; + + case "GROUP": + return ; + + default: + return null; + } +}; + +export default DynamicFilter; diff --git a/src/components/Filters/DynamicSourceFilters.js b/src/components/Filters/DynamicSourceFilters.js index 534e65c3..a0d23b5f 100644 --- a/src/components/Filters/DynamicSourceFilters.js +++ b/src/components/Filters/DynamicSourceFilters.js @@ -5,16 +5,9 @@ import Drawer from "@material-ui/core/Drawer"; import FormGroup from "@material-ui/core/FormGroup"; import FilterActions from "components/Filters/FilterActions"; import { makeStyles } from "@material-ui/styles"; -import { - selectFiltersLength, - selectFilterTypeAtIndex -} from "redux-ducks/filters"; +import { selectFiltersLength } from "redux-ducks/filters"; import { useSelector } from "react-redux"; -import FilterTextField from "components/Filters/FilterTextField"; -import FilterSelect from "components/Filters/FilterSelect"; -import FilterSort from "components/Filters/FilterSort"; -import FilterTristate from "components/Filters/FilterTristate"; -import FilterGroup from "components/Filters/FilterGroup"; +import DynamicFilter from "components/Filters/DynamicFilter"; import times from "lodash/times"; // FIXME: Weird blue line when clicking the @@ -92,34 +85,4 @@ const DynamicSourceFilters = () => { ); }; -type Props = { index: number }; - -const DynamicFilter = ({ index }: Props) => { - const type = useSelector(state => selectFilterTypeAtIndex(state, index)); - - if (["HEADER", "SEPARATOR", "CHECKBOX"].includes(type)) { - console.error(`Catalogue filters - ${type} is not implemented.`); - } - - switch (type) { - case "TEXT": - return ; - - case "SELECT": - return ; - - case "SORT": - return ; - - case "TRISTATE": - return ; - - case "GROUP": - return ; - - default: - return null; - } -}; - export default DynamicSourceFilters; From 684038709021ac52805369543ad59e50e9302da0 Mon Sep 17 00:00:00 2001 From: Alex Wang Date: Sat, 8 Jun 2019 11:20:04 -0700 Subject: [PATCH 06/13] Add some comments --- src/components/Filters/FilterGroup.js | 4 ++++ src/components/Filters/FilterSort.js | 3 +++ 2 files changed, 7 insertions(+) diff --git a/src/components/Filters/FilterGroup.js b/src/components/Filters/FilterGroup.js index d9ca6647..31a14217 100644 --- a/src/components/Filters/FilterGroup.js +++ b/src/components/Filters/FilterGroup.js @@ -10,6 +10,10 @@ import { useSelector, useDispatch } from "react-redux"; import { selectFilterAtIndex, updateFilterGroup } from "redux-ducks/filters"; import TristateCheckbox from "components/Filters/TristateCheckbox"; +// NOTE: This component is unoptimized. A single change will cause the entire list to rerender. +// The list of tristates this generates does tend to get quite long, but I think it's still +// reasonably performant. Not going to optimize this for now. + // NOTE: Assuming that GROUP will only contain TRISTATE children // NOTE: using name as the key, this shouldn't be a problem diff --git a/src/components/Filters/FilterSort.js b/src/components/Filters/FilterSort.js index 43f90425..296b0327 100644 --- a/src/components/Filters/FilterSort.js +++ b/src/components/Filters/FilterSort.js @@ -11,6 +11,9 @@ import { makeStyles } from "@material-ui/styles"; import { selectFilterAtIndex, updateFilterSort } from "redux-ducks/filters"; import { useSelector, useDispatch } from "react-redux"; +// NOTE: This component is unoptimized. A single change will cause the entire list to rerender. +// However, sorts tend to be short lists, so I'm not going to optimize this for now. + const useStyles = makeStyles({ item: { width: "100%", From b398c8adbfdd9ad29075b2a5659ff6fe9a829159 Mon Sep 17 00:00:00 2001 From: Alex Wang Date: Sat, 8 Jun 2019 11:23:44 -0700 Subject: [PATCH 07/13] Suppress a warning --- src/components/Filters/DynamicSourceFilters.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/components/Filters/DynamicSourceFilters.js b/src/components/Filters/DynamicSourceFilters.js index a0d23b5f..d392e2e1 100644 --- a/src/components/Filters/DynamicSourceFilters.js +++ b/src/components/Filters/DynamicSourceFilters.js @@ -76,6 +76,8 @@ const DynamicSourceFilters = () => { {times(filtersLength).map((_, index) => ( + // The order of filters is constant, so using index as the key is fine. + // eslint-disable-next-line react/no-array-index-key ))} From 6d83fa1f610a44a165604fdfd17849bb70fe0b13 Mon Sep 17 00:00:00 2001 From: Alex Wang Date: Sat, 8 Jun 2019 22:31:11 -0700 Subject: [PATCH 08/13] install react-window --- package-lock.json | 19 +++++++++++++++++++ package.json | 2 ++ 2 files changed, 21 insertions(+) diff --git a/package-lock.json b/package-lock.json index b3c3c6fc..23f0854d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8470,6 +8470,11 @@ } } }, + "memoize-one": { + "version": "5.0.4", + "resolved": "https://registry.npmjs.org/memoize-one/-/memoize-one-5.0.4.tgz", + "integrity": "sha512-P0z5IeAH6qHHGkJIXWw0xC2HNEgkx/9uWWBQw64FJj3/ol14VYdfVGWWr0fXfjhhv3TKVIqUq65os6O4GUNksA==" + }, "memory-fs": { "version": "0.4.1", "resolved": "https://registry.npmjs.org/memory-fs/-/memory-fs-0.4.1.tgz", @@ -11021,6 +11026,11 @@ "prop-types": "^15.6.2" } }, + "react-virtualized-auto-sizer": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/react-virtualized-auto-sizer/-/react-virtualized-auto-sizer-1.0.2.tgz", + "integrity": "sha512-MYXhTY1BZpdJFjUovvYHVBmkq79szK/k7V3MO+36gJkWGkrXKtyr4vCPtpphaTLRAdDNoYEYFZWE8LjN+PIHNg==" + }, "react-waypoint": { "version": "8.1.0", "resolved": "https://registry.npmjs.org/react-waypoint/-/react-waypoint-8.1.0.tgz", @@ -11031,6 +11041,15 @@ "react-is": "^16.6.3" } }, + "react-window": { + "version": "1.8.2", + "resolved": "https://registry.npmjs.org/react-window/-/react-window-1.8.2.tgz", + "integrity": "sha512-Qo9Z5qYvigRbSlCaWTor53G3EWVtHxXXny86EYsMG57H+4LJEEyh22PuWW6ECCkOh7sY6wWF78+wNGwPnMBCAg==", + "requires": { + "@babel/runtime": "^7.0.0", + "memoize-one": ">=3.1.1 <6" + } + }, "read-pkg": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/read-pkg/-/read-pkg-3.0.0.tgz", diff --git a/package.json b/package.json index e6bbcd14..12b02823 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,9 @@ "react-redux": "^7.1.0-rc.1", "react-router-dom": "^4.2.2", "react-scripts": "3.0.1", + "react-virtualized-auto-sizer": "^1.0.2", "react-waypoint": "^8.0.1", + "react-window": "^1.8.2", "redux": "^4.0.0", "redux-logger": "^3.0.6", "redux-thunk": "^2.2.0", From d8e12fe0785be5ac86535ce4f12cef6e0a8bafa9 Mon Sep 17 00:00:00 2001 From: Alex Wang Date: Sun, 9 Jun 2019 10:21:52 -0700 Subject: [PATCH 09/13] WIP virtualized chapter list --- src/components/MangaInfo/ChapterListItem.js | 6 +- .../MangaInfo/MangaInfoChapterList.js | 102 +++++++++++------- src/components/MangaInfo/index.js | 17 ++- src/components/ResponsiveGrid.js | 7 +- 4 files changed, 85 insertions(+), 47 deletions(-) diff --git a/src/components/MangaInfo/ChapterListItem.js b/src/components/MangaInfo/ChapterListItem.js index 5a9a25d3..c413b3cc 100644 --- a/src/components/MangaInfo/ChapterListItem.js +++ b/src/components/MangaInfo/ChapterListItem.js @@ -37,13 +37,14 @@ type Props = { mangaInfo: MangaType, chapter: ChapterType, toggleRead: Function -}; +}; // other props will be passed to the root ListItem const ChapterListItem = ({ classes, mangaInfo, chapter, - toggleRead + toggleRead, + ...otherProps }: Props) => { const urlPrefix = useContext(UrlPrefixContext); @@ -57,6 +58,7 @@ const ChapterListItem = ({ return ( ({ +type Props = { + mangaInfo: MangaType, + chapters: Array, + toggleRead: Function +}; + +const useStyles = makeStyles({ list: { paddingTop: 0, - paddingBottom: 0, + paddingBottom: 0 }, + virtualizedListParent: { flexGrow: 1 } }); -type Props = { - classes: Object, - mangaInfo: MangaType, - chapters: Array, - toggleRead: Function, -}; +const MangaInfoChapterList = ({ mangaInfo, chapters, toggleRead }: Props) => { + const classes = useStyles(); -const MangaInfoChapterList = ({ - classes, mangaInfo, chapters, toggleRead, -}: Props) => ( - - - - - {chapters.map(chapter => ( - - ))} - - - - -); + return ( + + + {/* itemSize is hard coded using what I saw in the inspector */} + + {({ height, width }) => ( + data[index].id} + > + {({ index, style, data }) => ( + + )} + + )} + + + + ); +}; -export default withStyles(styles)(MangaInfoChapterList); +export default MangaInfoChapterList; diff --git a/src/components/MangaInfo/index.js b/src/components/MangaInfo/index.js index c20e0416..55207e00 100644 --- a/src/components/MangaInfo/index.js +++ b/src/components/MangaInfo/index.js @@ -23,6 +23,7 @@ import { updateChapters, toggleRead } from "redux-ducks/chapters"; +import { makeStyles } from "@material-ui/styles"; type Props = { backUrl: string, @@ -32,7 +33,19 @@ type Props = { match: { params: Object } }; +const useStyles = makeStyles({ + // use flexbox to allow virtualized chapter list fill the remaining + // viewport height with flex-grow: 1 + parent: { + height: "100%", + display: "flex", + flexDirection: "column" + } +}); + const MangaInfo = ({ backUrl, defaultTab, match: { params } }: Props) => { + const classes = useStyles(); + const mangaId = parseInt(params.mangaId, 10); const [tabValue, setTabValue] = useState(defaultTab); @@ -118,7 +131,7 @@ const MangaInfo = ({ backUrl, defaultTab, match: { params } }: Props) => { if (!mangaInfo) return ; return ( - +
{ {tabContent()} {(isMangaInfosLoading || isChaptersLoading) && } - +
); }; diff --git a/src/components/ResponsiveGrid.js b/src/components/ResponsiveGrid.js index 4752565a..1382a5b1 100644 --- a/src/components/ResponsiveGrid.js +++ b/src/components/ResponsiveGrid.js @@ -29,13 +29,16 @@ type Props = { // Optional props spacing: number, - maxWidth: number | "xs" | "sm" | "md" | "lg" | "xl" + maxWidth: number | "xs" | "sm" | "md" | "lg" | "xl", + + parentProps: Object }; // other props get passed to the inner grid const ResponsiveGrid = ({ children, spacing, maxWidth, + parentProps, ...otherProps }: Props) => { const calcMaxWidth = @@ -49,7 +52,7 @@ const ResponsiveGrid = ({ }; return ( - + {children} From 768d7a742acdf055e7cf2a114d95ac2a6361854c Mon Sep 17 00:00:00 2001 From: Alex Wang Date: Sun, 9 Jun 2019 10:40:40 -0700 Subject: [PATCH 10/13] WIP visual updates to virtualized chapter list --- src/components/MangaInfo/ChapterListItem.js | 3 +- .../MangaInfo/MangaInfoChapterList.js | 61 +++++++++++-------- 2 files changed, 36 insertions(+), 28 deletions(-) diff --git a/src/components/MangaInfo/ChapterListItem.js b/src/components/MangaInfo/ChapterListItem.js index c413b3cc..af0b53c4 100644 --- a/src/components/MangaInfo/ChapterListItem.js +++ b/src/components/MangaInfo/ChapterListItem.js @@ -17,7 +17,8 @@ const styles = () => ({ color: "#AAA" }, listItem: { - paddingRight: 8 // decrease padding (default 24) + paddingRight: 8, // decrease padding (default 24) + backgroundColor: "white" }, chapterInfo: { flex: 1, diff --git a/src/components/MangaInfo/MangaInfoChapterList.js b/src/components/MangaInfo/MangaInfoChapterList.js index c67d0ba0..57a82388 100644 --- a/src/components/MangaInfo/MangaInfoChapterList.js +++ b/src/components/MangaInfo/MangaInfoChapterList.js @@ -1,6 +1,5 @@ // @flow import React from "react"; -import List from "@material-ui/core/List"; import Grid from "@material-ui/core/Grid"; import ResponsiveGrid from "components/ResponsiveGrid"; import Paper from "@material-ui/core/Paper"; @@ -20,6 +19,8 @@ import AutoSizer from "react-virtualized-auto-sizer"; // flexDirection: "column" // } +// https://react-window.now.sh/#/examples/list/memoized-list-items + type Props = { mangaInfo: MangaType, chapters: Array, @@ -27,11 +28,15 @@ type Props = { }; const useStyles = makeStyles({ - list: { - paddingTop: 0, - paddingBottom: 0 + virtualizedListParent: { + flexGrow: 1, + marginBottom: 16 }, - virtualizedListParent: { flexGrow: 1 } + paper: { + height: "100%", + backgroundColor: "#fafafa", + overflow: "hidden" + } }); const MangaInfoChapterList = ({ mangaInfo, chapters, toggleRead }: Props) => { @@ -43,28 +48,30 @@ const MangaInfoChapterList = ({ mangaInfo, chapters, toggleRead }: Props) => { parentProps={{ className: classes.virtualizedListParent }} > - {/* itemSize is hard coded using what I saw in the inspector */} - - {({ height, width }) => ( - data[index].id} - > - {({ index, style, data }) => ( - - )} - - )} - + + {/* itemSize is hard coded using what I saw in the inspector */} + + {({ height, width }) => ( + data[index].id} + > + {({ index, style, data }) => ( + + )} + + )} + + ); From dc271c31f92cc923a36480378ba62d90578310fc Mon Sep 17 00:00:00 2001 From: Alex Wang Date: Sun, 9 Jun 2019 11:14:05 -0700 Subject: [PATCH 11/13] Fix naming typo --- src/components/MangaInfo/ChapterMenu.js | 39 ++++++++++++++----------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/src/components/MangaInfo/ChapterMenu.js b/src/components/MangaInfo/ChapterMenu.js index fd1618b8..20cf1359 100644 --- a/src/components/MangaInfo/ChapterMenu.js +++ b/src/components/MangaInfo/ChapterMenu.js @@ -1,30 +1,30 @@ // @flow -import React, { Component } from 'react'; -import Menu from '@material-ui/core/Menu'; -import MenuItem from '@material-ui/core/MenuItem'; -import IconButton from '@material-ui/core/IconButton'; -import Icon from '@material-ui/core/Icon'; -import type { ChapterType } from 'types'; -import { withStyles } from '@material-ui/core/styles'; -import classNames from 'classnames'; +import React, { Component } from "react"; +import Menu from "@material-ui/core/Menu"; +import MenuItem from "@material-ui/core/MenuItem"; +import IconButton from "@material-ui/core/IconButton"; +import Icon from "@material-ui/core/Icon"; +import type { ChapterType } from "types"; +import { withStyles } from "@material-ui/core/styles"; +import classNames from "classnames"; // Because this is nested inside ChapterListItem, which wraps the entire thing with a Link // You have to e.preventDefault() everywhere to stop from navigating to the Reader unintentionally const styles = { - hiddenMenuItem: { display: 'none' }, + hiddenMenuItem: { display: "none" } }; type Props = { classes: Object, // injected styles chapter: ChapterType, - toggleRead: Function, + toggleRead: Function }; type State = { anchorEl: ?HTMLElement }; -class ChapterListItem extends Component { +class ChapterMenu extends Component { state = { - anchorEl: null, + anchorEl: null }; handleClick = (event: SyntheticEvent) => { @@ -51,7 +51,8 @@ class ChapterListItem extends Component { const { anchorEl } = this.state; const showMarkAsRead = !chapter.read; - const showMarkAsUnread = chapter.read || (!chapter.read && chapter.last_page_read > 0); + const showMarkAsUnread = + chapter.read || (!chapter.read && chapter.last_page_read > 0); return ( @@ -62,7 +63,7 @@ class ChapterListItem extends Component { {/* getContentAnchorEl must be null to make anchorOrigin work */} { Mark as Read Mark as Unread @@ -98,4 +103,4 @@ class ChapterListItem extends Component { } } -export default withStyles(styles)(ChapterListItem); +export default withStyles(styles)(ChapterMenu); From 0aec5027262ab05aaf8e4c1d979d8438eb5ad5ff Mon Sep 17 00:00:00 2001 From: Alex Wang Date: Sun, 9 Jun 2019 11:18:30 -0700 Subject: [PATCH 12/13] Implement virtualized chapter list. Not perfect but it works --- src/components/MangaInfo/ChapterListItem.js | 38 +++++------ .../MangaInfo/MangaInfoChapterList.js | 67 +++++++++++-------- src/components/MangaInfo/index.js | 11 +-- 3 files changed, 61 insertions(+), 55 deletions(-) diff --git a/src/components/MangaInfo/ChapterListItem.js b/src/components/MangaInfo/ChapterListItem.js index af0b53c4..8366c652 100644 --- a/src/components/MangaInfo/ChapterListItem.js +++ b/src/components/MangaInfo/ChapterListItem.js @@ -1,7 +1,6 @@ // @flow -import React, { useContext } from "react"; +import React, { useContext, memo } from "react"; import ListItem from "@material-ui/core/ListItem"; -import { withStyles } from "@material-ui/core/styles"; import Typography from "@material-ui/core/Typography"; import classNames from "classnames"; import Link from "components/Link"; @@ -11,8 +10,16 @@ import ChapterMenu from "components/MangaInfo/ChapterMenu"; import UrlPrefixContext from "components/UrlPrefixContext"; import { Client } from "api"; import dateFnsFormat from "date-fns/format"; +import { useDispatch } from "react-redux"; +import { toggleRead } from "redux-ducks/chapters"; +import { makeStyles } from "@material-ui/styles"; -const styles = () => ({ +type Props = { + mangaInfo: MangaType, + chapter: ChapterType +}; // other props will be passed to the root ListItem + +const useStyles = makeStyles({ read: { color: "#AAA" }, @@ -33,20 +40,13 @@ const styles = () => ({ lastReadPage: { color: "rgba(0, 0, 0, 0.87)" } }); -type Props = { - classes: Object, - mangaInfo: MangaType, - chapter: ChapterType, - toggleRead: Function -}; // other props will be passed to the root ListItem +const ChapterListItem = memo(({ mangaInfo, chapter, ...otherProps }: Props) => { + const classes = useStyles(); + const dispatch = useDispatch(); + + const handleToggleRead = (read: boolean) => + dispatch(toggleRead(mangaInfo.id, chapter.id, read)); -const ChapterListItem = ({ - classes, - mangaInfo, - chapter, - toggleRead, - ...otherProps -}: Props) => { const urlPrefix = useContext(UrlPrefixContext); const dimIfRead: Function = (read: boolean): ?String => @@ -85,10 +85,10 @@ const ChapterListItem = ({ - + ); -}; +}); // Helper Functions /* eslint-disable camelcase */ @@ -101,4 +101,4 @@ function chapterText(read: boolean, last_page_read: number) { } /* eslint-enable camelcase */ -export default withStyles(styles)(ChapterListItem); +export default ChapterListItem; diff --git a/src/components/MangaInfo/MangaInfoChapterList.js b/src/components/MangaInfo/MangaInfoChapterList.js index 57a82388..87bde9f6 100644 --- a/src/components/MangaInfo/MangaInfoChapterList.js +++ b/src/components/MangaInfo/MangaInfoChapterList.js @@ -1,12 +1,12 @@ // @flow -import React from "react"; +import React, { memo, createContext, useContext } from "react"; import Grid from "@material-ui/core/Grid"; import ResponsiveGrid from "components/ResponsiveGrid"; import Paper from "@material-ui/core/Paper"; import ChapterListItem from "components/MangaInfo/ChapterListItem"; import type { ChapterType, MangaType } from "types"; import { makeStyles } from "@material-ui/styles"; -import { FixedSizeList } from "react-window"; +import { FixedSizeList, areEqual } from "react-window"; import AutoSizer from "react-virtualized-auto-sizer"; // TODO: I've made ResponsiveGrid maxWidth="xs". What happens when the chapter title is too long? @@ -23,10 +23,27 @@ import AutoSizer from "react-virtualized-auto-sizer"; type Props = { mangaInfo: MangaType, - chapters: Array, - toggleRead: Function + chapters: Array }; +const RowContext = React.createContext(); + +type RowProps = { + index: number, + style: Object, + data: ChapterType +}; +const Row = memo(({ index, style, data }: RowProps) => { + const mangaInfo = useContext(RowContext); + return ( + + ); +}, areEqual); + const useStyles = makeStyles({ virtualizedListParent: { flexGrow: 1, @@ -39,7 +56,7 @@ const useStyles = makeStyles({ } }); -const MangaInfoChapterList = ({ mangaInfo, chapters, toggleRead }: Props) => { +const MangaInfoChapterList = ({ mangaInfo, chapters }: Props) => { const classes = useStyles(); return ( @@ -49,28 +66,24 @@ const MangaInfoChapterList = ({ mangaInfo, chapters, toggleRead }: Props) => { > - {/* itemSize is hard coded using what I saw in the inspector */} - - {({ height, width }) => ( - data[index].id} - > - {({ index, style, data }) => ( - - )} - - )} - + + {/* itemSize is hard coded using what I saw in the inspector */} + + {({ height, width }) => ( + data[index].id} + overscanCount={10} + > + {Row} + + )} + + diff --git a/src/components/MangaInfo/index.js b/src/components/MangaInfo/index.js index 55207e00..234fd988 100644 --- a/src/components/MangaInfo/index.js +++ b/src/components/MangaInfo/index.js @@ -20,8 +20,7 @@ import { selectFilteredSortedChapters, selectChaptersForManga, fetchChapters, - updateChapters, - toggleRead + updateChapters } from "redux-ducks/chapters"; import { makeStyles } from "@material-ui/styles"; @@ -58,8 +57,6 @@ const MangaInfo = ({ backUrl, defaultTab, match: { params } }: Props) => { const isChaptersLoading = useSelector(selectIsChaptersLoading); const dispatch = useDispatch(); - const handleToggleRead = (chapterId, read) => - dispatch(toggleRead(mangaId, chapterId, read)); const store = useStore(); @@ -117,11 +114,7 @@ const MangaInfo = ({ backUrl, defaultTab, match: { params } }: Props) => { - + ); } From 9138b7db5f415e9924ae9ba9e52f8d7d572c9d3f Mon Sep 17 00:00:00 2001 From: Alex Wang Date: Sun, 9 Jun 2019 11:21:57 -0700 Subject: [PATCH 13/13] Don't render chapters if the list is empty --- src/components/MangaInfo/MangaInfoChapterList.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/components/MangaInfo/MangaInfoChapterList.js b/src/components/MangaInfo/MangaInfoChapterList.js index 87bde9f6..95e1388c 100644 --- a/src/components/MangaInfo/MangaInfoChapterList.js +++ b/src/components/MangaInfo/MangaInfoChapterList.js @@ -59,6 +59,8 @@ const useStyles = makeStyles({ const MangaInfoChapterList = ({ mangaInfo, chapters }: Props) => { const classes = useStyles(); + if (!chapters.length) return null; + return (