-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Plugins (wpcom): Update design and text #4995
Changes from 5 commits
9e7c1d2
409af84
c77dc90
3cedf0d
70ba8c8
9879435
7c865ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,23 @@ | ||
import React, { PropTypes } from 'react'; | ||
import { connect } from 'react-redux'; | ||
import get from 'lodash/get'; | ||
import noop from 'lodash/noop'; | ||
|
||
import { recordTracksEvent } from 'state/analytics/actions'; | ||
|
||
import Gridicon from 'components/gridicon'; | ||
|
||
export const StandardPlugin = React.createClass( { | ||
const hasHttpProtocol = url => ( /^https?:\/\//.test( url ) ); | ||
|
||
export const Plugin = React.createClass( { | ||
getInitialState() { | ||
return { isUnderMouse: false }; | ||
}, | ||
|
||
startHover() { | ||
this.setState( { isUnderMouse: true } ); | ||
}, | ||
|
||
stopHover() { | ||
this.setState( { isUnderMouse: false } ); | ||
}, | ||
|
||
render() { | ||
const { | ||
category, | ||
|
@@ -18,11 +28,29 @@ export const StandardPlugin = React.createClass( { | |
descriptionLink | ||
} = this.props; | ||
|
||
const { isUnderMouse } = this.state; | ||
|
||
const isExternalLink = hasHttpProtocol( descriptionLink ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add this as a prop, instead of testing for this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @gwwar we deliberated on this point and I think it's best to keep the logic contained inside this component. it's only effect is visual and I don't think there's any need to couple that to the "outside world" I like being able to only care about adding the URL in the plugin data structure instead of having to think about how to represent that data. |
||
|
||
const target = isExternalLink | ||
? '_blank' | ||
: '_self'; | ||
|
||
const linkIcon = ( isExternalLink && isUnderMouse ) | ||
? 'external' | ||
: icon; | ||
|
||
return ( | ||
<div className="wpcom-plugins__plugin-item"> | ||
<a onClick={ onClick } href={ descriptionLink } target="_blank"> | ||
<a | ||
href={ descriptionLink } | ||
onClick={ onClick } | ||
onMouseEnter={ this.startHover } | ||
onMouseLeave={ this.stopHover } | ||
target={ target } | ||
> | ||
<div className="wpcom-plugins__plugin-icon"> | ||
<Gridicon { ...{ icon } } /> | ||
<Gridicon icon={ linkIcon } /> | ||
</div> | ||
<div className="wpcom-plugins__plugin-title">{ name }</div> | ||
<div className="wpcom-plugins__plugin-category">{ category }</div> | ||
|
@@ -33,7 +61,7 @@ export const StandardPlugin = React.createClass( { | |
} | ||
} ); | ||
|
||
StandardPlugin.propTypes = { | ||
Plugin.propTypes = { | ||
category: PropTypes.string.isRequired, | ||
description: PropTypes.string.isRequired, | ||
icon: PropTypes.string, | ||
|
@@ -42,16 +70,4 @@ StandardPlugin.propTypes = { | |
descriptionLink: PropTypes.string.isRequired | ||
}; | ||
|
||
const trackClick = name => recordTracksEvent( | ||
'calypso_plugin_wpcom_click', | ||
{ | ||
plugin_name: name, | ||
plugin_plan: 'standard' | ||
} | ||
); | ||
|
||
const mapDispatchToProps = ( dispatch, props ) => ( { | ||
onClick: get( props, 'onClick', () => dispatch( trackClick( props.name ) ) ) | ||
} ); | ||
|
||
export default connect( null, mapDispatchToProps )( StandardPlugin ); | ||
export default Plugin; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,19 @@ | ||
import React, { PropTypes } from 'react'; | ||
import { connect } from 'react-redux'; | ||
import classNames from 'classnames'; | ||
|
||
import Card from 'components/card'; | ||
import SectionHeader from 'components/section-header'; | ||
import PremiumPlugin from './plugin-types/premium-plugin'; | ||
import PurchaseButton from './purchase-button'; | ||
import { recordTracksEvent } from 'state/analytics/actions'; | ||
|
||
import Plugin from './plugin'; | ||
|
||
export const PremiumPluginsPanel = React.createClass( { | ||
render() { | ||
const { | ||
isActive = false, | ||
onClick, | ||
purchaseLink, | ||
plugins = [] | ||
} = this.props; | ||
|
@@ -26,9 +30,10 @@ export const PremiumPluginsPanel = React.createClass( { | |
|
||
<Card className={ cardClasses }> | ||
<div className="wpcom-plugins__list"> | ||
{ plugins.map( ( { name, descriptionLink, icon, plan, description } ) => | ||
<PremiumPlugin | ||
{ ...{ name, key: name, descriptionLink, icon, plan, description } } | ||
{ plugins.map( ( { name, descriptionLink, icon, category, description } ) => | ||
<Plugin | ||
onClick={ () => onClick( name ) } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can avoid a fat arrow here, if we move the default analytics tracking down a level to the Plugin component. The Plugin component will have name available as a prop. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was difficult. The plugin component is the same regardless of the plan level, so the analytics code needs to know whether it's for a standard, premium, or business plugin. I tried hard to eliminate the fat-arrow function but didn't find any good way to do so. Granted, I could eliminate it per se, but the underlying process (creating a new closure) was going to happen regardless. I chose to keep it here where it was obvious to someone reading the code. For example, I could have made Further, I didn't want to arbitrarily add props to the plugin piece itself or its Feel free to disagree and point out why you think this is wrong. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. :) The fat arrow warnings are non-blocking, but its just nice if we can avoid them. Another alternative is to let the plugin provide an interface, where the plugin prop values are provided, and we pass through a trackClick function. In Plugin:
|
||
{ ...{ name, key: name, descriptionLink, icon, category, description } } | ||
/> | ||
) } | ||
</div> | ||
|
@@ -44,4 +49,16 @@ PremiumPluginsPanel.propTypes = { | |
plugins: PropTypes.array | ||
}; | ||
|
||
export default PremiumPluginsPanel; | ||
const trackClick = name => recordTracksEvent( | ||
'calypso_plugin_wpcom_click', | ||
{ | ||
plugin_name: name, | ||
plugin_plan: 'premium' | ||
} | ||
); | ||
|
||
const mapDispatchToProps = dispatch => ( { | ||
onClick: name => dispatch( trackClick( name ) ) | ||
} ); | ||
|
||
export default connect( null, mapDispatchToProps )( PremiumPluginsPanel ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
categories?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question marks?
not sure I understand what you are commenting here @gwwar 😉
maybe this is a typo and it needs to remain
plans
- I did a find/replace so that's probably it