-
Notifications
You must be signed in to change notification settings - Fork 16
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
FIX Create campaign success message #37
Changes from 5 commits
ec321c7
245b9ad
f13a96c
7ef933b
82ec3f4
65e66c2
4162b88
12b80f0
4a8b2c0
32edfa0
ccdda08
1d4e524
e223da7
2828216
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,8 +14,7 @@ import CampaignAdminItem from './CampaignAdminItem'; | |
import Breadcrumb from 'components/Breadcrumb/Breadcrumb'; | ||
import Preview from 'components/Preview/Preview'; | ||
import i18n from 'i18n'; | ||
|
||
const sectionConfigKey = 'SilverStripe\\CMS\\Controllers\\CMSPagesController'; | ||
import classnames from 'classnames'; | ||
|
||
/** | ||
* Represents a campaign list view | ||
|
@@ -29,6 +28,16 @@ class CampaignAdminList extends SilverStripeComponent { | |
this.handleItemSelected = this.handleItemSelected.bind(this); | ||
this.setBreadcrumbs = this.setBreadcrumbs.bind(this); | ||
this.handleCloseItem = this.handleCloseItem.bind(this); | ||
|
||
if (!this.isRecordLoaded()) { | ||
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. since this is in the constructor, would be a good idea to pass through props to |
||
this.state = { | ||
loading: true, | ||
}; | ||
} else { | ||
this.state = { | ||
loading: false, | ||
}; | ||
} | ||
} | ||
|
||
componentDidMount() { | ||
|
@@ -37,13 +46,28 @@ class CampaignAdminList extends SilverStripeComponent { | |
this.setBreadcrumbs(); | ||
|
||
// Only load record if not already present | ||
if (!Object.keys(this.props.record).length) { | ||
if (!this.isRecordLoaded()) { | ||
this.props.recordActions | ||
.fetchRecord(this.props.treeClass, 'get', fetchURL) | ||
.then(this.setBreadcrumbs); | ||
.then(() => { | ||
this.setBreadcrumbs(); | ||
this.setState({ loading: false }); | ||
}); | ||
} | ||
} | ||
|
||
componentWillUnmount() { | ||
// Reset new create flag | ||
this.props.campaignActions.setNewCampaignCreated(false); | ||
} | ||
|
||
/** | ||
* @return {boolean} | ||
*/ | ||
isRecordLoaded() { | ||
return Object.keys(this.props.record).length !== 0; | ||
} | ||
|
||
/** | ||
* Update breadcrumbs for this view | ||
*/ | ||
|
@@ -94,6 +118,7 @@ class CampaignAdminList extends SilverStripeComponent { | |
const selectedClass = (!itemId) ? 'campaign-admin__campaign--hide-preview' : ''; | ||
const campaignId = this.props.campaignId; | ||
const campaign = this.props.record; | ||
const newItemCreated = this.props.newItemCreated; | ||
|
||
// Trigger different layout when preview is enabled | ||
const itemGroups = this.groupItemsForSet(); | ||
|
@@ -113,9 +138,12 @@ class CampaignAdminList extends SilverStripeComponent { | |
const group = itemGroups[className]; | ||
const groupCount = group.items.length; | ||
|
||
let listGroupItems = []; | ||
let title = `${groupCount} ${groupCount === 1 ? group.singular : group.plural}`; | ||
let groupid = `Set_${campaignId}_Group_${className}`; | ||
const listGroupItems = []; | ||
const title = ` | ||
${groupCount === 0 ? '' : groupCount} | ||
${groupCount === 1 ? group.singular : group.plural} | ||
`; | ||
const groupid = `Set_${campaignId}_Group_${className}`; | ||
|
||
// Create items for this group | ||
group.items.forEach(item => { | ||
|
@@ -163,29 +191,31 @@ class CampaignAdminList extends SilverStripeComponent { | |
); | ||
}); | ||
|
||
const wrapperClassnames = classnames('list-group-wrapper', { | ||
'list-group-wrapper--empty': listGroupItems.length === 0, | ||
}); | ||
|
||
// Merge into group | ||
accordionBlocks.push( | ||
<AccordionBlock key={groupid} groupid={groupid} title={title}> | ||
{listGroupItems} | ||
</AccordionBlock> | ||
<div className={wrapperClassnames}> | ||
<AccordionBlock key={groupid} groupid={groupid} title={title}> | ||
{ | ||
listGroupItems.length > 0 ? | ||
listGroupItems : | ||
<p className="list-group-item">{group.noItemText}</p> | ||
} | ||
</AccordionBlock> | ||
</div> | ||
); | ||
}); | ||
|
||
// Set body | ||
const pagesLink = [ | ||
this.props.config.absoluteBaseUrl, | ||
this.props.config.sections.find((section) => section.name === sectionConfigKey).url, | ||
].join(''); | ||
|
||
const body = accordionBlocks.length | ||
? (<Accordion>{accordionBlocks}</Accordion>) | ||
: ( | ||
<div className="alert alert-warning" role="alert"> | ||
<strong>This campaign is empty.</strong> You can add items to a campaign by | ||
selecting <em>Add to campaign</em> from within the <em>More Options </em> | ||
popup on <a href={pagesLink}>pages</a> and files. | ||
</div> | ||
); | ||
const newItemInfo = newItemCreated ? | ||
(<p className="alert alert-success alert--no-border" role="alert"> | ||
Nice one! You have successfully created a campaign. | ||
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. i18n this please. |
||
</p>) : | ||
null; | ||
|
||
const body = <Accordion>{accordionBlocks}</Accordion>; | ||
const bodyClass = [ | ||
'panel', 'panel--padded', 'panel--scrollable', 'flexbox-area-grow', | ||
]; | ||
|
@@ -196,18 +226,57 @@ class CampaignAdminList extends SilverStripeComponent { | |
<Toolbar showBackButton handleBackButtonClick={this.props.handleBackButtonClick}> | ||
<Breadcrumb multiline /> | ||
</Toolbar> | ||
{newItemInfo} | ||
<div className={bodyClass.join(' ')}> | ||
{body} | ||
</div> | ||
<div className="toolbar toolbar--south"> | ||
{this.renderButtonToolbar()} | ||
</div> | ||
</div> | ||
<Preview itemLinks={itemLinks} itemId={itemId} onBack={this.handleCloseItem} /> | ||
{this.renderPreview(itemLinks, itemId)} | ||
</div> | ||
); | ||
} | ||
|
||
renderPreview(itemLinks, itemId) { | ||
let preview = null; | ||
|
||
if (this.state.loading) { | ||
preview = ( | ||
<div | ||
className="flexbox-area-grow | ||
fill-height | ||
preview | ||
campaign-admin__campaign-preview | ||
campaign-admin__campaign-preview--empty" | ||
> | ||
<p>Loading...</p> | ||
</div> | ||
); | ||
} else if (!this.getItems() || this.getItems().length === 0) { | ||
preview = ( | ||
<div | ||
className="flexbox-area-grow | ||
fill-height | ||
preview | ||
campaign-admin__campaign-preview | ||
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. shouldn't have new lines in double quoted strings... |
||
campaign-admin__campaign-preview--empty" | ||
> | ||
<h2 className="campaign-admin__empty-heading">Getting started</h2> | ||
<p className="campaign-admin__empty-info"> | ||
Select <strong>Add to Campaign</strong> | ||
from pages, files, and other content types | ||
</p> | ||
</div> | ||
); | ||
} else { | ||
preview = <Preview itemLinks={itemLinks} itemId={itemId} onBack={this.handleCloseItem} />; | ||
} | ||
|
||
return preview; | ||
} | ||
|
||
/** | ||
* Callback for items being clicked on | ||
* | ||
|
@@ -225,11 +294,6 @@ class CampaignAdminList extends SilverStripeComponent { | |
renderButtonToolbar() { | ||
const items = this.getItems(); | ||
|
||
// let itemSummaryLabel; | ||
if (!items || !items.length) { | ||
return <div className="btn-toolbar"></div>; | ||
} | ||
|
||
// let itemSummaryLabel = i18n.sprintf( | ||
// items.length === 1 | ||
// ? i18n._t('CampaignAdmin.ITEM_SUMMARY_SINGULAR') | ||
|
@@ -239,7 +303,14 @@ class CampaignAdminList extends SilverStripeComponent { | |
|
||
let actionProps = {}; | ||
|
||
if (this.props.record.State === 'open') { | ||
if (!items || items.length === 0) { | ||
actionProps = Object.assign(actionProps, { | ||
title: i18n._t('CampaignAdmin.PUBLISHCAMPAIGN'), | ||
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. add translation default fallback |
||
buttonStyle: 'secondary-outline', | ||
icon: 'rocket', | ||
disabled: true, | ||
}); | ||
} else if (this.props.record.State === 'open') { | ||
actionProps = Object.assign(actionProps, { | ||
title: i18n._t('CampaignAdmin.PUBLISHCAMPAIGN'), | ||
buttonStyle: 'primary', | ||
|
@@ -289,7 +360,7 @@ class CampaignAdminList extends SilverStripeComponent { | |
* @return {object} | ||
*/ | ||
groupItemsForSet() { | ||
const groups = {}; | ||
const groups = this.getPlaceholderGroups(); | ||
const items = this.getItems(); | ||
if (!items) { | ||
return groups; | ||
|
@@ -315,6 +386,19 @@ class CampaignAdminList extends SilverStripeComponent { | |
return groups; | ||
} | ||
|
||
getPlaceholderGroups() { | ||
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. I'm not sure why we need this api; doesn't groupItemsForSet() build the groups automatically? 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. ok, @phalkunz has explained why this is necessary. The only change I'd request is the switch to using the existing config for important_classes instead. |
||
const groups = {}; | ||
|
||
if (this.props.record && this.props.record.placeholderGroups) { | ||
this.props.record.placeholderGroups.forEach((group) => { | ||
groups[group.baseClass] = { ...group }; | ||
groups[group.baseClass].items = [...group.items]; | ||
}); | ||
} | ||
|
||
return groups; | ||
} | ||
|
||
handlePublish(e) { | ||
e.preventDefault(); | ||
this.props.campaignActions.publishCampaign( | ||
|
@@ -352,6 +436,7 @@ function mapStateToProps(state, ownProps) { | |
record: record || {}, | ||
campaign: state.campaign, | ||
treeClass, | ||
newItemCreated: state.campaign.newItemCreated, | ||
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. proptypes definition for this |
||
}; | ||
} | ||
|
||
|
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.
Since the
id
is available, it would be more extensible to include the record or even just the id in the record, rather than just a boolean.In this case, you could ignore the
is
orhas
prefix I mentioned below. But try to keep the naming consistent stillsetNewItem
is perfectly fine.