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

CPT: Move PostTypeListPost to standalone block directory #7195

Merged
merged 7 commits into from
Aug 1, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions assets/stylesheets/_components.scss
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
@import 'layout/style';

@import 'auth/style';
@import 'blocks/post-item/style';
@import 'components/accordion/style';
@import 'components/app-promo/style';
@import 'blocks/author-selector/style';
Expand Down
27 changes: 27 additions & 0 deletions client/blocks/post-item/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
Post Item
=========

`<PostItem />` is a connected React component for rendering a post card, including its title, metadata, thumbnail, and actions for managing that post.

## Usage

Render the component, passing a global ID of the post to be rendered.

```jsx
function MyPostList() {
return <PostItem globalId="e532356fdb689509a1a5149072e8aafc" />;
}
```

The component does not render a [query component](https://github.com/Automattic/wp-calypso/blob/master/docs/our-approach-to-data.md#query-components), so it's assumed that the post will already have been loaded into global state prior to rendering the component.

## Props

### `globalId`

<table>
<tr><th>Type</th><td>Number</td></tr>
<tr><th>Required</th><td>Yes</td></tr>
</table>

The global ID of the post to be displayed.
46 changes: 46 additions & 0 deletions client/blocks/post-item/docs/example.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/**
* External dependencies
*/
import React from 'react';
import { connect } from 'react-redux';
import { get } from 'lodash';

/**
* Internal dependencies
*/
import QueryPosts from 'components/data/query-posts';
import QuerySites from 'components/data/query-sites';
import PostItem from '../';
import { getCurrentUser } from 'state/current-user/selectors';
import { getSitePosts } from 'state/posts/selectors';

function PostItemExample( { primarySiteId, globalId } ) {
return (
<div className="docs__design-assets-group">
<h2>
<a href="/devdocs/blocks/post-item">Post Item</a>
</h2>
{ primarySiteId && <QuerySites siteId={ primarySiteId } /> }
{ primarySiteId && (
<QueryPosts
siteId={ primarySiteId }
query={ { number: 1, type: 'any' } } />
) }
{ ! globalId && <em>No posts found</em> }
{ globalId && <PostItem globalId={ globalId } /> }
</div>
);
}

const ConnectedPostItemExample = connect( ( state ) => {
const primarySiteId = get( getCurrentUser( state ), 'primary_blog' );

return {
primarySiteId,
globalId: get( getSitePosts( state, primarySiteId ), [ 0, 'global_ID' ] )
};
} )( PostItemExample );

ConnectedPostItemExample.displayName = 'PostItem';

export default ConnectedPostItemExample;
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,25 @@ import { getEditorPath } from 'state/ui/editor/selectors';
import { getNormalizedPost } from 'state/posts/selectors';
import Card from 'components/card';
import PostRelativeTimeStatus from 'my-sites/post-relative-time-status';
import PostTypeListPostThumbnail from './post-thumbnail';
import PostActionsEllipsisMenu from './post-actions-ellipsis-menu';
import PostTypePostAuthor from './post-type-post-author';
import PostTypeListPostThumbnail from 'my-sites/post-type-list/post-thumbnail';
import PostActionsEllipsisMenu from 'my-sites/post-type-list/post-actions-ellipsis-menu';
import PostTypePostAuthor from 'my-sites/post-type-list/post-type-post-author';

export function PostTypeListPost( { translate, globalId, post, editUrl, className } ) {
const classes = classnames( 'post-type-list__post', className, {
export function PostItem( { translate, globalId, post, editUrl, className } ) {
const classes = classnames( 'post-item', className, {
'is-untitled': ! post.title
} );

return (
<Card compact className={ classes }>
<div className="post-type-list__post-detail">
<div className="post-type-list__post-title-meta">
<h1 className="post-type-list__post-title">
<a href={ editUrl }>
<div className="post-item__detail">
<div className="post-item__title-meta">
<h1 className="post-item__title">
<a href={ editUrl } className="post-item__title-link">
{ post.title || translate( 'Untitled' ) }
</a>
</h1>
<div className="post-type-list__post-meta">
<div className="post-item__meta">
<PostRelativeTimeStatus post={ post } />
<PostTypePostAuthor globalId={ globalId } />
</div>
Expand All @@ -43,7 +43,7 @@ export function PostTypeListPost( { translate, globalId, post, editUrl, classNam
);
}

PostTypeListPost.propTypes = {
PostItem.propTypes = {
translate: PropTypes.func,
globalId: PropTypes.string,
post: PropTypes.object,
Expand All @@ -55,6 +55,6 @@ export default connect( ( state, ownProps ) => {

return {
post,
editUrl: getEditorPath( state, state.ui.selectedSiteId, post.ID )
editUrl: getEditorPath( state, post.site_ID, post.ID )
};
} )( localize( PostTypeListPost ) );
} )( localize( PostItem ) );
63 changes: 63 additions & 0 deletions client/blocks/post-item/style.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
.card.post-item {
display: flex;
justify-content: space-between;
align-items: center;
}

.post-item__detail {
position: relative;
display: flex;
width: 100%;
overflow: hidden;
margin-right: auto;

&::after {
@include long-content-fade( $size: 40px );
right: 0;
}
}

.post-item__title-meta {
padding: 6px 0;
}

.post-item__title {
@extend %content-font;
margin-bottom: 2px;
font-weight: 700;
white-space: nowrap;
}

a.post-item__title-link {
color: $gray-dark;

&:hover {
color: darken( $gray, 20% );
}

.post-item.is-untitled & {
color: $gray;
font-style: italic;
}
}

.post-item__meta {
font-size: 12px;
color: lighten( $gray, 10% );
}

.post-item__meta .post-relative-time-status,
.post-item__meta .post-type-post-author {
float: left;
}

.post-item__meta .post-relative-time-status {
margin-bottom: 0;
}

.post-item__meta .post-relative-time-status .gridicon {
width: 14px;
height: 14px;
margin-top: -3px;
margin-right: 6px;
}
27 changes: 23 additions & 4 deletions client/components/data/query-sites/README.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,29 @@
Query Sites
===========================

`<QuerySites />` is a React component used in managing network requests for sites. When placed on the page
it requests all visible sites of the logged in user.
`<QuerySites />` is a React component used in managing network requests for sites. If passed a site ID, it will request the site when the component is mounted. If no site ID is passed, it will request all sites for the current user.

## Usage

Render the component. Any props passed to the component will be ignored. It does not accept any children,
nor does it render any elements to the page.
Render the component, optionally passing a site ID. The component does not accept any children, nor does it render any of its own.

```jsx
function AllSites() {
return <QuerySites />;
}

function SingleSite() {
return <QuerySites siteId={ 2916284 } />
}
```

## Props

### `siteId`

<table>
<tr><th>Type</th><td>Number</td></tr>
<tr><th>Required</th><td>No</td></tr>
</table>

An optional prop specifying a single site to be requested. If omitted, all sites for the current user will be requested.
43 changes: 28 additions & 15 deletions client/components/data/query-sites/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,31 @@
*/
import { Component, PropTypes } from 'react';
import { connect } from 'react-redux';
import { bindActionCreators } from 'redux';

/**
* Internal dependencies
*/
import { isRequestingSites } from 'state/sites/selectors';
import { requestSites } from 'state/sites/actions';
import { isRequestingSites, isRequestingSite } from 'state/sites/selectors';
import { requestSites, requestSite } from 'state/sites/actions';

class QuerySites extends Component {

componentWillMount() {
if ( ! this.props.requestingSites ) {
this.props.requestSites();
this.request( this.props );
}

componentWillReceiveProps( nextProps ) {
if ( nextProps.siteId !== this.props.siteId ) {
this.request( nextProps );
}
}

request( props ) {
if ( ! props.siteId && ! props.requestingSites ) {
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 to be careful about the case where we may need to wait until a siteId is available? We might accidentally trigger an all sites request if we pass through a falsy value before we pass through a siteId.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we need to be careful about the case where we may need to wait until a siteId is available? We might accidentally trigger an all sites request if we pass through a falsy value before we pass through a siteId.

Yes, but I think that should be on the developer to ensure that if they're intending to request a single site that they not render the <QuerySites /> component until the site ID is ready. That said, we could/should probably have better documentation about this behavior.

Copy link
Contributor

Choose a reason for hiding this comment

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

The sites request is heavy enough that I'd think it be worthwhile to try and avoid this case. I can see this being overlooked in a PR pretty easily.

Have we considered creating a QuerySites and QuerySite component?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have we considered creating a QuerySites and QuerySite component?

I know it had come up previously in the case of single post vs. all posts for a site, and IIRC there was a desire to keep it to a single flexible component that could accommodate both cases. I see benefits to both angles. Separate components not only avoids this as a mistake, but is also a simpler implementation for the individual components. Another alternative to consider is only doing the "mass" request if explicitly passing a null parameter for siteId or a separate allSites (or similarly named) prop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some context: #2350 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't hold up the PR, but I think splitting it into two keeps it simple and makes it easier to use.

Passing an explicit null parameter works too, but we'd need to communicate the convention. Currently, selectors often return null so this would be easy to accidentally trigger.

Copy link
Member

Choose a reason for hiding this comment

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

To me it's just an attribute difference of the same query — the same way I can say "query only posts for this author" I should be able to say "query all my latest drafts". Mentally I'm querying for the same item (posts) and I don't care how our API is setup, or whether I'm hitting different endpoints. Agreed on guarding against triggering unnecessary fetches, though. Perhaps allSites is a good prop to have.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough, and if we ever end up paging the results this makes sense too. I think an extra prop works better than giving extra meaning to null. Usage then looks like:

<QuerySites allSites />
<QuerySites siteId={ siteId } />

props.requestSites();
}

if ( props.siteId && ! props.requestingSite ) {
props.requestSite( props.siteId );
}
}

Expand All @@ -25,23 +37,24 @@ class QuerySites extends Component {
}

QuerySites.propTypes = {
siteId: PropTypes.number,
requestingSites: PropTypes.bool,
requestSites: PropTypes.func
requestingSite: PropTypes.bool,
requestSites: PropTypes.func,
requestSite: PropTypes.func
};

QuerySites.defaultProps = {
requestSites: () => {}
requestSites: () => {},
requestSite: () => {}
};

export default connect(
( state ) => {
( state, { siteId } ) => {
return {
requestingSites: isRequestingSites( state )
requestingSites: isRequestingSites( state ),
requestingSite: isRequestingSite( state, siteId )
};
},
( dispatch ) => {
return bindActionCreators( {
requestSites
}, dispatch );
}
{ requestSites, requestSite }
)( QuerySites );
2 changes: 2 additions & 0 deletions client/devdocs/design/blocks.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import PlanCompareCard from 'my-sites/plan-compare-card/docs/example';
import FeatureComparison from 'my-sites/feature-comparison/docs/example';
import DomainTip from 'my-sites/domain-tip/docs/example';
import PostCard from 'components/post-card/docs/example';
import PostItem from 'blocks/post-item/docs/example';
import ReaderAuthorLink from 'components/reader-author-link/docs/example';
import ReaderSiteStreamLink from 'components/reader-site-stream-link/docs/example';
import ReaderFullPostHeader from 'components/reader-full-post/docs/header-example';
Expand Down Expand Up @@ -82,6 +83,7 @@ export default React.createClass( {
<FeatureComparison />
<DomainTip />
<PostCard />
<PostItem />
<ReaderAuthorLink />
<ReaderSiteStreamLink />
<ReaderFullPostHeader />
Expand Down
4 changes: 2 additions & 2 deletions client/my-sites/post-type-list/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
getSitePostsForQueryIgnoringPage,
getSitePostsLastPageForQuery
} from 'state/posts/selectors';
import PostTypeListPost from './post';
import PostItem from 'blocks/post-item';
import PostTypeListPostPlaceholder from './post-placeholder';
import PostTypeListEmptyContent from './empty-content';

Expand Down Expand Up @@ -115,7 +115,7 @@ class PostTypeList extends Component {

renderPostRow( { index } ) {
const { global_ID: globalId } = this.props.posts[ index ];
return <PostTypeListPost key={ globalId } globalId={ globalId } />;
return <PostItem key={ globalId } globalId={ globalId } />;
}

render() {
Expand Down
4 changes: 2 additions & 2 deletions client/my-sites/post-type-list/post-placeholder.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { localize } from 'i18n-calypso';
/**
* Internal dependencies
*/
import { PostTypeListPost } from './post';
import { PostItem } from 'blocks/post-item';

function PostTypeListPostPlaceholder( { translate } ) {
const post = {
Expand All @@ -17,7 +17,7 @@ function PostTypeListPostPlaceholder( { translate } ) {
};

return (
<PostTypeListPost
<PostItem
post={ post }
className="post-type-list__post-placeholder" />
);
Expand Down
Loading