Skip to content

Commit

Permalink
Checklist: Unify WordPress.com checklist and banner (#26764)
Browse files Browse the repository at this point in the history 
This PR continues separation of state and declarative structure work outlined in #26216.

Inspired by suggestions from ockham, the Checklist is declared once in idiomatic React JSX, and can be rendered as a checklist of tasks or a banner (see `viewMode` prop).

* `ChecklistShow`, responsible only for rendering the WordPress.com checklist, has been removed.
* The array of task props in `onboardingChecklist` has been removed, and is now JSX in `WpcomChecklist`.
* `ChecklistBanner` has been removed and replaced by `<WpcomChecklist viewMode="banner" />`.
* `ChecklistMain` renders the `WpcomChecklist` and some surrounding UI. Clean up its usage of task status for calculating completion.
* Remove the `mergeObjectIntoArrayById` util which is no longer needed and related tests
  • Loading branch information
@sirreal @ockham
sirreal authored and ockham committed Sep 10, 2018
1 parent bf5fb0e commit 29af3b0
Show file tree
Hide file tree
Showing 13 changed files with 581 additions and 688 deletions.
2 changes: 1 addition & 1 deletion assets/stylesheets/_components.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@
@import 'my-sites/ads/style';
@import 'my-sites/all-sites-icon/style';
@import 'my-sites/checklist/style';
@import 'my-sites/checklist/wpcom-checklist/checklist-banner/style';
@import 'my-sites/current-site/style';
@import 'my-sites/current-site/sidebar-banner/style';
@import 'my-sites/customize/style';
Expand Down Expand Up @@ -447,7 +448,6 @@
@import 'my-sites/activity/activity-log-tasklist/style';
@import 'my-sites/stats/all-time/style';
@import 'my-sites/stats/annual-site-stats/style';
@import 'my-sites/stats/checklist-banner/style';
@import 'my-sites/stats/geochart/style';
@import 'my-sites/stats/most-popular/style';
@import 'my-sites/stats/post-performance/style';
Expand Down
65 changes: 41 additions & 24 deletions client/components/checklist/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,37 +16,54 @@ import TaskPlaceholder from 'components/checklist/task-placeholder';
export default class Checklist extends PureComponent {
static propTypes = {
isPlaceholder: PropTypes.bool,
updateCompletion: PropTypes.func,
};

state = { hideCompleted: false };

toggleCompleted = () =>
this.setState( ( { hideCompleted } ) => ( { hideCompleted: ! hideCompleted } ) );
componentDidMount() {
this.notifyCompletion();
}

renderPlaceholder() {
return (
<div className={ classNames( 'checklist', 'is-expanded', 'is-placeholder' ) }>
<ChecklistHeader total={ 0 } completed={ 0 } />
<div className="checklist__tasks">
{ times( Children.count( this.props.children ), index => (
<TaskPlaceholder key={ index } />
) ) }
</div>
</div>
);
componentDidUpdate() {
this.notifyCompletion();
}

render() {
if ( this.props.isPlaceholder ) {
return this.renderPlaceholder();
notifyCompletion() {
if ( 'function' === typeof this.props.updateCompletion ) {
const [ complete, total ] = this.calculateCompletion();
this.props.updateCompletion( { complete: complete >= total } );
}
}

calculateCompletion() {
const { children } = this.props;

const count = Children.map( children, child => child.props.completed ).reduce(
( acc, completed ) => ( true === completed ? acc + 1 : acc ),
const childrenArray = Children.toArray( children );
const completedCount = childrenArray.reduce(
( count, task ) => ( true === task.props.completed ? count + 1 : count ),
0
);
const total = childrenArray.length;
return [ completedCount, total ];
}

state = { hideCompleted: false };

toggleCompleted = () =>
this.setState( ( { hideCompleted } ) => ( { hideCompleted: ! hideCompleted } ) );

render() {
const [ completed, total ] = this.calculateCompletion();
if ( this.props.isPlaceholder ) {
return (
<div className={ classNames( 'checklist', 'is-expanded', 'is-placeholder' ) }>
<ChecklistHeader completed={ completed } total={ total } />
<div className="checklist__tasks">
{ times( total, index => (
<TaskPlaceholder key={ index } />
) ) }
</div>
</div>
);
}

return (
<div
Expand All @@ -56,12 +73,12 @@ export default class Checklist extends PureComponent {
} ) }
>
<ChecklistHeader
completed={ count }
completed={ completed }
hideCompleted={ this.state.hideCompleted }
onClick={ this.toggleCompleted }
total={ Children.count( children ) }
total={ total }
/>
<div className="checklist__tasks">{ children }</div>
<div className="checklist__tasks">{ this.props.children }</div>
</div>
);
}
Expand Down
113 changes: 0 additions & 113 deletions client/my-sites/checklist/checklist-show/index.jsx
View file

This file was deleted.

52 changes: 16 additions & 36 deletions client/my-sites/checklist/main.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,14 @@
* External dependencies
*/
import page from 'page';
import React, { Fragment, PureComponent } from 'react';
import React, { PureComponent } from 'react';
import { connect } from 'react-redux';
import { localize } from 'i18n-calypso';
import { some } from 'lodash';

/**
* Internal dependencies
*/
import ChecklistShow from './checklist-show';
import ChecklistShowShare from './share';
import DocumentHead from 'components/data/document-head';
import EmptyContent from 'components/empty-content';
Expand All @@ -22,21 +21,15 @@ import Main from 'components/main';
import PageViewTracker from 'lib/analytics/page-view-tracker';
import QuerySiteChecklist from 'components/data/query-site-checklist';
import SidebarNavigation from 'my-sites/sidebar-navigation';
import WpcomChecklist from './wpcom-checklist';
import { getCurrentUser } from 'state/current-user/selectors';
import { getSelectedSiteId } from 'state/ui/selectors';
import { getSiteSlug, isJetpackSite, isNewSite } from 'state/sites/selectors';
import { isEnabled } from 'config';

/**
* Included to fix regression.
* https://github.com/Automattic/wp-calypso/issues/26572
* @TODO clean up module separation.
*/
import getSiteChecklist from 'state/selectors/get-site-checklist';
import { mergeObjectIntoArrayById } from './util';
import { tasks as wpcomTasks } from './onboardingChecklist';

class ChecklistMain extends PureComponent {
state = { complete: false };

componentDidMount() {
this.maybeRedirectJetpack();
}
Expand All @@ -45,6 +38,8 @@ class ChecklistMain extends PureComponent {
this.maybeRedirectJetpack( prevProps );
}

handleCompletionUpdate = ( { complete } ) => void this.setState( { complete } );

/**
* Redirect Jetpack checklists to /plans/my-plan/:siteSlug
*
Expand Down Expand Up @@ -76,12 +71,12 @@ class ChecklistMain extends PureComponent {
}

renderHeader() {
const { displayMode, isNewlyCreatedSite, tasks, translate } = this.props;
const completed = tasks && ! some( tasks, { completed: false } );
const { displayMode, isNewlyCreatedSite, translate } = this.props;
const { complete } = this.state;

if ( completed ) {
if ( complete ) {
return (
<Fragment>
<>
<img
src="/calypso/images/signup/confetti.svg"
aria-hidden="true"
Expand All @@ -95,13 +90,13 @@ class ChecklistMain extends PureComponent {
) }
/>
<ChecklistShowShare className="checklist__share" siteSlug={ this.props.siteSlug } />
</Fragment>
</>
);
}

if ( isNewlyCreatedSite ) {
return (
<Fragment>
<>
<img
src="/calypso/images/signup/confetti.svg"
aria-hidden="true"
Expand Down Expand Up @@ -132,7 +127,7 @@ class ChecklistMain extends PureComponent {
)
}
/>
</Fragment>
</>
);
}

Expand Down Expand Up @@ -164,11 +159,11 @@ class ChecklistMain extends PureComponent {
<SidebarNavigation />
<DocumentHead title={ translatedTitle } />
{ checklistAvailable ? (
<Fragment>
<>
{ siteId && <QuerySiteChecklist siteId={ siteId } /> }
{ this.renderHeader() }
<ChecklistShow />
</Fragment>
<WpcomChecklist updateCompletion={ this.handleCompletionUpdate } />
</>
) : (
<EmptyContent title={ translate( 'Checklist not available for this site' ) } />
) }
Expand All @@ -182,14 +177,6 @@ export default connect( state => {
const isAtomic = isSiteAutomatedTransfer( state, siteId );
const isJetpack = isJetpackSite( state, siteId );

/**
* Included to fix regression.
* https://github.com/Automattic/wp-calypso/issues/26572
* @TODO clean up module separation.
*/
const siteChecklist = getSiteChecklist( state, siteId );
const tasksFromServer = siteChecklist && siteChecklist.tasks;

return {
checklistAvailable: ! isAtomic && ( isEnabled( 'jetpack/checklist' ) || ! isJetpack ),
isAtomic,
Expand All @@ -199,12 +186,5 @@ export default connect( state => {
siteId,
siteSlug: getSiteSlug( state, siteId ),
user: getCurrentUser( state ),

/**
* Included to fix regression.
* https://github.com/Automattic/wp-calypso/issues/26572
* @TODO clean up module separation.
*/
tasks: tasksFromServer ? mergeObjectIntoArrayById( wpcomTasks, tasksFromServer ) : null,
};
} )( localize( ChecklistMain ) );
Loading

0 comments on commit 29af3b0

Please sign in to comment.