Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Themes: Switch data fetching to Redux #840

Merged
merged 12 commits into from
Dec 16, 2015
60 changes: 29 additions & 31 deletions client/components/data/activating-theme/index.js
Original file line number Diff line number Diff line change
@@ -1,49 +1,47 @@
/**
* 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' );

function getState( props ) {
return {
isActivating: CurrentThemeStore.isActivating(),
hasActivated: CurrentThemeStore.hasActivated(),
currentTheme: CurrentThemeStore.getCurrentTheme( props.siteId )
};
}
import {
isActivating,
hasActivated,
getCurrentTheme
} from 'lib/themes/selectors/current-theme';

/**
* Passes the activating state of themes to the supplied child component.
*/
var ActivatingThemeData = React.createClass( {
const ActivatingThemeData = React.createClass( {

propTypes: {
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 ) );
children: React.PropTypes.element.isRequired,
// Connected props
isActivating: React.PropTypes.bool.isRequired,
hasActivated: React.PropTypes.bool.isRequired,
currentTheme: React.PropTypes.shape( {
name: React.PropTypes.string,
id: React.PropTypes.string
} )
},

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( {},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to assign into the component's own props as you're doing here. Instead, this could be simplified to:

export default connect(
    ( state, props ) => ( {
        isActivating: isActivating( state ),
        hasActivated: hasActivated( state ),
        currentTheme: getCurrentTheme( state, props.siteId )
    } )
)( ActivatingThemeData );

The connected props will be automatically merged with any other props passed to the component explicitly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for bringing that up! That goes on my things-to-refactor in our Redux subtree list... I need to file an issue for that TODO... :-)

props,
{
isActivating: isActivating( state ),
hasActivated: hasActivated( state ),
currentTheme: getCurrentTheme( state, props.siteId )
}
)
)( ActivatingThemeData );
67 changes: 33 additions & 34 deletions client/components/data/current-theme/index.js
Original file line number Diff line number Diff line change
@@ -1,65 +1,64 @@
/**
* 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/current-theme';

/**
* 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: {
children: React.PropTypes.element.isRequired,
site: React.PropTypes.oneOfType( [
React.PropTypes.object,
React.PropTypes.bool
] ).isRequired,
children: React.PropTypes.element.isRequired
// Connected props
currentTheme: React.PropTypes.shape( {
name: React.PropTypes.string,
id: React.PropTypes.string
} ),
fetchCurrentTheme: React.PropTypes.func.isRequired
},

getInitialState: function() {
return {
currentTheme: CurrentThemeStore.getCurrentTheme( this.props.site.ID )
};
componentDidMount() {
this.refresh( this.props );
},

componentWillMount: function() {
CurrentThemeStore.on( 'change', this.onCurrentThemeChange );

if ( ! this.state.currentTheme && this.props.site ) {
Actions.fetchCurrentTheme( this.props.site );
}
},

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,
{
currentTheme: getCurrentTheme( state, props.site.ID )
}
),
bindActionCreators.bind( null, { fetchCurrentTheme } )
)( CurrentThemeData );
9 changes: 6 additions & 3 deletions client/my-sites/customize/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ var Dispatcher = require( 'dispatcher' ),
page = require( 'page' ),
wpcom = require( 'lib/wp' ),
CartActions = require( 'lib/upgrades/actions' ),
ThemeActions = require( 'lib/themes/flux-actions' ),
ThemeHelper = require( 'lib/themes/helpers' ),
themeItem = require( 'lib/cart-values/cart-items' ).themeItem;

Expand Down Expand Up @@ -41,12 +40,16 @@ var CustomizeActions = {
} );
},

activated: function( id, site ) {
// TODO: Once this entire module is converted to Redux,
// `themeActivated` shouldn't be passed as an argument anymore,
// but directly imported and dispatch()ed from inside `activated()`,
// which needs to be turned into a Redux thunk.
activated: function( id, site, themeActivated ) {
ThemeHelper.trackClick( 'customizer', 'activate' );

page( '/design/' + site.slug );

ThemeActions.activated( id, site, 'customizer' );
themeActivated( id, site, 'customizer' );

Dispatcher.handleViewAction( {
type: 'ACTIVATED_THEME_WITH_CUSTOMIZER',
Expand Down
15 changes: 9 additions & 6 deletions client/my-sites/customize/controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/
var ReactDom = require( 'react-dom' ),
React = require( 'react' ),
ReduxProvider = require( 'react-redux' ).Provider,
Qs = require( 'qs' );

/**
Expand All @@ -26,12 +27,14 @@ module.exports = {
titleActions.setTitle( i18n.translate( 'Customizer', { textOnly: true } ), { siteID: siteID } );

ReactDom.render(
React.createElement( CustomizeComponent, {
domain: context.params.domain || '',
sites: sites,
prevPath: context.prevPath || '',
query: Qs.parse( context.querystring )
} ),
React.createElement( ReduxProvider, { store: context.store },
React.createElement( CustomizeComponent, {
domain: context.params.domain || '',
sites: sites,
prevPath: context.prevPath || '',
query: Qs.parse( context.querystring )
} )
),
document.getElementById( 'primary' )
);
}
Expand Down
17 changes: 13 additions & 4 deletions client/my-sites/customize/main.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ var React = require( 'react' ),
url = require( 'url' ),
Qs = require( 'qs' ),
cloneDeep = require( 'lodash/lang/cloneDeep' ),
bindActionCreators = require( 'redux' ).bindActionCreators,
connect = require( 'react-redux' ).connect,
debug = require( 'debug' )( 'calypso:my-sites:customize' );

/**
Expand All @@ -17,18 +19,20 @@ var notices = require( 'notices' ),
EmptyContent = require( 'components/empty-content' ),
SidebarNavigation = require( 'my-sites/sidebar-navigation' ),
museCustomizations = require( 'lib/customize/muse' ),
Actions = require( 'my-sites/customize/actions' );
Actions = require( 'my-sites/customize/actions' ),
themeActivated = require( 'lib/themes/actions' ).activated;

var mobileWidth = 400,
loadingTimer;

module.exports = React.createClass( {
var Customize = React.createClass( {
displayName: 'Customize',

propTypes: {
domain: React.PropTypes.string.isRequired,
sites: React.PropTypes.object.isRequired,
prevPath: React.PropTypes.string
prevPath: React.PropTypes.string,
themeActivated: React.PropTypes.func.isRequired
},

getDefaultProps: function() {
Expand Down Expand Up @@ -258,7 +262,7 @@ module.exports = React.createClass( {
break;
case 'activated':
themeSlug = message.theme.stylesheet.split( '/' )[1];
Actions.activated( themeSlug, site );
Actions.activated( themeSlug, site, this.props.themeActivated );
break;
case 'purchased':
themeSlug = message.theme.stylesheet.split( '/' )[1];
Expand Down Expand Up @@ -369,3 +373,8 @@ module.exports = React.createClass( {
} );
}
} );

export default connect(
( state, props ) => props,
bindActionCreators.bind( null, { themeActivated } )
)( Customize );
6 changes: 4 additions & 2 deletions client/my-sites/upgrades/controller.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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' ),
Expand Down Expand Up @@ -246,7 +246,9 @@ module.exports = {

if ( cartItems.hasOnlyProductsOf( cart, 'premium_theme' ) ) {
const { meta, extra: { source } } = cartAllItems[ 0 ];
ThemeActions.activated( meta, selectedSite, source, true );
// TODO: When this section is migrated to Redux altogether,
// use react-redux to `connect()` components and `dispatch()` actions.
context.store.dispatch( activated( meta, selectedSite, source, true ) );
page.redirect( '/design/' + selectedSite.slug );
return;
}
Expand Down
6 changes: 5 additions & 1 deletion client/state/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand All @@ -19,11 +21,13 @@ const reducer = combineReducers( {
sharing,
sites,
siteSettings,
themes,
ui
} );

export function createReduxStore() {
return applyMiddleware(
thunkMiddleware
thunkMiddleware,
analyticsMiddleware
)( createStore )( reducer );
};
Loading