From 72e08456a8dcebd61e8a3476504c25209546f780 Mon Sep 17 00:00:00 2001 From: Peter Fogg Date: Fri, 21 Jun 2013 16:50:32 -0400 Subject: [PATCH 1/3] Refactor Advanced Settings page to use Backbone notifications. --- cms/static/js/views/settings/advanced_view.js | 91 ++++++++++--------- cms/templates/base.html | 6 -- cms/templates/settings_advanced.html | 57 ------------ 3 files changed, 46 insertions(+), 108 deletions(-) diff --git a/cms/static/js/views/settings/advanced_view.js b/cms/static/js/views/settings/advanced_view.js index 863393d341..69a2c9f622 100644 --- a/cms/static/js/views/settings/advanced_view.js +++ b/cms/static/js/views/settings/advanced_view.js @@ -20,9 +20,6 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ self.render(); } ); - // because these are outside of this.$el, they can't be in the event hash - $('.save-button').on('click', this, this.saveView); - $('.cancel-button').on('click', this, this.revertView); this.listenTo(this.model, 'invalid', this.handleValidationError); }, render: function() { @@ -45,7 +42,6 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ var policyValues = listEle$.find('.json'); _.each(policyValues, this.attachJSONEditor, this); - this.showMessage(); return this; }, attachJSONEditor : function (textarea) { @@ -61,7 +57,9 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ mode: "application/json", lineNumbers: false, lineWrapping: false, onChange: function(instance, changeobj) { // this event's being called even when there's no change :-( - if (instance.getValue() !== oldValue) self.showSaveCancelButtons(); + if (instance.getValue() !== oldValue && !self.notificationBarShowing) { + self.showNotificationBar(); + } }, onFocus : function(mirror) { $(textarea).parent().children('label').addClass("is-focused"); @@ -99,59 +97,62 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ } }); }, - showMessage: function (type) { - $(".wrapper-alert").removeClass("is-shown"); - if (type) { - if (type === this.error_saving) { - $(".wrapper-alert-error").addClass("is-shown").attr('aria-hidden','false'); - } - else if (type === this.successful_changes) { - $(".wrapper-alert-confirmation").addClass("is-shown").attr('aria-hidden','false'); - this.hideSaveCancelButtons(); - } - } - else { - // This is the case of the page first rendering, or when Cancel is pressed. - this.hideSaveCancelButtons(); - } + showNotificationBar: function() { + var self = this; + var message = gettext("Your changes will not take effect until you save your progress. Take care with key and value formatting, as validation is not implemented.") + var confirm = new CMS.Views.Notification.Warning({ + title: gettext("You've Made Some Changes"), + message: message, + actions: { + primary: { + "text": gettext("Save Changes"), + "class": "action-save", + "click": function() { + self.saveView(); + confirm.hide(); + self.notificationBarShowing = false; + } + }, + secondary: [{ + "text": gettext("Cancel"), + "class": "action-cancel", + "click": function() { + self.revertView(); + confirm.hide(); + self.notificationBarShowing = false; + } + }], + }}); + this.notificationBarShowing = true; + confirm.show(); }, - showSaveCancelButtons: function(event) { - if (!this.notificationBarShowing) { - this.$el.find(".message-status").removeClass("is-shown"); - $('.wrapper-notification').removeClass('is-hiding').addClass('is-shown').attr('aria-hidden','false'); - this.notificationBarShowing = true; - } - }, - hideSaveCancelButtons: function() { - if (this.notificationBarShowing) { - $('.wrapper-notification').removeClass('is-shown').addClass('is-hiding').attr('aria-hidden','true'); - this.notificationBarShowing = false; - } - }, - saveView : function(event) { - window.CmsUtils.smoothScrollTop(event); + saveView : function() { // TODO one last verification scan: // call validateKey on each to ensure proper format // check for dupes - var self = event.data; - self.model.save({}, + var self = this; + this.model.save({}, { success : function() { self.render(); - self.showMessage(self.successful_changes); + var message = gettext("Please note that validation of your policy key and value pairs is not currently in place yet. If you are having difficulties, please review your policy pairs."); + var saving = new CMS.Views.Alert.Confirmation({ + title: gettext("Your policy changes have been saved."), + message: message, + closeIcon: false + }); + saving.show(); analytics.track('Saved Advanced Settings', { 'course': course_location_analytics }); - } }); }, - revertView : function(event) { - event.preventDefault(); - var self = event.data; - self.model.deleteKeys = []; - self.model.clear({silent : true}); - self.model.fetch({ + revertView : function() { + var self = this; + this.model.deleteKeys = []; + this.model.clear({silent : true}); + this.model.fetch({ success : function() { self.render(); }, reset: true }); diff --git a/cms/templates/base.html b/cms/templates/base.html index 11e8d41496..695a97f1da 100644 --- a/cms/templates/base.html +++ b/cms/templates/base.html @@ -61,8 +61,6 @@
<%include file="widgets/header.html" /> - ## remove this block after advanced settings notification is rewritten - <%block name="view_alerts">
<%block name="content"> @@ -74,13 +72,9 @@ <%include file="widgets/footer.html" /> <%include file="widgets/tender.html" /> - ## remove this block after advanced settings notification is rewritten - <%block name="view_notifications">
- ## remove this block after advanced settings notification is rewritten - <%block name="view_prompts">
<%block name="jsextra"> diff --git a/cms/templates/settings_advanced.html b/cms/templates/settings_advanced.html index 242148418e..6cc3468590 100644 --- a/cms/templates/settings_advanced.html +++ b/cms/templates/settings_advanced.html @@ -104,60 +104,3 @@ editor.render(); - -<%block name="view_notifications"> - - - - -<%block name="view_alerts"> - -
-
- - -
-

