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

Theme Sheet: Use information from WordPress.org for themes on Jetpack sites #9875

Merged
merged 27 commits into from
Dec 12, 2016
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
de72945
lib/wporg: Add fetchThemeInformation() function
ockham Dec 7, 2016
f15e9b5
state/themes/actions: Enable fetching of theme information from wporg
ockham Dec 7, 2016
b4b6fcc
Theme Sheet: On JP sites, try querying theme information from wporg
ockham Dec 7, 2016
6205d1b
state/themes/utils: Add normalizeWporgTheme() util
ockham Dec 7, 2016
5d99f41
state/theme/actions: Normalize wporg themes
ockham Dec 7, 2016
8a13f1c
state/themes/selectors#getTheme(): On JP sites, merge WP.org data if …
ockham Dec 7, 2016
0c70ad7
Theme Sheet: Only show 'Open Live Demo' link when we have a Demo URL
ockham Dec 8, 2016
26ec67e
Themes: Use getThemeForumUrl() selector instead of getForumUrl() helper
ockham Dec 6, 2016
ef5a339
Theme Sheet: Hide Download button for non-wporg themes on HP sites
ockham Dec 8, 2016
52e4b1d
Theme Sheet: Hide related theme suggestions on Jetpack sites
ockham Dec 8, 2016
4000d43
state/themes/selectors: Add isWporgTheme() selector
ockham Dec 8, 2016
83517d6
state/themes/selectors#getThemeForumUrl: Have test cover WP.org theme…
ockham Dec 8, 2016
5a08543
Theme Sheet: Hide forum link for non-wporg themes on JP sites
ockham Dec 8, 2016
5a9185b
state/themes/selectors: Have getTheme use taxonomies from wporg (for …
ockham Dec 8, 2016
6c80ebc
Theme Sheet: Don't link feature tags on JP sites
ockham Dec 8, 2016
23f07df
state/themes/actions: Fix one requestTheme() test to really mimic the…
ockham Dec 8, 2016
f80aab5
state/themes/actions: Add tests for requestTheme() with a Jetpack site
ockham Dec 8, 2016
9a54916
state/themes/actions: Add tests for requestTheme() with the WP.org API
ockham Dec 8, 2016
4cd7918
state/test/utils: Add tags/theme_feature check to normalizeWporgTheme…
ockham Dec 8, 2016
157a342
QueryTheme: Accept 'wporg' as possible siteId propType
ockham Dec 8, 2016
2b0a677
state/themes/schema: Accept 'wporg' as possible siteId key
ockham Dec 8, 2016
443292e
state/themes/schema: Don't accept wpcom as possible activeThemes site…
ockham Dec 8, 2016
738baab
QueryActiveTheme: Don't allow 'wpcom' as a siteId prop
ockham Dec 8, 2016
8cbee7e
Theme Sheet: Hide Features Card if no features are found
ockham Dec 9, 2016
62b6788
state/themes/schema: stylesheet and demo_uri aren't mandatory for the…
ockham Dec 9, 2016
42429c5
state/themes/schema: author_uri isn't mandatory for WP.org themes
ockham Dec 9, 2016
1834607
state/themes/actions: Guard agains false return value from WP.org API
ockham Dec 9, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions client/components/data/query-active-theme/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@ import { isRequestingActiveTheme } from 'state/themes/selectors';

class QueryActiveTheme extends Component {
static propTypes = {
siteId: PropTypes.oneOfType( [
PropTypes.number,
PropTypes.oneOf( [ 'wpcom' ] )
] ).isRequired,
siteId: PropTypes.number.isRequired,
// Connected props
isRequesting: PropTypes.bool.isRequired,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A theme can only be active on an actual site, not on all of WP.com 😄

(Discovered while adding wporg to some other propTypes and schemas.)

requestActiveTheme: PropTypes.func.isRequired,
Expand Down
2 changes: 1 addition & 1 deletion client/components/data/query-theme/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class QueryTheme extends Component {
static propTypes = {
siteId: PropTypes.oneOfType( [
PropTypes.number,
PropTypes.oneOf( [ 'wpcom' ] )
PropTypes.oneOf( [ 'wpcom', 'wporg' ] )
] ).isRequired,
themeId: PropTypes.string.isRequired,
// Connected props
Expand Down
36 changes: 35 additions & 1 deletion client/lib/wporg/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,5 +95,39 @@ module.exports = {
.end( function( err, data ) {
callback( err, data.body );
} );
}
},
/**
* Get information about a given theme from the WordPress.org API.
* If provided with a callback, will call that on succes with an object with theme details.
* Otherwise, will return a promise.
*
* @param {string} themeId The theme identifier.
* @param {function} callback Callback that gets executed after the XHR returns the results.
* @returns {?Promise} Promise that is returned if no callback parameter is passed
*/
fetchThemeInformation: function( themeId, callback ) {
const url = 'https://api.wordpress.org/themes/info/1.1/';
const query = { action: 'theme_information', 'request[slug]': themeId };
// if callback is provided, behave traditionally
if ( 'function' === typeof callback ) {
return superagent
.get( url )
.set( 'Accept', 'application/json' )
.query( query )
.end( ( err, { body } ) => {
callback( err, body );
} );
}

// otherwise, return a Promise
return new Promise( ( resolve, reject ) => {
return superagent
.get( url )
.set( 'Accept', 'application/json' )
.query( query )
.end( ( err, { body } ) => {
err ? reject( err ) : resolve( body );
} );
} );
},
};
59 changes: 42 additions & 17 deletions client/my-sites/theme/main.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import React from 'react';
import { connect } from 'react-redux';
import i18n from 'i18n-calypso';
import titlecase from 'to-title-case';
import { isArray } from 'lodash';

/**
* Internal dependencies
Expand All @@ -28,7 +29,6 @@ import { getSelectedSite } from 'state/ui/selectors';
import { getSiteSlug, isJetpackSite } from 'state/sites/selectors';
import { getCurrentUserId } from 'state/current-user/selectors';
import { isUserPaid } from 'state/purchases/selectors';
import { getForumUrl } from 'my-sites/themes/helpers';
import { isPremiumTheme as isPremium } from 'state/themes/utils';
import ThanksModal from 'my-sites/themes/thanks-modal';
import QueryActiveTheme from 'components/data/query-active-theme';
Expand All @@ -37,7 +37,7 @@ import QueryUserPurchases from 'components/data/query-user-purchases';
import QuerySitePurchases from 'components/data/query-site-purchases';
import ThemesSiteSelectorModal from 'my-sites/themes/themes-site-selector-modal';
import { connectOptions } from 'my-sites/themes/theme-options';
import { isThemeActive, isThemePurchased, getThemeRequestErrors } from 'state/themes/selectors';
import { isThemeActive, isThemePurchased, getThemeRequestErrors, getThemeForumUrl } from 'state/themes/selectors';
import { getBackPath } from 'state/themes/themes-ui/selectors';
import EmptyContentComponent from 'components/empty-content';
import ThemePreview from 'my-sites/themes/theme-preview';
Expand Down Expand Up @@ -156,6 +156,17 @@ const ThemeSheet = React.createClass( {
return null;
},

renderPreviewButton() {
return (
<a className="theme__sheet-preview-link" onClick={ this.togglePreview } data-tip-target="theme-sheet-preview">
<Gridicon icon="themes" size={ 18 } />
<span className="theme__sheet-preview-link-text">
{ i18n.translate( 'Open Live Demo', { context: 'Individual theme live preview button' } ) }
</span>
</a>
);
},

renderScreenshot() {
let screenshot;
if ( this.props.isJetpack ) {
Expand All @@ -166,12 +177,7 @@ const ThemeSheet = React.createClass( {
const img = screenshot && <img className="theme__sheet-img" src={ screenshot + '?=w680' } />;
return (
<div className="theme__sheet-screenshot">
<a className="theme__sheet-preview-link" onClick={ this.togglePreview } data-tip-target="theme-sheet-preview">
<Gridicon icon="themes" size={ 18 } />
<span className="theme__sheet-preview-link-text">
{ i18n.translate( 'Open Live Demo', { context: 'Individual theme live preview button' } ) }
</span>
</a>
{ this.props.demo_uri && this.renderPreviewButton() }
{ img }
</div>
);
Expand Down Expand Up @@ -232,7 +238,7 @@ const ThemeSheet = React.createClass( {
</Card>
{ this.renderFeaturesCard() }
{ this.renderDownload() }
{ this.renderRelatedThemes() }
{ ! this.props.isJetpack && this.renderRelatedThemes() }
</div>
);
},
Expand Down Expand Up @@ -261,6 +267,10 @@ const ThemeSheet = React.createClass( {
},

renderThemeForumCard( isPrimary = false ) {
if ( ! this.props.forumUrl ) {
return null;
}

const description = isPremium( this.props )
? i18n.translate( 'Get in touch with the theme author' )
: i18n.translate( 'Get help from volunteers and staff' );
Expand All @@ -272,7 +282,7 @@ const ThemeSheet = React.createClass( {
{ i18n.translate( 'Have a question about this theme?' ) }
<small>{ description }</small>
</div>
<Button primary={ isPrimary } href={ getForumUrl( this.props ) }>Visit forum</Button>
<Button primary={ isPrimary } href={ this.props.forumUrl }>Visit forum</Button>
</Card>
);
},
Expand Down Expand Up @@ -310,16 +320,22 @@ const ThemeSheet = React.createClass( {
},

renderFeaturesCard() {
const { siteSlug, taxonomies } = this.props;
const themeFeatures = taxonomies && taxonomies.theme_feature instanceof Array
? taxonomies.theme_feature.map( function( item ) {
const { isJetpack, siteSlug, taxonomies } = this.props;
if ( ! taxonomies || ! isArray( taxonomies.theme_feature ) ) {
return null;
}

const themeFeatures = taxonomies.theme_feature.map( function( item ) {
const term = isValidTerm( item.slug ) ? item.slug : `feature:${ item.slug }`;
return (
<li key={ 'theme-features-item-' + item.slug }>
<a href={ `/design/filter/${ term }/${ siteSlug || '' }` }>{ item.name }</a>
{ isJetpack
? <a>{ item.name }</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious if adding the <a> is actually doing anything here. Are styles applied to it because its an anchor?
Maybe it could just be { item.name }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we're styling it. (I had removed it and noticed it didn't look the same.) I opted for keeping the element instead of tweaking the styling 😄

: <a href={ `/design/filter/${ term }/${ siteSlug || '' }` }>{ item.name }</a>
}
</li>
);
} ) : [];
} );

return (
<div>
Expand All @@ -334,7 +350,13 @@ const ThemeSheet = React.createClass( {
},

renderDownload() {
if ( isPremium( this.props ) ) {
// Don't render download button:
// * If it's a premium theme
// * If it's on a Jetpack site, and the theme object doesn't have a 'download' attr
// Note that not having a 'download' attr would be permissible for a theme on WPCOM
// since we don't provide any for some themes found on WordPress.org (notably the 'Twenties').
Copy link
Contributor

Choose a reason for hiding this comment

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

(notably the 'Twenties')

Think it is just Twenty Sixteen

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked a bunch of other Twenties to which the same seems to apply, but possibly not all of them.

// The <ThemeDownloadCard /> component can handle that case.
if ( isPremium( this.props ) || ( this.props.isJetpack && ! this.props.download ) ) {
return null;
}
return <ThemeDownloadCard theme={ this.props.id } href={ this.props.download } />;
Expand Down Expand Up @@ -426,7 +448,7 @@ const ThemeSheet = React.createClass( {
const analyticsPath = `/theme/:slug${ section ? '/' + section : '' }${ siteID ? '/:site_id' : '' }`;
const analyticsPageTitle = `Themes > Details Sheet${ section ? ' > ' + titlecase( section ) : '' }${ siteID ? ' > Site' : '' }`;

const { name: themeName, description, currentUserId, siteIdOrWpcom } = this.props;
const { name: themeName, description, currentUserId, isJetpack, siteIdOrWpcom } = this.props;
const title = themeName && i18n.translate( '%(themeName)s Theme', {
args: { themeName }
} );
Expand All @@ -451,6 +473,7 @@ const ThemeSheet = React.createClass( {
return (
<Main className="theme__sheet">
<QueryTheme themeId={ this.props.id } siteId={ siteIdOrWpcom } />
{ isJetpack && <QueryTheme themeId={ this.props.id } siteId="wporg" /> }
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need the two instances of <QueryTheme/>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the first one fetches the theme object from the JP site, and the second one fetches the supplementary one from the WP.org API.

{ currentUserId && <QueryUserPurchases userId={ currentUserId } /> }
{ siteID && <QuerySitePurchases siteId={ siteID } /> }
{ siteID && <QuerySitePlans siteId={ siteID } /> }
Expand Down Expand Up @@ -576,6 +599,7 @@ export default connect(
const isCurrentUserPaid = isUserPaid( state, currentUserId );
const theme = getTheme( state, siteIdOrWpcom, id );
const error = theme ? false : getThemeRequestErrors( state, id, siteIdOrWpcom );

return {
...theme,
id,
Expand All @@ -593,6 +617,7 @@ export default connect(
isThemePurchased( state, id, selectedSite.ID ) ||
hasFeature( state, selectedSite.ID, FEATURE_UNLIMITED_PREMIUM_THEMES )
),
forumUrl: selectedSite && getThemeForumUrl( state, id, selectedSite.ID )
};
}
)( ThemeSheetWithOptions );
1 change: 0 additions & 1 deletion client/my-sites/theme/test/main.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ describe( 'main', function() {
mockery.registerMock( 'lib/analytics', {} );
mockery.registerMock( 'my-sites/themes/helpers', {
isPremium: noop,
getForumUrl: noop,
getDetailsUrl: noop,
} );
mockery.registerSubstitute( 'matches-selector', 'component-matches-selector' );
Expand Down
12 changes: 2 additions & 10 deletions client/my-sites/themes/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,9 @@ import mapValues from 'lodash/mapValues';
*/
import config from 'config';
import { sectionify } from 'lib/route/path';
import { oldShowcaseUrl, isPremiumTheme as isPremium } from 'state/themes/utils';

export function getPreviewUrl( theme, site ) {
if ( site && site.jetpack ) {
return site.options.admin_url + 'customize.php?theme=' + theme.id + '&return=' + encodeURIComponent( window.location );
}
import { oldShowcaseUrl } from 'state/themes/utils';

export function getPreviewUrl( theme ) {
return `${ theme.demo_uri }?demo=true&iframe=true&theme_preview=true`;
}

Expand Down Expand Up @@ -68,10 +64,6 @@ export function getSupportUrl( theme, site ) {
return `${ oldShowcaseUrl }${ sitePart }${ theme.id }/support`;
}

export function getForumUrl( theme ) {
return isPremium( theme ) ? '//premium-themes.forums.wordpress.com/forum/' + theme.id : '//en.forums.wordpress.com/forum/themes';
}

export function getHelpUrl( theme, site ) {
if ( site && site.jetpack ) {
return getSupportUrl( theme, site );
Expand Down
6 changes: 4 additions & 2 deletions client/my-sites/themes/thanks-modal.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@ import { translate } from 'i18n-calypso';
*/
import Dialog from 'components/dialog';
import PulsingDot from 'components/pulsing-dot';
import { getForumUrl, trackClick } from './helpers';
import { trackClick } from './helpers';
import { isJetpackSite } from 'state/sites/selectors';
import {
getActiveTheme,
getTheme,
getThemeDetailsUrl,
getThemeCustomizeUrl,
getThemeForumUrl,
isActivatingTheme,
hasActivatedTheme
} from 'state/themes/selectors';
Expand Down Expand Up @@ -82,7 +83,7 @@ const ThanksModal = React.createClass( {
<li>
{ translate( 'Have questions? Stop by our {{a}}support forums.{{/a}}', {
components: {
a: <a href={ getForumUrl( this.props.currentTheme ) }
a: <a href={ this.props.forumUrl }
onClick={ this.onLinkClick( 'support' ) } />
}
} ) }
Expand Down Expand Up @@ -209,6 +210,7 @@ export default connect(
currentTheme,
detailsUrl: site && getThemeDetailsUrl( state, currentTheme, site.ID ),
customizeUrl: site && getThemeCustomizeUrl( state, currentTheme, site.ID ),
forumUrl: getThemeForumUrl( state, currentThemeId ),
isActivating: !! ( site && isActivatingTheme( state, site.ID ) ),
hasActivated: !! ( site && hasActivatedTheme( state, site.ID ) )
};
Expand Down
26 changes: 25 additions & 1 deletion client/state/themes/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import debugFactory from 'debug';
* Internal dependencies
*/
import wpcom from 'lib/wp';
import wporg from 'lib/wporg';
import {
// Old action names
THEME_BACK_PATH_SET,
Expand Down Expand Up @@ -43,7 +44,12 @@ import {
import { isJetpackSite } from 'state/sites/selectors';
import { getActiveTheme } from './selectors';
import { getQueryParams } from './themes-list/selectors';
import { getThemeIdFromStylesheet, filterThemesForJetpack, normalizeWpcomTheme } from './utils';
import {
getThemeIdFromStylesheet,
filterThemesForJetpack,
normalizeWpcomTheme,
normalizeWporgTheme
} from './utils';

const debug = debugFactory( 'calypso:themes:actions' ); //eslint-disable-line no-unused-vars

Expand Down Expand Up @@ -257,6 +263,24 @@ export function requestTheme( themeId, siteId ) {
themeId
} );

if ( siteId === 'wporg' ) {
return wporg.fetchThemeInformation( themeId ).then( ( theme ) => {
dispatch( receiveTheme( normalizeWporgTheme( theme ), siteId ) );
dispatch( {
type: THEME_REQUEST_SUCCESS,
siteId,
themeId
} );
} ).catch( ( error ) => {
dispatch( {
type: THEME_REQUEST_FAILURE,
siteId,
themeId,
error
} );
} );
}

if ( siteId === 'wpcom' ) {
return wpcom.undocumented().themeDetails( themeId ).then( ( theme ) => {
dispatch( receiveTheme( normalizeWpcomTheme( theme ), siteId ) );
Expand Down
4 changes: 2 additions & 2 deletions client/state/themes/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const queriesSchema = {
type: 'object',
patternProperties: {
// Site ID
'^(wpcom|\\d+)$': {
'^(wpcom|wporg|\\d+)$': {
type: 'object',
properties: {
data: {
Expand Down Expand Up @@ -80,7 +80,7 @@ export const queriesSchema = {
export const activeThemesSchema = {
type: 'object',
patternProperties: {
'^(wpcom|\\d+)$': {
'^\\d+$': {
description: 'Theme ID',
type: 'string'
}
Expand Down
Loading