From 6b3e56a2e799110ed615d3dd2476dccf58c017dd Mon Sep 17 00:00:00 2001 From: Bernhard Reiter Date: Tue, 8 Dec 2015 14:11:53 +0100 Subject: [PATCH 01/12] README: Remove theme-last-event section --- shared/lib/themes/README.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/shared/lib/themes/README.md b/shared/lib/themes/README.md index 6a1cfda63dd72..bce017a5b96c8 100644 --- a/shared/lib/themes/README.md +++ b/shared/lib/themes/README.md @@ -11,10 +11,6 @@ We're transitioning to a more `redux`-like architecture, so our Flux `./stores` Manages data concerning each site's currently selected theme. -### theme-last-event - -Tracks last searched terms and activated theme, for analytics purposes. - ### themes-last-query Tracks the last themes query. From 2389cbf3064c95c4dcdcc43714989676db610cff Mon Sep 17 00:00:00 2001 From: Bernhard Reiter Date: Mon, 14 Dec 2015 14:57:45 +0100 Subject: [PATCH 02/12] Themes: Switch data fetching to Redux MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Substantial refactor of our data fetching components, which are now Redux-`connect`ed. - `ThemesListFetcher`, `ActivatingTheme`, and `CurrentTheme` (i.e. the data component) are kept as generic data components that relay application state to their children. - Selectors are given much greater importance, as they allow us to simplify the data model by avoiding forms of data duplication — notably, avoiding storage of derived data next to essential data. - Use of selectors allowed to altogether eliminate one action: `SEARCH_THEMES`, which was basically a _virtual_ action (read: _hack_) to trigger client-side filtering of themes, a feature specific to Jetpack sites. An @mcsf and @ockham project, with fixes by @ehgy :-) --- .../components/data/activating-theme/index.js | 49 +++--- client/components/data/current-theme/index.js | 70 +++++---- client/my-sites/customize/actions.js | 6 +- client/my-sites/customize/controller.js | 3 +- client/my-sites/customize/main.jsx | 5 +- client/my-sites/upgrades/controller.jsx | 4 +- client/state/index.js | 6 +- .../data/themes-list-fetcher/index.jsx | 145 ++++++++++-------- shared/lib/themes/README.md | 6 - shared/lib/themes/actions.js | 87 +++++------ shared/lib/themes/constants.js | 1 - shared/lib/themes/flux-actions.js | 41 ----- shared/lib/themes/reducers/current-theme.js | 4 +- shared/lib/themes/reducers/index.js | 19 +++ .../lib/themes/reducers/themes-last-query.js | 18 +-- shared/lib/themes/reducers/themes-list.js | 65 +------- shared/lib/themes/reducers/themes.js | 4 +- shared/lib/themes/selectors.js | 83 ++++++++++ shared/my-sites/themes/controller.js | 30 ++-- shared/my-sites/themes/main.jsx | 33 ++-- shared/my-sites/themes/thanks-modal.jsx | 11 +- shared/my-sites/themes/theme-options.js | 7 +- shared/my-sites/themes/themes-selection.jsx | 14 +- .../themes/themes-site-selector-modal.jsx | 4 +- 24 files changed, 359 insertions(+), 356 deletions(-) delete mode 100644 shared/lib/themes/flux-actions.js create mode 100644 shared/lib/themes/reducers/index.js create mode 100644 shared/lib/themes/selectors.js diff --git a/client/components/data/activating-theme/index.js b/client/components/data/activating-theme/index.js index fac44b6fd0a3c..691c703df564c 100644 --- a/client/components/data/activating-theme/index.js +++ b/client/components/data/activating-theme/index.js @@ -1,49 +1,46 @@ /** * External dependencies */ -var React = require( 'react' ); +import React from 'react'; +import { connect } from 'react-redux'; +import omit from 'lodash/object/omit'; /** * Internal dependencies */ -var CurrentThemeStore = require( 'lib/themes/stores/current-theme' ); +import { isActivating, hasActivated, getCurrentTheme } from 'lib/themes/selectors'; -function getState( props ) { +function getState( state, { siteId } ) { return { - isActivating: CurrentThemeStore.isActivating(), - hasActivated: CurrentThemeStore.hasActivated(), - currentTheme: CurrentThemeStore.getCurrentTheme( props.siteId ) + isActivating: isActivating( state ), + hasActivated: hasActivated( state ), + currentTheme: getCurrentTheme( state, siteId ) }; } /** * Passes the activating state of themes to the supplied child component. */ -var ActivatingThemeData = React.createClass( { +const ActivatingThemeData = React.createClass( { propTypes: { + isActivating: React.PropTypes.bool.isRequired, + hasActivated: React.PropTypes.bool.isRequired, + currentTheme: React.PropTypes.shape( { + name: React.PropTypes.string, + id: React.PropTypes.string + } ), children: React.PropTypes.element.isRequired }, - getInitialState: function() { - return getState( this.props ); - }, - - componentWillMount: function() { - CurrentThemeStore.on( 'change', this.onActivatingTheme ); - }, - - componentWillUnmount: function() { - CurrentThemeStore.off( 'change', this.onActivatingTheme ); - }, - - onActivatingTheme: function() { - this.setState( getState( this.props ) ); - }, - - render: function() { - return React.cloneElement( this.props.children, this.state ); + render() { + return React.cloneElement( this.props.children, omit( this.props, 'children' ) ); } } ); -module.exports = ActivatingThemeData; +export default connect( + ( state, props ) => Object.assign( {}, + props, + getState( state, props ) + ) +)( ActivatingThemeData ); diff --git a/client/components/data/current-theme/index.js b/client/components/data/current-theme/index.js index 51c2afaa32eaf..652ac81b1b423 100644 --- a/client/components/data/current-theme/index.js +++ b/client/components/data/current-theme/index.js @@ -1,65 +1,67 @@ /** * External dependencies */ -var React = require( 'react' ); +import React from 'react'; +import { bindActionCreators } from 'redux'; +import { connect } from 'react-redux'; +import omit from 'lodash/object/omit'; /** * Internal dependencies */ -var CurrentThemeStore = require( 'lib/themes/stores/current-theme' ), - Actions = require( 'lib/themes/flux-actions' ); +import { fetchCurrentTheme } from 'lib/themes/actions'; +import { getCurrentTheme } from 'lib/themes/selectors'; + +function getState( state, { site } ) { + return { + currentTheme: getCurrentTheme( state, site.ID ) + }; +} /** * Fetches the currently active theme of the supplied site * and passes it to the supplied child component. */ -var CurrentThemeData = React.createClass( { +const CurrentThemeData = React.createClass( { propTypes: { site: React.PropTypes.oneOfType( [ React.PropTypes.object, React.PropTypes.bool ] ).isRequired, - children: React.PropTypes.element.isRequired - }, - - getInitialState: function() { - return { - currentTheme: CurrentThemeStore.getCurrentTheme( this.props.site.ID ) - }; + currentTheme: React.PropTypes.shape( { + name: React.PropTypes.string, + id: React.PropTypes.string + } ), + fetchCurrentTheme: React.PropTypes.func.isRequired, + children: React.PropTypes.element.isRequired, }, - componentWillMount: function() { - CurrentThemeStore.on( 'change', this.onCurrentThemeChange ); - - if ( ! this.state.currentTheme && this.props.site ) { - Actions.fetchCurrentTheme( this.props.site ); - } + componentDidMount() { + this.refresh( this.props ); }, - componentWillReceiveProps: function( nextProps ) { - if ( this.state.currentTheme ) { - return; - } - + componentWillReceiveProps( nextProps ) { if ( nextProps.site && nextProps.site !== this.props.site ) { - Actions.fetchCurrentTheme( nextProps.site ); + this.refresh( nextProps ); } }, - componentWillUnmount: function() { - CurrentThemeStore.off( 'change', this.onCurrentThemeChange ); - }, - - onCurrentThemeChange: function() { - this.setState( { - currentTheme: CurrentThemeStore.getCurrentTheme( this.props.site.ID ) - } ); + refresh( props ) { + if ( ! this.props.currentTheme && props.site ) { + this.props.fetchCurrentTheme( props.site ); + } }, - render: function() { - return React.cloneElement( this.props.children, this.state ); + render() { + return React.cloneElement( this.props.children, omit( this.props, 'children' ) ); } } ); -module.exports = CurrentThemeData; +export default connect( + ( state, props ) => Object.assign( {}, + props, + getState( state, props ) + ), + bindActionCreators.bind( null, { fetchCurrentTheme } ) +)( CurrentThemeData ); diff --git a/client/my-sites/customize/actions.js b/client/my-sites/customize/actions.js index 9706f8a2a843b..99a3107f5bc9f 100644 --- a/client/my-sites/customize/actions.js +++ b/client/my-sites/customize/actions.js @@ -10,7 +10,7 @@ var Dispatcher = require( 'dispatcher' ), page = require( 'page' ), wpcom = require( 'lib/wp' ), CartActions = require( 'lib/upgrades/actions' ), - ThemeActions = require( 'lib/themes/flux-actions' ), + activated = require( 'lib/themes/actions' ).activated, ThemeHelper = require( 'lib/themes/helpers' ), themeItem = require( 'lib/cart-values/cart-items' ).themeItem; @@ -41,12 +41,12 @@ var CustomizeActions = { } ); }, - activated: function( id, site ) { + activated: function( id, site, dispatch ) { ThemeHelper.trackClick( 'customizer', 'activate' ); page( '/design/' + site.slug ); - ThemeActions.activated( id, site, 'customizer' ); + dispatch( activated( id, site, 'customizer' ) ); Dispatcher.handleViewAction( { type: 'ACTIVATED_THEME_WITH_CUSTOMIZER', diff --git a/client/my-sites/customize/controller.js b/client/my-sites/customize/controller.js index b51a060c8c207..e59c043ad3142 100644 --- a/client/my-sites/customize/controller.js +++ b/client/my-sites/customize/controller.js @@ -30,7 +30,8 @@ module.exports = { domain: context.params.domain || '', sites: sites, prevPath: context.prevPath || '', - query: Qs.parse( context.querystring ) + query: Qs.parse( context.querystring ), + dispatchRedux: context.store.dispatch } ), document.getElementById( 'primary' ) ); diff --git a/client/my-sites/customize/main.jsx b/client/my-sites/customize/main.jsx index 5c6d6dad6ead5..1a8f22437c383 100644 --- a/client/my-sites/customize/main.jsx +++ b/client/my-sites/customize/main.jsx @@ -28,7 +28,8 @@ module.exports = React.createClass( { propTypes: { domain: React.PropTypes.string.isRequired, sites: React.PropTypes.object.isRequired, - prevPath: React.PropTypes.string + prevPath: React.PropTypes.string, + dispatchRedux: React.PropTypes.func.isRequired }, getDefaultProps: function() { @@ -258,7 +259,7 @@ module.exports = React.createClass( { break; case 'activated': themeSlug = message.theme.stylesheet.split( '/' )[1]; - Actions.activated( themeSlug, site ); + Actions.activated( themeSlug, site, this.props.dispatchRedux ); break; case 'purchased': themeSlug = message.theme.stylesheet.split( '/' )[1]; diff --git a/client/my-sites/upgrades/controller.jsx b/client/my-sites/upgrades/controller.jsx index f48a93dc38783..59f40295d4209 100644 --- a/client/my-sites/upgrades/controller.jsx +++ b/client/my-sites/upgrades/controller.jsx @@ -12,7 +12,7 @@ var analytics = require( 'analytics' ), sites = require( 'lib/sites-list' )(), route = require( 'lib/route' ), i18n = require( 'lib/mixins/i18n' ), - ThemeActions = require( 'lib/themes/flux-actions' ), + activated = require( 'lib/themes/actions' ).activated, Main = require( 'components/main' ), upgradesActions = require( 'lib/upgrades/actions' ), titleActions = require( 'lib/screen-title/actions' ), @@ -246,7 +246,7 @@ module.exports = { if ( cartItems.hasOnlyProductsOf( cart, 'premium_theme' ) ) { const { meta, extra: { source } } = cartAllItems[ 0 ]; - ThemeActions.activated( meta, selectedSite, source, true ); + context.store.dispatch( activated( meta, selectedSite, source, true ) ); page.redirect( '/design/' + selectedSite.slug ); return; } diff --git a/client/state/index.js b/client/state/index.js index 4db942fa16ff6..bcfcb7d4a3d8b 100644 --- a/client/state/index.js +++ b/client/state/index.js @@ -7,9 +7,11 @@ import { createStore, applyMiddleware, combineReducers } from 'redux'; /** * Internal dependencies */ +import { analyticsMiddleware } from 'lib/themes/middlewares.js'; import sharing from './sharing/reducer'; import sites from './sites/reducer'; import siteSettings from './site-settings/reducer' +import themes from 'lib/themes/reducers'; import ui from './ui/reducer'; /** @@ -19,11 +21,13 @@ const reducer = combineReducers( { sharing, sites, siteSettings, + themes, ui } ); export function createReduxStore() { return applyMiddleware( - thunkMiddleware + thunkMiddleware, + analyticsMiddleware )( createStore )( reducer ); }; diff --git a/shared/components/data/themes-list-fetcher/index.jsx b/shared/components/data/themes-list-fetcher/index.jsx index e448efe07ce2e..f3386fade25eb 100644 --- a/shared/components/data/themes-list-fetcher/index.jsx +++ b/shared/components/data/themes-list-fetcher/index.jsx @@ -1,44 +1,38 @@ /** * External dependencies */ -var React = require( 'react' ), - omit = require( 'lodash/object/omit' ), - once = require( 'lodash/function/once' ), - isEqual = require( 'lodash/lang/isEqual' ); +import React from 'react'; +import omit from 'lodash/object/omit'; +import once from 'lodash/function/once'; +import { bindActionCreators } from 'redux'; +import { connect } from 'react-redux'; /** * Internal dependencies */ -var ThemesStore = require( 'lib/themes/stores/themes' ), - ThemesListStore = require( 'lib/themes/stores/themes-list' ), - Actions = require( 'lib/themes/flux-actions' ), - Constants = require( 'lib/themes/constants' ); - -function queryThemes( props ) { - Actions.query( { - search: props.search, - tier: props.tier, - page: 0, - perPage: Constants.PER_PAGE - } ); - - Actions.fetchNextPage( props.site ); -} - -function getThemesInList() { - return ThemesListStore.getThemesList().map( ThemesStore.getById ); -} - -function getThemesState() { +import Constants from 'lib/themes/constants'; +import { query, fetchNextPage } from 'lib/themes/actions'; +import { + getFilteredThemes, + hasSiteChanged, + isJetpack, + isLastPage, + isFetchingNextPage +} from 'lib/themes/selectors'; + +function getState( state, { search } ) { return { - themes: getThemesInList(), - lastPage: ThemesListStore.isLastPage(), - loading: ThemesListStore.isFetchingNextPage(), - search: ThemesListStore.getQueryParams().search + themes: getFilteredThemes( state, search ), + lastPage: isLastPage( state ), + loading: isFetchingNextPage( state ), + lastQuery: { + hasSiteChanged: hasSiteChanged( state ), + isJetpack: isJetpack( state ) + } }; } -let ThemesListFetcher = React.createClass( { +const ThemesListFetcher = React.createClass( { propTypes: { children: React.PropTypes.element.isRequired, site: React.PropTypes.oneOfType( [ @@ -49,76 +43,91 @@ let ThemesListFetcher = React.createClass( { search: React.PropTypes.string, tier: React.PropTypes.string, onRealScroll: React.PropTypes.func, - onLastPage: React.PropTypes.func - }, + onLastPage: React.PropTypes.func, - getInitialState: function() { - return Object.assign( getThemesState(), { loading: true } ); + themes: React.PropTypes.array.isRequired, + lastPage: React.PropTypes.bool.isRequired, + loading: React.PropTypes.bool.isRequired, + query: React.PropTypes.func.isRequired, + fetchNextPage: React.PropTypes.func.isRequired, }, componentDidMount: function() { - ThemesListStore.on( 'change', this.onThemesChange ); - if ( this.props.site || this.props.isMultisite ) { - this.queryThemes( this.props ); - } - }, - - componentWillUnmount: function() { - ThemesListStore.off( 'change', this.onThemesChange ); + this.refresh( this.props ); }, componentWillReceiveProps: function( nextProps ) { - const ignoreProps = [ 'children', 'onLastPage', 'site' ]; - - if ( isEqual( - omit( this.props, ignoreProps ), - omit( nextProps, ignoreProps ) ) ) { - return; + if ( + nextProps.tier !== this.props.tier || ( + nextProps.search !== this.props.search && ( + ! nextProps.lastQuery.isJetpack || + nextProps.lastQuery.hasSiteChanged + ) + ) + ) { + this.refresh( nextProps ); } + }, - if ( nextProps.site || nextProps.isMultisite ) { - this.queryThemes( nextProps ); + refresh: function( props ) { + if ( this.props.site || this.props.isMultisite ) { + this.queryThemes( props ); } }, queryThemes: function( props ) { - const { onLastPage } = this.props; - this.onLastPage = onLastPage ? once( onLastPage ) : null; - queryThemes( props ); - }, + const { + onLastPage, + site, + search, + tier, + } = props; - onThemesChange: function() { - this.setState( getThemesState() ); + this.onLastPage = onLastPage ? once( onLastPage ) : null; - const { page } = ThemesListStore.getQueryParams(); - const { loading, lastPage } = this.state; + this.props.query( { + search, + tier, + page: 0, + perPage: Constants.PER_PAGE, + } ); - if ( page > 1 && ! loading && lastPage ) { - this.onLastPage && this.onLastPage(); - } + this.props.fetchNextPage( site ); }, fetchNextPage: function( options ) { // FIXME: While this function is passed on by `ThemesList` to `InfiniteList`, // which has a `shouldLoadNextPage()` check (in scroll-helper.js), we can't // rely on that; fetching would break without the following check. - if ( this.state.loading || this.state.lastPage ) { + if ( this.props.loading || this.props.lastPage ) { return; } + const { + site = false, + onRealScroll = () => null, + } = this.props; + if ( options.triggeredByScroll ) { - this.props.onRealScroll && this.props.onRealScroll(); + onRealScroll(); } - Actions.fetchNextPage( this.props.site ); + this.props.fetchNextPage( site ); }, render: function() { - var childrenProps = Object.assign( { fetchNextPage: this.fetchNextPage }, this.state ); - // Clone the child element along and pass along state (containing data from the store) - return React.cloneElement( this.props.children, childrenProps ); + const props = omit( this.props, 'children' ); + return React.cloneElement( this.props.children, Object.assign( {}, props, { + fetchNextPage: this.fetchNextPage + } ) ); } } ); -module.exports = ThemesListFetcher; +export default connect( + ( state, props ) => Object.assign( {}, + props, + getState( state, props ) + ), + bindActionCreators.bind( null, { query, fetchNextPage } ) +)( ThemesListFetcher ); diff --git a/shared/lib/themes/README.md b/shared/lib/themes/README.md index bce017a5b96c8..3bb286ecf87f4 100644 --- a/shared/lib/themes/README.md +++ b/shared/lib/themes/README.md @@ -30,12 +30,6 @@ Contains a global list of themes queried so far. Redux actions. Note that async actions require the [redux-thunk][thunk] middleware. Examples can be found inside. -### flux-actions - -Flux wrapper around the Redux actions. Uses Redux's -[`bindActionCreators`][bind] along with our own `combineStores()` -for wrapping. Also applies the [thunk] and [analytics] middlewares. - ## Middlewares ### middlewares diff --git a/shared/lib/themes/actions.js b/shared/lib/themes/actions.js index 07f2250911e72..b2ff0d9ababb0 100644 --- a/shared/lib/themes/actions.js +++ b/shared/lib/themes/actions.js @@ -11,15 +11,17 @@ const debug = debugFactory( 'calypso:themes:actions' ); //eslint-disable-line no */ import ThemeConstants from 'lib/themes/constants'; import ThemeHelpers from './helpers'; -import { getCurrentTheme } from './reducers/current-theme'; -import { getThemeById } from './reducers/themes'; -import { getQueryParams } from './reducers/themes-list'; -import { hasSiteChanged, hasParams } from './reducers/themes-last-query'; +import { + getThemeById, + getCurrentTheme, + getQueryParams, + isJetpack +} from './selectors'; import wpcom from 'lib/wp'; export function fetchThemes( site ) { return ( dispatch, getState ) => { - const queryParams = getQueryParams( getState().themes.themesList ); + const queryParams = getQueryParams( getState() ); const startTime = new Date().getTime(); const callback = ( error, data ) => { debug( 'Received themes data', data ); @@ -36,32 +38,10 @@ export function fetchThemes( site ) { } } -/* - * On Jetpack sites, we're not querying themes using the REST API, but fetch all of the installed - * themes, and filter them within Calypso. - */ -export function fetchJetpackThemes( site ) { - return ( dispatch, getState ) => { - const themesLastQueryState = getState().themes.themesLastQuery; - if ( hasSiteChanged( themesLastQueryState ) || ! hasParams( themesLastQueryState ) ) { - return dispatch( fetchThemes( site ) ); - } - - dispatch( { - type: ThemeConstants.SEARCH_THEMES, - } ); - } -} - export function fetchNextPage( site ) { return dispatch => { dispatch( incrementThemesPage( site ) ); - - if ( site.jetpack ) { - dispatch( fetchJetpackThemes( site ) ); - } else { - dispatch( fetchThemes( site ) ); - } + dispatch( fetchThemes( site ) ); } } @@ -104,32 +84,35 @@ export function receiveServerError( error ) { } export function receiveThemes( data, site, queryParams, responseTime ) { - let meta = {}; - - if ( queryParams.search && queryParams.page === 1 ) { - meta = { - analytics: { - type: 'calypso_themeshowcase_search', - payload: { - search_term: queryParams.search || null, - tier: queryParams.tier, - response_time_in_ms: responseTime, - result_count: data.found, - results_first_page: data.themes.map( theme => theme.id ) + return ( dispatch, getState ) => { + let meta = {}; + + if ( queryParams.search && queryParams.page === 1 ) { + meta = { + analytics: { + type: 'calypso_themeshowcase_search', + payload: { + search_term: queryParams.search || null, + tier: queryParams.tier, + response_time_in_ms: responseTime, + result_count: data.found, + results_first_page: data.themes.map( theme => theme.id ) + } } } } - } - return { - type: ThemeConstants.RECEIVE_THEMES, - siteId: site.ID, - isJetpack: !! site.jetpack, - themes: data.themes, - found: data.found, - queryParams: queryParams, - meta - }; + dispatch( { + type: ThemeConstants.RECEIVE_THEMES, + siteId: site.ID, + isJetpack: !! site.jetpack, + wasJetpack: isJetpack( getState() ), + themes: data.themes, + found: data.found, + queryParams: queryParams, + meta + } ); + } } export function activate( theme, site, source = 'unknown' ) { @@ -154,11 +137,11 @@ export function activate( theme, site, source = 'unknown' ) { export function activated( theme, site, source = 'unknown', purchased = false ) { return ( dispatch, getState ) => { - const previousTheme = getCurrentTheme( getState().themes.currentTheme, site.ID ); + const previousTheme = getCurrentTheme( getState(), site.ID ); const queryParams = getState().themes.themesList.get( 'query' ); if ( typeof theme !== 'object' ) { - theme = getThemeById( getState().themes.themes, theme ); + theme = getThemeById( getState(), theme ); } defer( () => dispatch( { diff --git a/shared/lib/themes/constants.js b/shared/lib/themes/constants.js index 84dbe9abc9a58..f5411688ef52a 100644 --- a/shared/lib/themes/constants.js +++ b/shared/lib/themes/constants.js @@ -17,7 +17,6 @@ module.exports = assign( keyMirror( { ACTIVATE_THEME: null, ACTIVATED_THEME: null, CLEAR_ACTIVATED_THEME: null, - SEARCH_THEMES: null, THEME_DETAILS: null, THEME_SUPPORT: null, THEME_CUSTOMIZE: null diff --git a/shared/lib/themes/flux-actions.js b/shared/lib/themes/flux-actions.js deleted file mode 100644 index 6053b096e79ee..0000000000000 --- a/shared/lib/themes/flux-actions.js +++ /dev/null @@ -1,41 +0,0 @@ -/** - * External dependencies - */ -import { applyMiddleware } from 'redux'; -import { bindActionCreators } from 'redux'; -import thunkMiddleware from 'redux-thunk'; -import debugModule from 'debug'; -const debug = debugModule( 'calypso:themes:flux-actions' ); //eslint-disable-line no-unused-vars - -/** - * Internal dependencies - */ -import ThemesStore from './stores/themes'; -import ThemesListStore from './stores/themes-list'; -import CurrentThemeStore from './stores/current-theme'; -import ThemesLastQueryStore from './stores/themes-last-query'; -import * as actions from './actions'; -import { combineStores } from 'lib/store'; -import { analyticsMiddleware } from './middlewares.js'; - -const combineStoresWithMiddleware = applyMiddleware( - thunkMiddleware, - analyticsMiddleware -)( combineStores ); - -const combinedStore = combineStoresWithMiddleware( { - themes: ThemesStore, - themesList: ThemesListStore, - themesLastQuery: ThemesLastQueryStore, - currentTheme: CurrentThemeStore -}, 'themes' ); - -/** - * As we're wrapping `./actions` in bulk here, we cannot individually specify - * the `source` argument to `combinedStore.dispatch()` (`VIEW_ACTION` or `SERVER_ACTION`) -- - * cf. `shared/dispatcher`. This doesn't seem to be much of a concern, as we aren't currently - * using that information anyway. To retain it, we'd need to call `bindActionCreators()` - * individually for each action, and bind `combinedStore.dispatch` to the desired `source` - * constant. - */ -export default bindActionCreators( actions, combinedStore.dispatch ); diff --git a/shared/lib/themes/reducers/current-theme.js b/shared/lib/themes/reducers/current-theme.js index 98021c1c6fbeb..21a3393798ee6 100644 --- a/shared/lib/themes/reducers/current-theme.js +++ b/shared/lib/themes/reducers/current-theme.js @@ -14,9 +14,7 @@ export const initialState = fromJS( { currentThemes: {} } ); -export const reducer = ( state = initialState, payload ) => { - const { action = payload } = payload; - +export const reducer = ( state = initialState, action ) => { switch ( action.type ) { case ThemeConstants.RECEIVE_CURRENT_THEME: return state.setIn( [ 'currentThemes', action.site.ID ], { diff --git a/shared/lib/themes/reducers/index.js b/shared/lib/themes/reducers/index.js new file mode 100644 index 0000000000000..317e3ab0b8a4a --- /dev/null +++ b/shared/lib/themes/reducers/index.js @@ -0,0 +1,19 @@ +/** + * External dependencies + */ +import { combineReducers } from 'redux'; + +/** + * Internal dependencies + */ +import { reducer as themes } from './themes'; +import { reducer as themesList } from './themes-list'; +import { reducer as themesLastQuery } from './themes-last-query'; +import { reducer as currentTheme } from './current-theme'; + +export default combineReducers( { + themes, + themesList, + themesLastQuery, + currentTheme +} ); diff --git a/shared/lib/themes/reducers/themes-last-query.js b/shared/lib/themes/reducers/themes-last-query.js index 76ec960d9b08c..4337115085194 100644 --- a/shared/lib/themes/reducers/themes-last-query.js +++ b/shared/lib/themes/reducers/themes-last-query.js @@ -15,19 +15,7 @@ export const initialState = fromJS( { lastParams: null, } ); -export const reducer = ( state = initialState, payload ) => { - const { action = payload } = payload; - - // FIXME To fully convert this store to a reducer, we need to remove - // dependency on the dispatcher (and, by extension, other stores). Will - // probably be easier to do down the road when we have better - // infrastructure to accommodate reducers? - const Dispatcher = require( 'dispatcher' ); - const ThemesListStore = require( '../stores/themes-list' ); - if ( Dispatcher._isDispatching ) { - Dispatcher.waitFor( [ ThemesListStore.dispatchToken ] ); - } - +export const reducer = ( state = initialState, action ) => { switch ( action.type ) { case ThemeConstants.QUERY_THEMES: return state.set( 'lastParams', action.params ); @@ -37,10 +25,8 @@ export const reducer = ( state = initialState, payload ) => { .set( 'previousSiteId', state.get( 'currentSiteId' ) ) .set( 'currentSiteId', action.site.ID ) .set( 'isJetpack', !! action.site.jetpack ); - - case ThemeConstants.SEARCH_THEMES: - return state.set( 'lastParams', null ); } + return state; }; diff --git a/shared/lib/themes/reducers/themes-list.js b/shared/lib/themes/reducers/themes-list.js index 785132cef0bcb..35ebd6097f1fb 100644 --- a/shared/lib/themes/reducers/themes-list.js +++ b/shared/lib/themes/reducers/themes-list.js @@ -2,15 +2,12 @@ * External dependencies */ import { fromJS } from 'immutable'; -import filter from 'lodash/collection/filter'; import pluck from 'lodash/collection/pluck'; import unique from 'lodash/array/unique'; /** * Internal dependencies */ -import ThemesStore from '../stores/themes'; -import ThemesLastQueryStore from '../stores/themes-last-query'; import ThemeConstants from '../constants'; const defaultQuery = fromJS( { @@ -33,7 +30,7 @@ export const initialState = query( fromJS( { } ) ); /** - * Mutating helpers + * Helpers */ function add( ids, list ) { @@ -51,46 +48,13 @@ function query( state, params = {} ) { .update( 'nextId', id => id + 1 ); } -function searchJetpackThemes( state ) { - if ( ! ThemesLastQueryStore.isJetpack() ) { - return state; - } - - const { search } = ThemesLastQueryStore.getParams(); - const themes = search - ? filter( ThemesStore.getThemes(), theme => matches( theme, search ) ) - : ThemesStore.getThemes(); - - return state.set( 'list', pluck( themes, 'id' ) ); -} - -/** - * Pure helpers - */ - function isActionForLastPage( list, action ) { return ! action.found || list.length === action.found || action.themes.length === 0; } -function join( value ) { - return Array.isArray( value ) ? value.join( ' ' ) : value; -} - -function matches( theme, rawSearch ) { - const search = rawSearch.toLowerCase().trim(); - - return [ 'name', 'tags', 'description', 'author' ].some( field => ( - theme[ field ] && join( theme[ field ] ) - .toLowerCase().replace( '-', ' ' ) - .indexOf( search ) >= 0 - ) ); -} - -export const reducer = ( state = initialState, payload ) => { - const { action = payload } = payload; - +export const reducer = ( state = initialState, action ) => { switch ( action.type ) { case ThemeConstants.QUERY_THEMES: return query( state, action.params ); @@ -98,15 +62,14 @@ export const reducer = ( state = initialState, payload ) => { case ThemeConstants.RECEIVE_THEMES: if ( ( action.queryParams.id === state.getIn( [ 'query', 'id' ] ) ) || - ThemesLastQueryStore.isJetpack() + action.wasJetpack ) { const newState = state .setIn( [ 'queryState', 'isFetchingNextPage' ], false ) .update( 'list', add.bind( null, pluck( action.themes, 'id' ) ) ); - return searchJetpackThemes( - newState.setIn( [ 'queryState', 'isLastPage' ], - isActionForLastPage( newState.get( 'list' ), action ) ) ); + return newState.setIn( [ 'queryState', 'isLastPage' ], + isActionForLastPage( newState.get( 'list' ), action ) ); } return state; @@ -126,23 +89,7 @@ export const reducer = ( state = initialState, payload ) => { // state is different from the old one, we need something to change // here. return state.set( 'active', action.theme.id ); - - case ThemeConstants.SEARCH_THEMES: - return searchJetpackThemes( - state.setIn( [ 'queryState', 'isFetchingNextPage' ], false ) - ); } + return state; }; - -export function getThemesList( state ) { - return state.get( 'list' ); -} - -export function getQueryParams( state ) { - return state.get( 'query' ).toObject(); -} - -export function isFetchingNextPage( state ) { - return state.getIn( [ 'queryState', 'isFetchingNextPage' ] ); -} diff --git a/shared/lib/themes/reducers/themes.js b/shared/lib/themes/reducers/themes.js index cd9495fd11180..88ff0d4025130 100644 --- a/shared/lib/themes/reducers/themes.js +++ b/shared/lib/themes/reducers/themes.js @@ -26,9 +26,7 @@ function setActiveTheme( themeId, themes ) { .setIn( [ themeId, 'active' ], true ); } -export const reducer = ( state = initialState, payload ) => { - const { action = payload } = payload; - +export const reducer = ( state = initialState, action ) => { switch ( action.type ) { case ThemeConstants.RECEIVE_THEMES: const isNewSite = action.isJetpack && ( action.siteId !== state.get( 'currentSiteId' ) ); diff --git a/shared/lib/themes/selectors.js b/shared/lib/themes/selectors.js new file mode 100644 index 0000000000000..1e8076310e675 --- /dev/null +++ b/shared/lib/themes/selectors.js @@ -0,0 +1,83 @@ +/** + * External dependencies + */ +import filter from 'lodash/collection/filter'; + +export function isJetpack( state ) { + return state.themes.themesLastQuery.get( 'isJetpack' ); +} + +export function getParams( state ) { + return state.themes.themesLastQuery.get( 'lastParams' ) || {}; +} + +export function hasSiteChanged( state ) { + return state.themes.themesLastQuery.get( 'previousSiteId' ) !== + state.themes.themesLastQuery.get( 'currentSiteId' ); +}; + +export function hasParams( state ) { + return !! state.themes.themesLastQuery.get( 'lastParams' ); +} + +export function isFetchingNextPage( state ) { + return state.themes.themesList.getIn( [ 'queryState', 'isFetchingNextPage' ] ); +} + +export function isLastPage( state ) { + return state.themes.themesList.getIn( [ 'queryState', 'isLastPage' ] ); +} + +export function getThemes( state ) { + return state.themes.themes.get( 'themes' ).toJS(); +} + +export function getThemeById( state, id ) { + const theme = state.themes.themes.getIn( [ 'themes', id ] ); + return theme ? theme.toJS() : undefined; +} + +export function getThemesList( state ) { + return state.themes.themesList.get( 'list' ); +} + +export function getQueryParams( state ) { + return state.themes.themesList.get( 'query' ).toObject(); +} + +export function getFilteredThemes( state, search ) { + const allThemes = getThemesList( state ) + .map( getThemeById.bind( null, state ) ); + + if ( ! isJetpack( state ) || ! search ) { + return allThemes; + } + + return filter( allThemes, theme => matches( theme, search ) ); +} + +export function getCurrentTheme( state, siteId ) { + return state.themes.currentTheme.get( 'currentThemes' ).get( siteId ); +} + +export function isActivating( state ) { + return state.themes.currentTheme.get( 'isActivating' ); +} + +export function hasActivated( state ) { + return state.themes.currentTheme.get( 'hasActivated' ); +} + +function matches( theme, rawSearch ) { + const search = rawSearch.toLowerCase().trim(); + + return [ 'name', 'tags', 'description', 'author' ].some( field => ( + theme[ field ] && join( theme[ field ] ) + .toLowerCase().replace( '-', ' ' ) + .indexOf( search ) >= 0 + ) ); +} + +function join( value ) { + return Array.isArray( value ) ? value.join( ' ' ) : value; +} diff --git a/shared/my-sites/themes/controller.js b/shared/my-sites/themes/controller.js index 70b69042e3d9b..ae4de6c53775e 100644 --- a/shared/my-sites/themes/controller.js +++ b/shared/my-sites/themes/controller.js @@ -3,6 +3,7 @@ */ var ReactDom = require( 'react-dom' ), React = require( 'react' ), + ReduxProvider = require( 'react-redux' ).Provider, titlecase = require( 'to-title-case' ); /** @@ -40,19 +41,22 @@ var controller = { analytics.pageView.record( basePath, analyticsPageTitle ); ReactDom.render( - React.createElement( ThemesComponent, { - key: site_id, - siteId: site_id, - sites: sites, - tier: tier, - search: context.query.s, - trackScrollPage: trackScrollPage.bind( - null, - basePath, - analyticsPageTitle, - 'Themes' - ) - } ), + React.createElement( ReduxProvider, { store: context.store }, + React.createElement( ThemesComponent, { + key: site_id, + siteId: site_id, + sites: sites, + tier: tier, + search: context.query.s, + trackScrollPage: trackScrollPage.bind( + null, + basePath, + analyticsPageTitle, + 'Themes' + ), + store: context.store + } ) + ), document.getElementById( 'primary' ) ); } diff --git a/shared/my-sites/themes/main.jsx b/shared/my-sites/themes/main.jsx index 605dc669b6a09..4dc365b4e75e9 100644 --- a/shared/my-sites/themes/main.jsx +++ b/shared/my-sites/themes/main.jsx @@ -2,6 +2,7 @@ * External dependencies */ var React = require( 'react/addons' ), + bindActionCreators = require( 'redux' ).bindActionCreators, partialRight = require( 'lodash/function/partialRight' ); /** @@ -10,7 +11,7 @@ var React = require( 'react/addons' ), var Main = require( 'components/main' ), CurrentThemeData = require( 'components/data/current-theme' ), ActivatingTheme = require( 'components/data/activating-theme' ), - Action = require( 'lib/themes/flux-actions' ), + Action = require( 'lib/themes/actions' ), WebPreview = require( 'components/web-preview' ), Button = require( 'components/button' ), CurrentTheme = require( 'my-sites/themes/current-theme' ), @@ -31,7 +32,8 @@ var Themes = React.createClass( { propTypes: { siteId: React.PropTypes.string, - sites: React.PropTypes.object.isRequired + sites: React.PropTypes.object.isRequired, + store: React.PropTypes.object.isRequired }, getInitialState: function() { @@ -55,7 +57,9 @@ var Themes = React.createClass( { renderThankYou: function() { return ( - + ); }, @@ -67,7 +71,7 @@ var Themes = React.createClass( { togglePreview: function( theme ) { const site = this.props.sites.getSelectedSite(); if ( site.jetpack ) { - Action.customize( theme, site ); + this.props.store.dispatch( Action.customize( theme, site ) ); } else { const previewUrl = ThemeHelpers.getPreviewUrl( theme, site ); this.setState( { showPreview: ! this.state.showPreview, previewUrl: previewUrl, previewingTheme: theme } ); @@ -97,7 +101,8 @@ var Themes = React.createClass( { render: function() { var site = this.props.sites.getSelectedSite(), isJetpack = site.jetpack, - jetpackEnabled = config.isEnabled( 'manage/themes-jetpack' ); + jetpackEnabled = config.isEnabled( 'manage/themes-jetpack' ), + dispatch = this.props.store.dispatch; if ( isJetpack && jetpackEnabled && ! site.hasJetpackThemes ) { return ; @@ -121,7 +126,7 @@ var Themes = React.createClass( {