From 1eca17e0a93166023a83d96a302172d6457b94e3 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Fri, 14 Sep 2007 17:29:08 +0000 Subject: [PATCH] elofgren: USABILITY: Rework the tabs in the Newsletter section to be less confusing as per issue #57 of the Usability Report Includes the removal of all tabs and submit action buttons when 'Drafts' or 'Sent Items' are clicked and the addition of an 'add one' link on 'Drafts' folder page. For more information see: http://www.elijahlofgren.com/silverstripe/newletter-tree-and-tab-row-are-confusing/ Includes a few fixes for undefined variables. (merged from branches/gsoc) git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/cms/trunk@41772 467b73ca-7a2a-4603-9d3b-597d59a354a9 --- code/Newsletter/Newsletter.php | 2 +- code/Newsletter/NewsletterList.php | 22 +--- code/NewsletterAdmin.php | 172 +++++++++++++++++++++++++--- javascript/NewsletterAdmin_left.js | 22 +--- javascript/NewsletterAdmin_right.js | 39 ++++++- templates/NewsletterList.ss | 25 +--- 6 files changed, 204 insertions(+), 78 deletions(-) diff --git a/code/Newsletter/Newsletter.php b/code/Newsletter/Newsletter.php index 533f59c4..2c8bd2ee 100755 --- a/code/Newsletter/Newsletter.php +++ b/code/Newsletter/Newsletter.php @@ -14,7 +14,7 @@ class Newsletter extends DataObject { $ret = new FieldSet( new TabSet("Root", - $mailTab = new Tab("Mail", + $mailTab = new Tab("Newsletter", new TextField("Subject", "Subject", $this->Subject), new HtmlEditorField("Content", "Content") ) diff --git a/code/Newsletter/NewsletterList.php b/code/Newsletter/NewsletterList.php index 46fcacd7..0d69b97a 100755 --- a/code/Newsletter/NewsletterList.php +++ b/code/Newsletter/NewsletterList.php @@ -20,10 +20,11 @@ class NewsletterList extends FormField { function setController($controller) { $this->controller = $controller; } - + /* No longer used function Newsletters() { return DataObject::get( 'Newsletter', "`ParentID`='{$this->mailType->ID}' AND `Status`='{$this->status}'" ); } + */ function DraftNewsletters() { return $this->mailType->DraftNewsletters(); @@ -36,22 +37,9 @@ class NewsletterList extends FormField { function Status() { return $this->status; } - - -/* function AddRecordForm() { - return new TabularStyle(new Form($this->controller,'AddRecordForm', - new FieldSet( - new TextField("FirstName", "First Name"), - new TextField("Surname", "Surname"), - new TextField("Email", "Email"), - new TextField("Password", "Password"), - new HiddenField("GroupID", null, $this->group->ID) - ), - new FieldSet( - new FormAction("addmember", "Add") - ) - )); - }*/ + function mailTypeID() { + return $this->mailType->ID; + } } ?> diff --git a/code/NewsletterAdmin.php b/code/NewsletterAdmin.php index b0364d50..72dd071a 100755 --- a/code/NewsletterAdmin.php +++ b/code/NewsletterAdmin.php @@ -87,15 +87,62 @@ class NewsletterAdmin extends LeftAndMain { $result = $this->renderWith($this->class . "_right"); return $this->getLastFormIn($result); } - - public function shownewsletter($params) { - return $this->showWithEditForm( $params, $this->getNewsletterEditForm( $params['ID'] ) ); + + /** + * Called when a mailing list is clicked on the left menu + */ + public function showrecipients($params) { + return $this->showWithEditForm( $params, $this->getMailingListEditForm( $params['ID'] ) ); + } + + /** + * Called when a draft or sent newsletter is clicked on the left menu + */ + public function shownewsletter($params) { + return $this->showWithEditForm( $params, $this->getNewsletterEditForm( $params['ID'] ) ); + } + + /** + * Called when a newsletter type is clicked on the left menu + */ + public function showmailtype($params) { + return $this->showWithEditForm( $params, $this->getNewsletterTypeEditForm( $params['ID'] ) ); + } + + /** + * Called when a 'Drafts' folder is clicked on the left menu + */ + public function showdrafts($params) { + return $this->ShowNewsletterFolder($params, 'Draft'); + } + + /** + * Called when a 'Sent Items' folder is clicked on the left menu + */ + public function showsent($params) { + return $this->ShowNewsletterFolder($params, 'Sent'); } - public function showtype($params) { - return $this->showWithEditForm( $params, $this->getNewsletterTypeEditForm( $params['ID'] ) ); + /** + * Shows either the 'Sent' or 'Drafts' folder using the NewsletterList template + */ + public function ShowNewsletterFolder($params, $type) { + $id = $params['ID']; + if(!is_numeric($id)) { + $id = Session::get('currentPage'); + } + if( is_a( $id, 'NewsletterType' ) ) { + $mailType = $id; + $id = $mailType->ID; + } else { + if($id && is_numeric($id)) { + $mailType = DataObject::get_by_id( 'NewsletterType', $id ); + } + } + $draftList = new NewsletterList($type, $mailType, $type); + return $draftList->renderWith("NewsletterList"); } - + public function removenewsletter($params) { if( !is_numeric( $params['ID'] ) ) return ''; @@ -131,7 +178,12 @@ class NewsletterAdmin extends LeftAndMain { if((isset($_REQUEST['ID']) && isset($_REQUEST['Type']) && $_REQUEST['Type'] == 'Newsletter') || isset($_REQUEST['action_savenewsletter'])) { return $this->NewsletterEditForm(); } else { - return $this->TypeEditForm(); + // If a mailing list member is being added to a group, then call the Recipient form + if (isset($_REQUEST['fieldName']) && 'Recipients' == $_REQUEST['fieldName']) { + return $this->MailingListEditForm(); + } else { + return $this->TypeEditForm(); + } } } @@ -150,6 +202,10 @@ class NewsletterAdmin extends LeftAndMain { } return $this->getNewsletterTypeEditForm($id); } + public function MailingListEditForm() { + $id = isset($_REQUEST['ID']) ? $_REQUEST['ID'] : $this->currentPageID(); + return $this->getMailingListEditForm($id); + } public function getNewsletterTypeEditForm($id) { if(!is_numeric($id)) { @@ -164,10 +220,22 @@ class NewsletterAdmin extends LeftAndMain { } } - if(isset($mailType) && $mailType) { - $fields = $mailType->getCMSFields(); + if(isset($mailType) && is_object($mailType) && $mailType->GroupID) { + $group = DataObject::get_one("Group", "ID = $mailType->GroupID"); + } - // $actions = new FieldSet(new FormAction('adddraft', 'Add Draft')); + if(isset($mailType)) { + $fields = new FieldSet( + new TextField("Title", "Newsletter Type"), + new TextField("FromEmail", "Send newsletters from"), + $templates = new TemplateList("Template","Template", $mailType->Template, self::template_path()) + ); + + $templates->setController($this); + + $fields->push($idField = new HiddenField("ID")); + $fields->push( new HiddenField( "executeForm", "", "TypeEditForm" ) ); + $idField->setValue($id); $actions = new FieldSet(new FormAction('save','Save')); @@ -183,6 +251,71 @@ class NewsletterAdmin extends LeftAndMain { return $form; } + public function getMailingListEditForm($id) { + if(!is_numeric($id)) { + $id = Session::get('currentPage'); + } + if( is_a( $id, 'NewsletterType' ) ) { + $mailType = $id; + $id = $mailType->ID; + } else { + if($id && is_numeric($id)) { + $mailType = DataObject::get_by_id( 'NewsletterType', $id ); + } + } + + if(isset($mailType) && is_object($mailType) && $mailType->GroupID) { + $group = DataObject::get_one("Group", "ID = $mailType->GroupID"); + } + + if(isset($mailType) && is_object($mailType)) { + $fields = new FieldSet( + new TabSet("Root", + new Tab( "Recipients", + $recipients = new MemberTableField( + $this, + "Recipients", + $group + ) + ), + new Tab( "Import", + $importField = new RecipientImportField("ImportFile","Import from file", $group ) + ), + new Tab("Unsubscribers", + $unsubscribedList = new UnsubscribedList("Unsubscribed", $mailType) + ), + new Tab("Bounced", + $bouncedList = new BouncedList("Bounced", $mailType ) + ) + ) + ); + + $recipients->setController($this); + $importField->setController($this); + $unsubscribedList->setController($this); + $bouncedList->setController($this); + + $importField->setTypeID( $id ); + + $fields->push($idField = new HiddenField("ID")); + $fields->push( new HiddenField( "executeForm", "", "MailingListEditForm" ) ); + $idField->setValue($id); + + $actions = new FieldSet(new FormAction('save','Save')); + + $form = new Form($this, "MailingListEditForm", $fields, $actions); + $form->loadDataFrom(array( + 'Title' => $mailType->Title, + 'FromEmail' => $mailType->FromEmail + )); + } else { + $form = false; + } + + return $form; + + } + /** * Reloads the list of recipients via ajax */ @@ -199,10 +332,12 @@ class NewsletterAdmin extends LeftAndMain { else return self::$template_path = project() . '/templates/email'; } - public function showdraft( $params ) { - return $this->showWithEditForm( $params, $this->getNewsletterEditForm( $params['ID'] ) ); - } - + /* Does not seem to be used + public function showdraft( $params ) { + return $this->showWithEditForm( $params, $this->getNewsletterEditForm( $params['ID'] ) ); + } + */ + public function getNewsletterEditForm($myId){ $email = DataObject::get_by_id("Newsletter", $myId); @@ -246,7 +381,7 @@ class NewsletterAdmin extends LeftAndMain { public function sendnewsletter( /*$data, $form = null*/ ) { - $id = $_REQUEST['ID'] ? $_REQUEST['ID'] : $_REQUEST['NewsletterID']; + $id = isset($_REQUEST['ID']) ? $_REQUEST['ID'] : $_REQUEST['NewsletterID']; if( !$id ) { FormResponse::status_message('No newsletter specified','bad'); @@ -307,6 +442,9 @@ class NewsletterAdmin extends LeftAndMain { if( isset($_REQUEST['Type']) && $_REQUEST['Type'] == 'Newsletter' ) return $this->savenewsletter( $urlParams, $form ); + // @TODO: Find the real fix for this. + $className = ''; + $id = $_REQUEST['ID']; $record = DataObject::get_one('NewsletterType', "`NewsletterType`.ID = $id"); @@ -325,7 +463,9 @@ class NewsletterAdmin extends LeftAndMain { public function savenewsletter($urlParams, $form) { $id = $_REQUEST['ID']; - $record = DataObject::get_one('Newsletter', "`Newsletter`.ID = $id"); + // @TODO: Find the real fix for this. + $className = ''; + $record = DataObject::get_one('Newsletter', "`$className`.ID = $id"); // Is the template attached to the type, or the newsletter itself? $type = $record->getNewsletterType(); diff --git a/javascript/NewsletterAdmin_left.js b/javascript/NewsletterAdmin_left.js index c4e85a2b..63535629 100755 --- a/javascript/NewsletterAdmin_left.js +++ b/javascript/NewsletterAdmin_left.js @@ -206,22 +206,7 @@ SiteTreeNode.prototype.getPageFromServer = function() { var otherID = null; var type = null; - var forceReload = $('Form_EditForm_Type') && $('Form_EditForm_Type').value == 'Newsletter'; - if( match ) { - - // open the appropriate tab - switch( match[1] ) { - case 'recipients': - openTabName = 'Root_Recipients_set_Recipients'; - break; - case 'drafts': - openTabName = 'Root_Drafts'; - break; - case 'sent': - openTabName = 'Root_Sent_set_Sent'; - break; - } newPageID = match[2]; type = match[1]; @@ -229,13 +214,8 @@ SiteTreeNode.prototype.getPageFromServer = function() { newPageID = RegExp.$2; type = RegExp.$1; otherID = RegExp.$3; - forceReload = true; } - - if( forceReload || currentPageID != newPageID ) - $('Form_EditForm').getPageFromServer(newPageID, type, otherID, openTabName); - else - openTab( openTabName ); + $('Form_EditForm').getPageFromServer(newPageID, type, otherID, openTabName); }; function draft_sent_ok( newsletterID, draftID ) { diff --git a/javascript/NewsletterAdmin_right.js b/javascript/NewsletterAdmin_right.js index 64a89dd3..4dc66edd 100755 --- a/javascript/NewsletterAdmin_right.js +++ b/javascript/NewsletterAdmin_right.js @@ -33,11 +33,11 @@ Behaviour.register({ if(treeNode) treeNode.addNodeClass('loading'); statusMessage("loading..."); - var requestURL = 'admin/newsletter/showtype/' + id; + var requestURL = 'admin/newsletter/show' + type + '/' + id; - if( otherid ) - requestURL = 'admin/newsletter/shownewsletter/' + otherid; - + if( otherid ) { + requestURL = 'admin/newsletter/shownewsletter/' + otherid; + } new Ajax.Request(requestURL, { asynchronous : true, method : 'post', @@ -47,6 +47,13 @@ Behaviour.register({ errorMessage('error loading page',response); } }); + // Hide the action buttons if 'Drafts' or 'Sent Items' is clicked on + if ('drafts' == type || 'sent' == type) + { + Element.hide('form_actions'); + Element.hide('form_actions_right'); + } + } else { throw("getPageFromServer: Bad page ID: " + id); } @@ -534,3 +541,27 @@ RecipientImportField.prototype = { } } }); + +/** + * Handle 'add one' link action. Adds a new draft to the site tree and loads it up to edit. + * Adapted from NewsletterAdmin_left.js + */ +function addNewDraft(parentID) { + var type = 'draft'; + var request = new Ajax.Request( 'admin/newsletter/addtype?ajax=1&PageType=' + type + '&ParentID=' + parentID, { + method: 'get', + asynchronous: true, + onSuccess : function( response ) { + $('Form_EditForm').loadNewPage(response.responseText); + + // create a new node and add it to the site tree + $('sitetree').addDraftNode('New draft newsletter', parentID, $('Form_EditForm_ID').value ); + + statusMessage('Added new ' + type); + }, + onFailure : function(response) { + alert(response.responseText); + statusMessage('Could not add new ' + type ); + } + }); +} diff --git a/templates/NewsletterList.ss b/templates/NewsletterList.ss index 2a282c66..ffa15ec0 100755 --- a/templates/NewsletterList.ss +++ b/templates/NewsletterList.ss @@ -1,19 +1,6 @@ - - - - - - - - - - <% control Newsletters %> - - - - - <% end_control %> - - - -
Content
$Subject$Content
+<% if Status = Draft %> +

Please choose a draft on the left, or add one.

+<% end_if %> +<% if Status = Sent %> +

Please choose a sent item on the left.

+<% end_if %>