Your policy changes have been saved.

-

Please note that validation of your policy key and value pairs is not currently in place yet. If you are having difficulties, please review your policy pairs.

-
- - - - close alert - -
-
- - -
-
- - -
-

There was an error saving your information

-

Please see the error below and correct it to ensure there are no problems in rendering your course.

-
-
-
- From 3e376bd78031db5a87c890359f1fbc776030404e Mon Sep 17 00:00:00 2001 From: Peter Fogg Date: Mon, 24 Jun 2013 11:06:53 -0400 Subject: [PATCH 2/3] Prevent "saved" and "error" views from showing at the same time. Previously the "saved" view was never hidden, even after more data was edited. So if one field was saved successfully and then another was not, we would find ourselves in the unfortunate situation of seeing both views at once, leading to much confusion. --- cms/static/js/views/settings/advanced_view.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/cms/static/js/views/settings/advanced_view.js b/cms/static/js/views/settings/advanced_view.js index 69a2c9f622..102bb71a52 100644 --- a/cms/static/js/views/settings/advanced_view.js +++ b/cms/static/js/views/settings/advanced_view.js @@ -21,6 +21,7 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ } ); this.listenTo(this.model, 'invalid', this.handleValidationError); + this.savedBar = undefined; }, render: function() { // catch potential outside call before template loaded @@ -136,15 +137,22 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ success : function() { self.render(); var message = gettext("Please note that validation of your policy key and value pairs is not currently in place yet. If you are having difficulties, please review your policy pairs."); - var saving = new CMS.Views.Alert.Confirmation({ + self.saved = new CMS.Views.Alert.Confirmation({ title: gettext("Your policy changes have been saved."), message: message, closeIcon: false }); - saving.show(); + self.saved.show(); analytics.track('Saved Advanced Settings', { 'course': course_location_analytics }); + }, + error: function() { + // If we've already saved some data this will be + // shown; hide it away again. + if(self.saved) { + self.saved.hide(); + } } }); }, From fb573a1db64eefc328169d2881c9b1dd25187d14 Mon Sep 17 00:00:00 2001 From: Peter Fogg Date: Mon, 24 Jun 2013 12:40:45 -0400 Subject: [PATCH 3/3] Hide "success" alert as soon as we start editing another field. --- cms/static/js/views/settings/advanced_view.js | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/cms/static/js/views/settings/advanced_view.js b/cms/static/js/views/settings/advanced_view.js index 102bb71a52..302a918de1 100644 --- a/cms/static/js/views/settings/advanced_view.js +++ b/cms/static/js/views/settings/advanced_view.js @@ -21,7 +21,6 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ } ); this.listenTo(this.model, 'invalid', this.handleValidationError); - this.savedBar = undefined; }, render: function() { // catch potential outside call before template loaded @@ -122,10 +121,13 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ confirm.hide(); self.notificationBarShowing = false; } - }], + }] }}); this.notificationBarShowing = true; confirm.show(); + if(this.saved) { + this.saved.hide(); + } }, saveView : function() { // TODO one last verification scan: @@ -146,13 +148,6 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ analytics.track('Saved Advanced Settings', { 'course': course_location_analytics }); - }, - error: function() { - // If we've already saved some data this will be - // shown; hide it away again. - if(self.saved) { - self.saved.hide(); - } } }); },