From 473eda43cb24a66964d932a4b29376326b5b5587 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Wed, 18 Apr 2012 21:15:31 +0200 Subject: [PATCH 1/8] ENHANCEMENT Allowing multiple fragments to be returned by PjaxResponseNegotiator through JSON --- control/PjaxResponseNegotiator.php | 31 +++++++++++++++----- tests/control/PjaxResponseNegotiatorTest.php | 16 ++++++++++ 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/control/PjaxResponseNegotiator.php b/control/PjaxResponseNegotiator.php index b4f0fb8a2..06f598e5b 100644 --- a/control/PjaxResponseNegotiator.php +++ b/control/PjaxResponseNegotiator.php @@ -44,19 +44,36 @@ class PjaxResponseNegotiator { array_change_key_case($this->callbacks, CASE_LOWER), array_change_key_case($extraCallbacks, CASE_LOWER) ); + + $response = new SS_HTTPResponse(); - if($fragment = $request->getHeader('X-Pjax')) { - $fragment = strtolower($fragment); - if(isset($callbacks[$fragment])) { - return call_user_func($callbacks[$fragment]); + $responseParts = array(); + if($fragmentStr = $request->getHeader('X-Pjax')) { + $fragments = explode(',', strtolower($fragmentStr)); + foreach($fragments as $fragment) { + if(isset($callbacks[$fragment])) { + $responseParts[$fragment] = call_user_func($callbacks[$fragment]); + } else { + throw new SS_HTTPResponse_Exception("X-Pjax = '$fragment' not supported for this URL.", 400); + } + } + if(count($responseParts) == 1) { + $response->setBody(array_pop($responseParts)); } else { - throw new SS_HTTPResponse_Exception("X-Pjax = '$fragment' not supported for this URL.", 400); + if($request->getHeader('Accept') != 'text/json') { + throw new SS_HTTPResponse_Exception( + 'Multiple comma-separated fragments can only be returne with an "Accept: text/json" header', + 400 + ); + } + $response->setBody(Convert::raw2json($responseParts)); + $response->addHeader('Content-Type', 'text/json'); } } else { if($request->isAjax()) throw new SS_HTTPResponse_Exception("Ajax requests to this URL require an X-Pjax header.", 400); - return call_user_func($callbacks['default']); + $response->setBody(call_user_func($callbacks['default'])); } - + return $response; } /** diff --git a/tests/control/PjaxResponseNegotiatorTest.php b/tests/control/PjaxResponseNegotiatorTest.php index 1b44e09b2..32633b223 100644 --- a/tests/control/PjaxResponseNegotiatorTest.php +++ b/tests/control/PjaxResponseNegotiatorTest.php @@ -19,4 +19,20 @@ class PjaxResponseNegotiatorTest extends SapphireTest { $this->assertEquals('myfragment response', $negotiator->respond($request)); } + function testMultipleFragments() { + $negotiator = new PjaxResponseNegotiator(array( + 'default' => function() {return 'default response';}, + 'myfragment' => function() {return 'myfragment response';}, + 'otherfragment' => function() {return 'otherfragment response';}, + )); + $request = new SS_HTTPRequest('GET', '/'); + $request->addHeader('X-Pjax', 'myfragment,otherfragment'); + $request->addHeader('Accept', 'text/json'); + $json = json_decode($negotiator->respond($request)); + $this->assertObjectHasAttribute('myfragment', $json); + $this->assertEquals('myfragment response', $json->myfragment); + $this->assertObjectHasAttribute('otherfragment', $json); + $this->assertEquals('otherfragment response', $json->otherfragment); + } + } From 5178954311419b203a1ca4857b74c2665380eb75 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Wed, 18 Apr 2012 22:11:40 +0200 Subject: [PATCH 2/8] UNFINISHED Processing multiple PJAX responses on CMS JavaScript, introducing data-pjax-fragment attribute to identify reloadable template parts --- admin/code/LeftAndMain.php | 6 +- admin/code/ModelAdmin.php | 1 + admin/code/SecurityAdmin.php | 1 + admin/javascript/LeftAndMain.Content.js | 3 +- admin/javascript/LeftAndMain.js | 118 +++++++++--------- admin/templates/CMSBreadcrumbs.ss | 2 +- .../templates/Includes/LeftAndMain_Content.ss | 2 +- .../templates/Includes/ModelAdmin_Content.ss | 2 +- docs/en/reference/cms-architecture.md | 75 +++++++++-- forms/gridfield/GridFieldDetailForm.php | 1 + forms/gridfield/GridFieldLevelup.php | 2 +- 11 files changed, 142 insertions(+), 71 deletions(-) diff --git a/admin/code/LeftAndMain.php b/admin/code/LeftAndMain.php index 8ace852d0..f08b1394e 100644 --- a/admin/code/LeftAndMain.php +++ b/admin/code/LeftAndMain.php @@ -352,7 +352,6 @@ class LeftAndMain extends Controller implements PermissionProvider { if($this->request->isAjax()) { $this->response->addHeader('X-ControllerURL', $url); if($header = $this->request->getHeader('X-Pjax')) $this->response->addHeader('X-Pjax', $header); - if($header = $this->request->getHeader('X-Pjax-Selector')) $this->response->addHeader('X-Pjax-Selector', $header); return ''; // Actual response will be re-requested by client } else { parent::redirect($url, $code); @@ -437,6 +436,9 @@ class LeftAndMain extends Controller implements PermissionProvider { 'Content' => function() use(&$controller) { return $controller->renderWith($controller->getTemplatesWithSuffix('_Content')); }, + 'Breadcrumbs' => function() use (&$controller) { + return $controller->renderWith('CMSBreadcrumbs'); + }, 'default' => function() use(&$controller) { return $controller->renderWith($controller->getViewer('show')); } @@ -951,6 +953,7 @@ class LeftAndMain extends Controller implements PermissionProvider { $form->addExtraClass('cms-edit-form'); $form->loadDataFrom($record); $form->setTemplate($this->getTemplatesWithSuffix('_EditForm')); + $form->setAttribute('data-pjax-fragment', 'CurrentForm'); // Set this if you want to split up tabs into a separate header row // if($form->Fields()->hasTabset()) $form->Fields()->findOrMakeTab('Root')->setTemplate('CMSTabSet'); @@ -1015,6 +1018,7 @@ class LeftAndMain extends Controller implements PermissionProvider { $form->addExtraClass('cms-edit-form'); $form->addExtraClass('root-form'); $form->setTemplate($this->getTemplatesWithSuffix('_EditForm')); + $form->setAttribute('data-pjax-fragment', 'CurrentForm'); return $form; } diff --git a/admin/code/ModelAdmin.php b/admin/code/ModelAdmin.php index 0c8e4c370..0696324f7 100644 --- a/admin/code/ModelAdmin.php +++ b/admin/code/ModelAdmin.php @@ -151,6 +151,7 @@ abstract class ModelAdmin extends LeftAndMain { $form->addExtraClass('cms-edit-form cms-panel-padded center'); $form->setTemplate($this->getTemplatesWithSuffix('_EditForm')); $form->setFormAction(Controller::join_links($this->Link($this->modelClass), 'EditForm')); + $form->setAttribute('data-pjax-fragment', 'CurrentForm'); $this->extend('updateEditForm', $form); diff --git a/admin/code/SecurityAdmin.php b/admin/code/SecurityAdmin.php index 7ad486ffd..0766c2ada 100755 --- a/admin/code/SecurityAdmin.php +++ b/admin/code/SecurityAdmin.php @@ -160,6 +160,7 @@ class SecurityAdmin extends LeftAndMain implements PermissionProvider { $form->setTemplate($this->getTemplatesWithSuffix('_EditForm')); if($form->Fields()->hasTabset()) $form->Fields()->findOrMakeTab('Root')->setTemplate('CMSTabSet'); $form->addExtraClass('center ss-tabset cms-tabset ' . $this->BaseCSSClasses()); + $form->setAttribute('data-pjax-fragment', 'CurrentForm'); $this->extend('updateEditForm', $form); diff --git a/admin/javascript/LeftAndMain.Content.js b/admin/javascript/LeftAndMain.Content.js index 96ef06ead..2f69663ad 100644 --- a/admin/javascript/LeftAndMain.Content.js +++ b/admin/javascript/LeftAndMain.Content.js @@ -86,8 +86,7 @@ // sending back different `X-Pjax` headers and content jQuery.ajax(jQuery.extend({ headers: { - "X-Pjax" : "CurrentForm", - 'X-Pjax-Selector': '.cms-edit-form' + "X-Pjax" : "CurrentForm,Breadcrumbs" }, url: form.attr('action'), data: formData, diff --git a/admin/javascript/LeftAndMain.js b/admin/javascript/LeftAndMain.js index 8e36d467b..8fec12c78 100644 --- a/admin/javascript/LeftAndMain.js +++ b/admin/javascript/LeftAndMain.js @@ -40,10 +40,7 @@ jQuery.noConflict(); // Normalize trailing slashes in URL to work around routing weirdnesses in SS_HTTPRequest. var isSame = (url && History.getPageUrl().replace(/\/+$/, '') == url.replace(/\/+$/, '')); if(url && !isSame) { - opts = { - pjax: xhr.getResponseHeader('X-Pjax') ? xhr.getResponseHeader('X-Pjax') : settings.headers['X-Pjax'], - selector: xhr.getResponseHeader('X-Pjax-Selector') ? xhr.getResponseHeader('X-Pjax-Selector') : settings.headers['X-Pjax-Selector'] - }; + opts = {pjax: xhr.getResponseHeader('X-Pjax') ? xhr.getResponseHeader('X-Pjax') : settings.headers['X-Pjax']}; window.History.pushState(opts, '', url); } } @@ -205,30 +202,24 @@ jQuery.noConflict(); * if the URL is loaded without ajax. */ handleStateChange: function() { - var self = this, h = window.History, state = h.getState(); - // Don't allow parallel loading to avoid edge cases if(this.getCurrentXHR()) this.getCurrentXHR().abort(); - - var selector = state.data.selector || '.cms-content', contentEl = $(selector); + + var self = this, h = window.History, state = h.getState(), + fragments = state.data.pjax || 'Content', headers = {}, + reduceFn = function(fragment) {return '[data-pjax-fragment="' + fragment + '"]';}, + contentEls = $($.map(fragments.split(','), reduceFn).join(',')); this.trigger('beforestatechange', { - state: state, element: contentEl + state: state, element: contentEls }); // Set Pjax headers, which can declare a preference for the returned view. // The actually returned view isn't always decided upon when the request // is fired, so the server might decide to change it based on its own logic. - var headers = {}; - if(state.data.pjax) { - headers['X-Pjax'] = state.data.pjax; - } else { - // Standard Pjax behaviour is to replace right content area - headers["X-Pjax"] = 'Content'; - } - headers['X-Pjax-Selector'] = selector; + headers['X-Pjax'] = fragments; - contentEl.addClass('loading'); + contentEls.addClass('loading'); var xhr = $.ajax({ headers: headers, url: state.url, @@ -240,46 +231,60 @@ jQuery.noConflict(); // Update title var title = xhr.getResponseHeader('X-Title'); if(title) document.title = title; - - // Update panels - var newContentEl = $(data); - if(newContentEl.find('.cms-container').length) { - throw 'Content loaded via ajax is not allowed to contain tags matching the ".cms-container" selector to avoid infinite loops'; + + // Remove loading indication from old content els (regardless of which are replaced) + contentEls.removeClass('loading'); + + var newFragments = {}; + if(xhr.getResponseHeader('Content-Type') == 'text/json') { + newFragments = data; + } else { + // Fall back to replacing the first fragment only if HTML is returned + newFragments[fragments.split(',').pop()] = data; } + + $.each(newFragments, function(newFragment, html) { + var contentEl = $('[data-pjax-fragment=' + newFragment + ']'), newContentEl = $(html); + + // Update panels + if(newContentEl.find('.cms-container').length) { + throw 'Content loaded via ajax is not allowed to contain tags matching the ".cms-container" selector to avoid infinite loops'; + } + + // Set loading state and store element state + newContentEl.addClass('loading'); + var origStyle = contentEl.attr('style'); + var layoutClasses = ['east', 'west', 'center', 'north', 'south']; + var elemClasses = contentEl.attr('class'); + + var origLayoutClasses = []; + if(elemClasses) { + origLayoutClasses = $.grep( + elemClasses.split(' '), + function(val) { return ($.inArray(val, layoutClasses) >= 0);} + ); + } + + newContentEl + .removeClass(layoutClasses.join(' ')) + .addClass(origLayoutClasses.join(' ')); + if(origStyle) newContentEl.attr('style', origStyle); + newContentEl.css('visibility', 'hidden'); + + // Allow injection of inline styles, as they're not allowed in the document body. + // Not handling this through jQuery.ondemand to avoid parsing the DOM twice. + var styles = newContentEl.find('style').detach(); + if(styles.length) $(document).find('head').append(styles); + + // Replace panel completely (we need to override the "layout" attribute, so can't replace the child instead) + contentEl.replaceWith(newContentEl); + + // Unset loading and restore element state (to avoid breaking existing panel visibility, e.g. with preview expanded) + self.redraw(); + newContentEl.css('visibility', 'visible'); + newContentEl.removeClass('loading'); + }); - // Set loading state and store element state - newContentEl.addClass('loading'); - var origStyle = contentEl.attr('style'); - var layoutClasses = ['east', 'west', 'center', 'north', 'south']; - var elemClasses = contentEl.attr('class'); - - var origLayoutClasses = []; - if(elemClasses) { - origLayoutClasses = $.grep( - elemClasses.split(' '), - function(val) { return ($.inArray(val, layoutClasses) >= 0);} - ); - } - - newContentEl - .removeClass(layoutClasses.join(' ')) - .addClass(origLayoutClasses.join(' ')); - if(origStyle) newContentEl.attr('style', origStyle); - newContentEl.css('visibility', 'hidden'); - - // Allow injection of inline styles, as they're not allowed in the document body. - // Not handling this through jQuery.ondemand to avoid parsing the DOM twice. - var styles = newContentEl.find('style').detach(); - if(styles.length) $(document).find('head').append(styles); - - // Replace panel completely (we need to override the "layout" attribute, so can't replace the child instead) - contentEl.replaceWith(newContentEl); - - // Unset loading and restore element state (to avoid breaking existing panel visibility, e.g. with preview expanded) - self.redraw(); - newContentEl.css('visibility', 'visible'); - newContentEl.removeClass('loading'); - self.trigger('afterstatechange', {data: data, status: status, xhr: xhr, element: newContentEl}); }, error: function(xhr, status, e) { @@ -290,6 +295,7 @@ jQuery.noConflict(); this.setCurrentXHR(xhr); }, + /** * Function: refresh * diff --git a/admin/templates/CMSBreadcrumbs.ss b/admin/templates/CMSBreadcrumbs.ss index 77a1cdd42..9be8125c0 100644 --- a/admin/templates/CMSBreadcrumbs.ss +++ b/admin/templates/CMSBreadcrumbs.ss @@ -1,4 +1,4 @@ -
+
<% loop Breadcrumbs %> <% if Last %> $Title.XML diff --git a/admin/templates/Includes/LeftAndMain_Content.ss b/admin/templates/Includes/LeftAndMain_Content.ss index 05f1082ed..9283f94f0 100644 --- a/admin/templates/Includes/LeftAndMain_Content.ss +++ b/admin/templates/Includes/LeftAndMain_Content.ss @@ -1,4 +1,4 @@ -
+
$Tools diff --git a/admin/templates/Includes/ModelAdmin_Content.ss b/admin/templates/Includes/ModelAdmin_Content.ss index 08a165e77..9a2a512b2 100644 --- a/admin/templates/Includes/ModelAdmin_Content.ss +++ b/admin/templates/Includes/ModelAdmin_Content.ss @@ -1,4 +1,4 @@ -
+
diff --git a/docs/en/reference/cms-architecture.md b/docs/en/reference/cms-architecture.md index 5390866ad..1f1390398 100644 --- a/docs/en/reference/cms-architecture.md +++ b/docs/en/reference/cms-architecture.md @@ -145,7 +145,7 @@ Selectors used in these files should mirrow the "scope" set by its filename, so don't place a rule applying to all form buttons inside `ModelAdmin.js`. The CMS relies heavily on Ajax-loading of interfaces, so each interface and the JavaScript -driving it have to assume its underlying DOM structure is appended via Ajax callback +driving it have to assume its underlying DOM structure is appended via an Ajax callback rather than being available when the browser window first loads. jQuery.entwine is effectively an advanced version of [jQuery.live](http://api.jquery.com/live/) and [jQuery.delegate](http://api.jquery.com/delegate/), so takes care of dynamic event binding. @@ -167,9 +167,6 @@ a CMS developer needs to fire a navigation event rather than invoking the Ajax c The main point of contact here is `$('.cms-container').loadPanel(, , <data>)` in `LeftAndMain.js`. The `data` object can contain additional state which is required in case the same navigation event is fired again (e.g. when the user pressed the back button). -Most commonly, the (optional) `data.selector` property declares which DOM element to replace -with the newly loaded HTML (it defaults to `.cms-content`). This is handy to only replace -e.g. an edit form, but leave the search panel in the same "content area" unchanged. No callbacks are allowed in this style of Ajax loading, as all state needs to be "repeatable". Any logic required to be exected after the Ajax call @@ -177,16 +174,79 @@ should be placed in jQuery.entinwe `onmatch()` rules which apply to the newly cr See `$('.cms-container').handleStateChange()` in `LeftAndMain.js` for details. Alternatively, form-related Ajax calls can be invoked through their own wrappers, -which don't cause history events and hence allow callbacks: `$('.cms-content').submitForm()`. +which don't cause history events and hence allow callbacks: `$('.cms-container').submitForm()`. + +## PJAX: Partial template replacement through Ajax + +Many user interactions can change more than one area in the CMS. +For example, editing a page title in the CMS form changes it in the page tree +as well as the breadcrumbs. In order to avoid unnecessary processing, +we often want to update these sections independently from their neighbouring content. + +In order for this to work, the CMS templates declare certain sections as "PJAX fragments" +through a `data-pjax-fragment` attribute. These names correlate to specific +rendering logic in the PHP controllers, through the `[api:PjaxResponseNegotiator]` class. -Within the PHP logic, the `[api:PjaxResponseNegotiator]` class determines which view is rendered. Through a custom `X-Pjax` HTTP header, the client can declare which view he's expecting, through identifiers like `CurrentForm` or `Content` (see `[api:LeftAndMain->getResponseNegotiator()]`). These identifiers are passed to `loadPanel()` via the `pjax` data option. +The HTTP response is a JSON object literal, with template replacements keyed by their Pjax fragment. +Through PHP callbacks, we ensure that only the required template parts are actually executed and rendered. +When the same URL is loaded without Ajax (and hence without `X-Pjax` headers), +it should behave like a normal full page template, but using the same controller logic. + +Example: Create a bare-bones CMS subclass which shows breadcrumbs (a built-in method), +as well as info on the current record. A single link updates both sections independently +in a single Ajax request. + + :::php + // mysite/code/MyAdmin.php + class MyAdmin extends LeftAndMain { + static $url_segment = 'myadmin'; + public function getResponseNegotiator() { + $negotiator = parent::getResponseNegotiator(); + $controller = $this; + // Register a new callback + $negotiator->setCallback('MyRecordInfo', function() use(&$controller) { + return $controller->MyRecordInfo(); + }); + return $negotiator; + } + public function MyRecordInfo() { + return $this->renderWith('MyRecordInfo'); + } + } + + :::js + // MyAdmin.ss + <% include CMSBreadcrumbs %> + <div>Static content (not affected by update)</div> + <% include MyRecordInfo %> + <a href="admin/myadmin" class="cms-panel-link" data-pjax-target="MyRecordInfo,Breadcrumbs"> + Update record info + </a> + + :::ss + // MyRecordInfo.ss + <div data-pjax-fragment="MyRecordInfo"> + Current Record: $currentPage.Title + </div> + +A click on the link will cause the following (abbreviated) ajax HTTP request: + + GET /admin/myadmin HTTP/1.1 + X-Pjax:Content + X-Requested-With:XMLHttpRequest + +... and result in the following response: + + {"MyRecordInfo": "<div...", "CMSBreadcrumbs": "<div..."} + Keep in mind that the returned view isn't always decided upon when the Ajax request is fired, so the server might decide to change it based on its own logic, sending back different `X-Pjax` headers and content. + ## Ajax Redirects Sometimes, a server response represents a new URL state, e.g. when submitting an "add record" form, @@ -226,8 +286,7 @@ Built-in headers are: Some links should do more than load a new page in the browser window. To avoid repetition, we've written some helpers for various use cases: - * Load into a panel: `<a href="..." class="cms-panel-link" data-target-panel=".cms-content">` - * Load via ajax, and show response status message: `<a href="..." class="cms-link-ajax">` + * Load into a PJAX panel: `<a href="..." class="cms-panel-link" data-pjax-target="Content">` * Load URL as an iframe into a popup/dialog: `<a href="..." class="ss-ui-dialog-link">` ## Buttons diff --git a/forms/gridfield/GridFieldDetailForm.php b/forms/gridfield/GridFieldDetailForm.php index 0a5b8de71..6fd8233eb 100644 --- a/forms/gridfield/GridFieldDetailForm.php +++ b/forms/gridfield/GridFieldDetailForm.php @@ -326,6 +326,7 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler { // TODO Allow customization, e.g. to display an edit form alongside a search form from the CMS controller $form->setTemplate('LeftAndMain_EditForm'); $form->addExtraClass('cms-content cms-edit-form center ss-tabset'); + $form->setAttribute('data-pjax-fragment', 'CurrentForm Content'); if($form->Fields()->hasTabset()) $form->Fields()->findOrMakeTab('Root')->setTemplate('CMSTabSet'); // TODO Link back to controller action (and edited root record) rather than index, // which requires more URL knowledge than the current link to this field gives us. diff --git a/forms/gridfield/GridFieldLevelup.php b/forms/gridfield/GridFieldLevelup.php index 51f3170da..a7327842b 100644 --- a/forms/gridfield/GridFieldLevelup.php +++ b/forms/gridfield/GridFieldLevelup.php @@ -33,7 +33,7 @@ class GridFieldLevelup implements GridField_HTMLProvider{ //$controller = $gridField->getForm()->Controller(); $forTemplate = new ArrayData(array( 'UpLink' => sprintf( - '<a class="cms-panel-link list-parent-link" href="?ParentID=%d&view=list" data-target-panel="#Form_ListViewForm" data-pjax="ListViewForm">%s</a>', + '<a class="cms-panel-link list-parent-link" href="?ParentID=%d&view=list" data-pjax-target="ListViewForm,Breadcrumbs">%s</a>', $parentID, _t('GridField.LEVELUP', 'Level up' ) ), From b3d99d5418136e5b1f36203a3ec2c4942bb18cc1 Mon Sep 17 00:00:00 2001 From: Ingo Schommer <ingo@silverstripe.com> Date: Wed, 30 May 2012 11:53:54 +0200 Subject: [PATCH 3/8] MINOR Retaining case in PjaxResponseNegotiator, in order to pass back to client correctly --- control/PjaxResponseNegotiator.php | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/control/PjaxResponseNegotiator.php b/control/PjaxResponseNegotiator.php index 06f598e5b..6aa4e9732 100644 --- a/control/PjaxResponseNegotiator.php +++ b/control/PjaxResponseNegotiator.php @@ -40,16 +40,12 @@ class PjaxResponseNegotiator { */ public function respond(SS_HTTPRequest $request, $extraCallbacks = array()) { // Prepare the default options and combine with the others - $callbacks = array_merge( - array_change_key_case($this->callbacks, CASE_LOWER), - array_change_key_case($extraCallbacks, CASE_LOWER) - ); - + $callbacks = array_merge($this->callbacks, $extraCallbacks); $response = new SS_HTTPResponse(); $responseParts = array(); if($fragmentStr = $request->getHeader('X-Pjax')) { - $fragments = explode(',', strtolower($fragmentStr)); + $fragments = explode(',', $fragmentStr); foreach($fragments as $fragment) { if(isset($callbacks[$fragment])) { $responseParts[$fragment] = call_user_func($callbacks[$fragment]); From 75e51de9ed26bbb0138460d6529f1e8c4f2d815a Mon Sep 17 00:00:00 2001 From: Ingo Schommer <ingo@silverstripe.com> Date: Wed, 30 May 2012 11:57:49 +0200 Subject: [PATCH 4/8] API CHANGE Moved submitForm() from $('.cms-content') to $('.cms-container'), unifying ajax response handling between panel loading and form submission --- admin/javascript/LeftAndMain.AddForm.js | 2 +- admin/javascript/LeftAndMain.Content.js | 162 ----------------------- admin/javascript/LeftAndMain.EditForm.js | 3 +- admin/javascript/LeftAndMain.js | 92 +++++++++++++ 4 files changed, 94 insertions(+), 165 deletions(-) diff --git a/admin/javascript/LeftAndMain.AddForm.js b/admin/javascript/LeftAndMain.AddForm.js index 5f32a841a..57eef8278 100644 --- a/admin/javascript/LeftAndMain.AddForm.js +++ b/admin/javascript/LeftAndMain.AddForm.js @@ -91,7 +91,7 @@ data.push({name:button.attr('name'),value:button.val()}); // TODO Should be set by hiddenfield already - $('.cms-content').submitForm( + $('.cms-container').submitForm( this, button, function() { diff --git a/admin/javascript/LeftAndMain.Content.js b/admin/javascript/LeftAndMain.Content.js index 2f69663ad..819292a0d 100644 --- a/admin/javascript/LeftAndMain.Content.js +++ b/admin/javascript/LeftAndMain.Content.js @@ -27,168 +27,6 @@ this.add(this.find('.cms-tabset')).redrawTabs(); this.layout(); - }, - - /** - * Function: ajaxSubmit - * - * Parameters: - * {DOMElement} button - The pressed button (optional) - * {Function} callback - Called in complete() handler of jQuery.ajax() - * {Object} ajaxOptions - Object literal to merge into $.ajax() call - * {boolean} loadResponse - Render response through _loadResponse() (Default: true) - * - * Returns: - * (boolean) - */ - submitForm: function(form, button, callback, ajaxOptions, loadResponse) { - var self = this; - - // look for save button - if(!button) button = this.find('.Actions :submit[name=action_save]'); - // default to first button if none given - simulates browser behaviour - if(!button) button = this.find('.Actions :submit:first'); - - form.trigger('beforesave'); - this.trigger('submitform', {form: form, button: button}); - - // set button to "submitting" state - $(button).addClass('loading'); - - // validate if required - if(!form.validate()) { - // TODO Automatically switch to the tab/position of the first error - statusMessage("Validation failed.", "bad"); - - $(button).removeClass('loading'); - - return false; - } - - // save tab selections in order to reconstruct them later - var selectedTabs = []; - form.find('.cms-tabset').each(function(i, el) { - if($(el).attr('id')) selectedTabs.push({id:$(el).attr('id'), selected:$(el).tabs('option', 'selected')}); - }); - - // get all data from the form - var formData = form.serializeArray(); - // add button action - formData.push({name: $(button).attr('name'), value:'1'}); - // Artificial HTTP referer, IE doesn't submit them via ajax. - // Also rewrites anchors to their page counterparts, which is important - // as automatic browser ajax response redirects seem to discard the hash/fragment. - formData.push({name: 'BackURL', value:History.getPageUrl()}); - - // Standard Pjax behaviour is to replace the submitted form with new content. - // The returned view isn't always decided upon when the request - // is fired, so the server might decide to change it based on its own logic, - // sending back different `X-Pjax` headers and content - jQuery.ajax(jQuery.extend({ - headers: { - "X-Pjax" : "CurrentForm,Breadcrumbs" - }, - url: form.attr('action'), - data: formData, - type: 'POST', - complete: function(xmlhttp, status) { - $(button).removeClass('loading'); - - // TODO This should be using the plugin API - form.removeClass('changed'); - - if(callback) callback(xmlhttp, status); - - // pass along original form data to enable old/new comparisons - if(loadResponse !== false) { - self.submitForm_responseHandler(form, xmlhttp.responseText, status, xmlhttp, formData); - } - - // Re-init tabs (in case the form tag itself is a tabset) - if(self.hasClass('cms-tabset')) self.removeClass('cms-tabset').addClass('cms-tabset'); - - // re-select previously saved tabs - $.each(selectedTabs, function(i, selectedTab) { - form.find('#' + selectedTab.id).tabs('select', selectedTab.selected); - }); - - // Redraw the layout - $('.cms-container').redraw(); - }, - dataType: 'html' - }, ajaxOptions)); - - return false; - }, - - /** - * Function: _loadResponse - * - * Parameters: - * {String} data - Either HTML for straight insertion, or eval'ed JavaScript. - * If passed as HTML, it is assumed that everying inside the <form> tag is replaced, - * but the old <form> tag itself stays intact. - * {String} status - * {XMLHTTPRequest} xmlhttp - .. - * {Array} origData - The original submitted data, useful to do comparisons of changed - * values in new form output, e.g. to detect a URLSegment being changed on the serverside. - * Array in jQuery serializeArray() notation. - */ - submitForm_responseHandler: function(oldForm, data, status, xmlhttp, origData) { - if(status == 'success') { - if(!data) return; - - var form, newContent = $(data); - - // HACK If response contains toplevel panel rather than a form, replace it instead. - // For example, a page view shows tree + edit form. Deleting this page redirects to - // the "pages" overview, which doesn't have a separate tree panel. - if(newContent.is('.cms-content')) { - $('.cms-content').replaceWith(newContent); - } else { - form = this.replaceForm(oldForm, newContent); - } - - if(typeof(Behaviour) != 'undefined') Behaviour.apply(); // refreshes ComplexTableField - - this.trigger('reloadeditform', {form: form, origData: origData, xmlhttp: xmlhttp}); - } - }, - - /** - * @return {jQuery} New form element - */ - replaceForm: function(form, html) { - if(html) { - var parent = form.parent(), id = form.attr('id'); - form.replaceWith(html); - // Try to get the new form by ID (assuming they're identical), otherwise fall back to the first form in the parent - return id ? $('#' + id) : parent.children('form:first'); - } else { - this.removeForm(form); - return null; - } - }, - - /** - * Function: removeForm - * - * Remove everying inside the <form> tag - * with a custom HTML fragment. Useful e.g. for deleting a page in the CMS. - * Checks for unsaved changes before removing the form - * - * Parameters: - * {String} placeholderHtml - Short note why the form has been removed, displayed in <p> tags. - * Falls back to the default RemoveText() option (Optional) - */ - removeForm: function(form, placeholderHtml) { - if(!placeholderHtml) placeholderHtml = this.getPlaceholderHtml(); - // Alert when unsaved changes are present - if(!form.confirmUnsavedChanges()) return; - this.trigger('removeform'); - this.html(placeholderHtml); - // TODO This should be using the plugin API - this.removeClass('changed'); } }); diff --git a/admin/javascript/LeftAndMain.EditForm.js b/admin/javascript/LeftAndMain.EditForm.js index 821a79f0f..df21d57eb 100644 --- a/admin/javascript/LeftAndMain.EditForm.js +++ b/admin/javascript/LeftAndMain.EditForm.js @@ -29,7 +29,6 @@ * Events: * ajaxsubmit - Form is about to be submitted through ajax * validate - Contains validation result - * removeform - A form is about to be removed from the DOM * load - Form is about to be loaded through ajax */ $('.cms-edit-form').entwine(/** @lends ss.Form_EditForm */{ @@ -158,7 +157,7 @@ // which means the browser auto-selects the first available form button. // This might be an unrelated button of the form field, // or a destructive action (if "save" is not available, or not on first position). - if(button) this.closest('.cms-content').submitForm(this, button); + if(button) this.closest('.cms-container').submitForm(this, button); return false; }, diff --git a/admin/javascript/LeftAndMain.js b/admin/javascript/LeftAndMain.js index 8fec12c78..d3fe34a4f 100644 --- a/admin/javascript/LeftAndMain.js +++ b/admin/javascript/LeftAndMain.js @@ -178,6 +178,98 @@ jQuery.noConflict(); window.location = $.path.makeUrlAbsolute(url, $('base').attr('href')); } }, + + /** + * Function: submitForm + * + * Parameters: + * {DOMElement} form - The form to be submitted. Needs to be passed + * in to avoid entwine methods/context being removed through replacing the node itself. + * {DOMElement} button - The pressed button (optional) + * {Function} callback - Called in complete() handler of jQuery.ajax() + * {Object} ajaxOptions - Object literal to merge into $.ajax() call + * + * Returns: + * (boolean) + */ + submitForm: function(form, button, callback, ajaxOptions) { + var self = this; + + // look for save button + if(!button) button = this.find('.Actions :submit[name=action_save]'); + // default to first button if none given - simulates browser behaviour + if(!button) button = this.find('.Actions :submit:first'); + + form.trigger('beforesave'); + this.trigger('submitform', {form: form, button: button}); + + // set button to "submitting" state + $(button).addClass('loading'); + + // validate if required + if(!form.validate()) { + // TODO Automatically switch to the tab/position of the first error + statusMessage("Validation failed.", "bad"); + + $(button).removeClass('loading'); + + return false; + } + + // save tab selections in order to reconstruct them later + var selectedTabs = []; + form.find('.cms-tabset').each(function(i, el) { + if($(el).attr('id')) selectedTabs.push({id:$(el).attr('id'), selected:$(el).tabs('option', 'selected')}); + }); + + // get all data from the form + var formData = form.serializeArray(); + // add button action + formData.push({name: $(button).attr('name'), value:'1'}); + // Artificial HTTP referer, IE doesn't submit them via ajax. + // Also rewrites anchors to their page counterparts, which is important + // as automatic browser ajax response redirects seem to discard the hash/fragment. + formData.push({name: 'BackURL', value:History.getPageUrl()}); + + // Standard Pjax behaviour is to replace the submitted form with new content. + // The returned view isn't always decided upon when the request + // is fired, so the server might decide to change it based on its own logic, + // sending back different `X-Pjax` headers and content + jQuery.ajax(jQuery.extend({ + headers: {"X-Pjax" : "CurrentForm,Breadcrumbs"}, + url: form.attr('action'), + data: formData, + type: 'POST', + complete: function() { + $(button).removeClass('loading'); + }, + success: function(data, status, xhr) { + form.removeClass('changed'); // TODO This should be using the plugin API + if(callback) callback(xmlhttp, status); + + var newContentEls = self.handleAjaxResponse(data, status, xhr); + if(!newContentEls) return; + + var newForm = newContentEls.filter('form'); + + // Re-init tabs (in case the form tag itself is a tabset) + if(newForm.hasClass('cms-tabset')) newForm.removeClass('cms-tabset').addClass('cms-tabset'); + + // re-select previously saved tabs + $.each(selectedTabs, function(i, selectedTab) { + newForm.find('#' + selectedTab.id).tabs('select', selectedTab.selected); + }); + + // Redraw the layout + $('.cms-container').redraw(); + + form.trigger('reloadeditform', {form: newForm, formData: formData, xmlhttp: xhr}); + }, + dataType: 'json' + }, ajaxOptions)); + + return false; + }, /** * Handles ajax loading of new panels through the window.History object. From 1102bbdf17e40ec88d2da92d05250f711f503eb8 Mon Sep 17 00:00:00 2001 From: Ingo Schommer <ingo@silverstripe.com> Date: Wed, 30 May 2012 14:58:24 +0200 Subject: [PATCH 5/8] MINOR Removed ability in PjaxResponseNegotiator to send non-JSON responses for single fragments, as we're now relying on the server communicating which segment needs replacement (through the JSON key) --- control/PjaxResponseNegotiator.php | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/control/PjaxResponseNegotiator.php b/control/PjaxResponseNegotiator.php index 6aa4e9732..71c852b32 100644 --- a/control/PjaxResponseNegotiator.php +++ b/control/PjaxResponseNegotiator.php @@ -53,18 +53,8 @@ class PjaxResponseNegotiator { throw new SS_HTTPResponse_Exception("X-Pjax = '$fragment' not supported for this URL.", 400); } } - if(count($responseParts) == 1) { - $response->setBody(array_pop($responseParts)); - } else { - if($request->getHeader('Accept') != 'text/json') { - throw new SS_HTTPResponse_Exception( - 'Multiple comma-separated fragments can only be returne with an "Accept: text/json" header', - 400 - ); - } - $response->setBody(Convert::raw2json($responseParts)); - $response->addHeader('Content-Type', 'text/json'); - } + $response->setBody(Convert::raw2json($responseParts)); + $response->addHeader('Content-Type', 'text/json'); } else { if($request->isAjax()) throw new SS_HTTPResponse_Exception("Ajax requests to this URL require an X-Pjax header.", 400); $response->setBody(call_user_func($callbacks['default'])); From 19bfd01a603bb6f7bcae98a917e8c2490414820d Mon Sep 17 00:00:00 2001 From: Ingo Schommer <ingo@silverstripe.com> Date: Wed, 30 May 2012 15:01:40 +0200 Subject: [PATCH 6/8] ENHANCEMENT Declare a single DOM element with multiple Pjax fragments. Unified ajax response handling in CMS, followup from submitForm() refactoring. Removed replacement of arbitrary CSS selectors through Pjax, relies on a fragment now (to keep logic consistent). --- admin/javascript/LeftAndMain.js | 181 ++++++++++++++++++-------------- 1 file changed, 103 insertions(+), 78 deletions(-) diff --git a/admin/javascript/LeftAndMain.js b/admin/javascript/LeftAndMain.js index d3fe34a4f..c3c9b3545 100644 --- a/admin/javascript/LeftAndMain.js +++ b/admin/javascript/LeftAndMain.js @@ -154,11 +154,10 @@ jQuery.noConflict(); loadPanel: function(url, title, data) { if(!data) data = {}; if(!title) title = ""; - if(!data.selector) data.selector = '.cms-content'; - var contentEl = $(data.selector); - + // Check change tracking (can't use events as we need a way to cancel the current state change) - var trackedEls = contentEl.find(':data(changetracker)').add(contentEl.filter(':data(changetracker)')); + var contentEls = this._findFragments(data.pjax ? data.pjax.split(',') : ['Content']); + var trackedEls = contentEls.find(':data(changetracker)').add(contentEls.filter(':data(changetracker)')); if(trackedEls.length) { var abort = false; @@ -299,12 +298,9 @@ jQuery.noConflict(); var self = this, h = window.History, state = h.getState(), fragments = state.data.pjax || 'Content', headers = {}, - reduceFn = function(fragment) {return '[data-pjax-fragment="' + fragment + '"]';}, - contentEls = $($.map(fragments.split(','), reduceFn).join(',')); + contentEls = this._findFragments(fragments.split(',')); - this.trigger('beforestatechange', { - state: state, element: contentEls - }); + this.trigger('beforestatechange', {state: state, element: contentEls}); // Set Pjax headers, which can declare a preference for the returned view. // The actually returned view isn't always decided upon when the request @@ -315,79 +311,111 @@ jQuery.noConflict(); var xhr = $.ajax({ headers: headers, url: state.url, - success: function(data, status, xhr) { - // Pseudo-redirects via X-ControllerURL might return empty data, in which - // case we'll ignore the response - if(!data) return; - - // Update title - var title = xhr.getResponseHeader('X-Title'); - if(title) document.title = title; - + complete: function() { // Remove loading indication from old content els (regardless of which are replaced) contentEls.removeClass('loading'); - - var newFragments = {}; - if(xhr.getResponseHeader('Content-Type') == 'text/json') { - newFragments = data; - } else { - // Fall back to replacing the first fragment only if HTML is returned - newFragments[fragments.split(',').pop()] = data; - } - - $.each(newFragments, function(newFragment, html) { - var contentEl = $('[data-pjax-fragment=' + newFragment + ']'), newContentEl = $(html); - - // Update panels - if(newContentEl.find('.cms-container').length) { - throw 'Content loaded via ajax is not allowed to contain tags matching the ".cms-container" selector to avoid infinite loops'; - } - - // Set loading state and store element state - newContentEl.addClass('loading'); - var origStyle = contentEl.attr('style'); - var layoutClasses = ['east', 'west', 'center', 'north', 'south']; - var elemClasses = contentEl.attr('class'); - - var origLayoutClasses = []; - if(elemClasses) { - origLayoutClasses = $.grep( - elemClasses.split(' '), - function(val) { return ($.inArray(val, layoutClasses) >= 0);} - ); - } - - newContentEl - .removeClass(layoutClasses.join(' ')) - .addClass(origLayoutClasses.join(' ')); - if(origStyle) newContentEl.attr('style', origStyle); - newContentEl.css('visibility', 'hidden'); - - // Allow injection of inline styles, as they're not allowed in the document body. - // Not handling this through jQuery.ondemand to avoid parsing the DOM twice. - var styles = newContentEl.find('style').detach(); - if(styles.length) $(document).find('head').append(styles); - - // Replace panel completely (we need to override the "layout" attribute, so can't replace the child instead) - contentEl.replaceWith(newContentEl); - - // Unset loading and restore element state (to avoid breaking existing panel visibility, e.g. with preview expanded) - self.redraw(); - newContentEl.css('visibility', 'visible'); - newContentEl.removeClass('loading'); - }); - - self.trigger('afterstatechange', {data: data, status: status, xhr: xhr, element: newContentEl}); }, - error: function(xhr, status, e) { - contentEl.removeClass('loading'); - errorMessage(e); + success: function(data, status, xhr) { + var els = self.handleAjaxResponse(data, status, xhr); + self.trigger('afterstatechange', {data: data, status: status, xhr: xhr, element: els}); } }); this.setCurrentXHR(xhr); }, + /** + * Handles ajax responses containing plain HTML, or mulitple + * PJAX fragments wrapped in JSON (see PjaxResponseNegotiator PHP class). + * Can be hooked into an ajax 'success' callback. + */ + handleAjaxResponse: function(data, status, xhr) { + var self = this; + + // Pseudo-redirects via X-ControllerURL might return empty data, in which + // case we'll ignore the response + if(!data) return; + + // Update title + var title = xhr.getResponseHeader('X-Title'); + if(title) document.title = title; + + var newFragments = {}, newContentEls = $([]); + if(xhr.getResponseHeader('Content-Type') == 'text/json') { + newFragments = data; + } else { + // Fall back to replacing the content fragment if HTML is returned + newFragments['Content'] = data; + } + + // Replace each fragment individually + $.each(newFragments, function(newFragment, html) { + var contentEl = $('[data-pjax-fragment]').filter(function() { + return $.inArray(newFragment, $(this).data('pjaxFragment').split(' ')) != -1; + }), newContentEl = $(html); + + // Add to result collection + newContentEls.add(newContentEl); + + // Update panels + if(newContentEl.find('.cms-container').length) { + throw 'Content loaded via ajax is not allowed to contain tags matching the ".cms-container" selector to avoid infinite loops'; + } + + // Set loading state and store element state + var origStyle = contentEl.attr('style'); + var origVisible = contentEl.is(':visible'); + var layoutClasses = ['east', 'west', 'center', 'north', 'south']; + var elemClasses = contentEl.attr('class'); + var origLayoutClasses = []; + if(elemClasses) { + origLayoutClasses = $.grep( + elemClasses.split(' '), + function(val) { return ($.inArray(val, layoutClasses) >= 0);} + ); + } + + newContentEl + .removeClass(layoutClasses.join(' ')) + .addClass(origLayoutClasses.join(' ')); + if(origStyle) newContentEl.attr('style', origStyle); + newContentEl.css('visibility', 'hidden'); + + // Allow injection of inline styles, as they're not allowed in the document body. + // Not handling this through jQuery.ondemand to avoid parsing the DOM twice. + var styles = newContentEl.find('style').detach(); + if(styles.length) $(document).find('head').append(styles); + + // Replace panel completely (we need to override the "layout" attribute, so can't replace the child instead) + contentEl.replaceWith(newContentEl); + + // Unset loading and restore element state (to avoid breaking existing panel visibility, e.g. with preview expanded) + if(origVisible) newContentEl.css('visibility', 'visible'); + }); + + this.redraw(); + + return newContentEls; + }, + + /** + * + * + * Parameters: + * - fragments {Array} + * Returns: jQuery collection + */ + _findFragments: function(fragments) { + return $('[data-pjax-fragment]').filter(function() { + // Allows for more than one fragment per node + var i, nodeFragments = $(this).data('pjaxFragment').split(' '); + for(i in fragments) { + if($.inArray(fragments[i], nodeFragments) != -1) return true; + } + return false; + }); + }, + /** * Function: refresh * @@ -440,16 +468,13 @@ jQuery.noConflict(); * as opposed to triggering a full page reload. * Little helper to avoid repetition, and make it easy to * "opt in" to panel loading, while by default links still exhibit their default behaviour. - * Same goes for breadcrumbs in the CMS. + * The PJAX target can be specified via a 'data-pjax-target' attribute. */ $('.cms .cms-panel-link').entwine({ onclick: function(e) { var href = this.attr('href'), url = (href && !href.match(/^#/)) ? href : this.data('href'), - data = { - selector: this.data('targetPanel'), - pjax: this.data('pjax') - }; + data = {pjax: this.data('pjaxTarget')}; $('.cms-container').loadPanel(url, null, data); e.preventDefault(); From 0eac6ea5c33b24d3655de027c84506b1742417b5 Mon Sep 17 00:00:00 2001 From: Ingo Schommer <ingo@silverstripe.com> Date: Wed, 30 May 2012 15:38:08 +0200 Subject: [PATCH 7/8] MINOR Apply loading indicators to all forms, not just .cms-edit-form (which means it includes the "pages list view"). Move to correct place in LeftAndMain.js (not specific to '.cms-content') --- admin/javascript/LeftAndMain.Content.js | 11 ----------- admin/javascript/LeftAndMain.js | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/admin/javascript/LeftAndMain.Content.js b/admin/javascript/LeftAndMain.Content.js index 819292a0d..e93de9617 100644 --- a/admin/javascript/LeftAndMain.Content.js +++ b/admin/javascript/LeftAndMain.Content.js @@ -77,17 +77,6 @@ this._super(); } }); - - $('.cms-content.loading,.cms-edit-form.loading,.cms-content-fields.loading,.cms-content-view.loading').entwine({ - onmatch: function() { - this.append('<div class="cms-content-loading-overlay ui-widget-overlay-light"></div><div class="cms-content-loading-spinner"></div>'); - this._super(); - }, - onunmatch: function() { - this.find('.cms-content-loading-overlay,.cms-content-loading-spinner').remove(); - this._super(); - } - }); }); })(jQuery); diff --git a/admin/javascript/LeftAndMain.js b/admin/javascript/LeftAndMain.js index c3c9b3545..66db2931e 100644 --- a/admin/javascript/LeftAndMain.js +++ b/admin/javascript/LeftAndMain.js @@ -436,6 +436,22 @@ jQuery.noConflict(); } }); + /** + * Add loading overlay to selected regions in the CMS automatically. + * Not applied to all "*.loading" elements to avoid secondary regions + * like the breadcrumbs showing unnecessary loading status. + */ + $('form.loading,.cms-content.loading,.cms-content-fields.loading,.cms-content-view.loading').entwine({ + onmatch: function() { + this.append('<div class="cms-content-loading-overlay ui-widget-overlay-light"></div><div class="cms-content-loading-spinner"></div>'); + this._super(); + }, + onunmatch: function() { + this.find('.cms-content-loading-overlay,.cms-content-loading-spinner').remove(); + this._super(); + } + }); + /** * Make all buttons "hoverable" with jQuery theming. * Also sets the clicked button on a form submission, making it available through From 5a93ee77f894bc26df486ae9bde16250f638cdd9 Mon Sep 17 00:00:00 2001 From: Ingo Schommer <ingo@silverstripe.com> Date: Wed, 30 May 2012 16:05:30 +0200 Subject: [PATCH 8/8] MINOR Fixed tab selection state --- admin/javascript/LeftAndMain.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/admin/javascript/LeftAndMain.js b/admin/javascript/LeftAndMain.js index 66db2931e..813c241de 100644 --- a/admin/javascript/LeftAndMain.js +++ b/admin/javascript/LeftAndMain.js @@ -810,7 +810,7 @@ jQuery.noConflict(); this.rewriteHashlinks(); var id = this.attr('id'), cookieId = 'ui-tabs-' + id, - selectedTab = this.find('ul:first .ui-state-selected'); + selectedTab = this.find('ul:first .ui-tabs-selected'); // Fix for wrong cookie storage of deselected tabs if($.cookie && id && $.cookie(cookieId) == -1) $.cookie(cookieId, 0);