Refactor Backbone Notifications

Models are extraneous for something as temporary as a notifaction -- this change moves all the configuration into the views, and removes the models entirely.
2013-06-18 09:55:41 -04:00
parent 048be222a6
commit b6e3b0ee03
9 changed files with 182 additions and 184 deletions
--- a/cms/envs/common.py
+++ b/cms/envs/common.py
@@ -235,8 +235,7 @@ PIPELINE_JS = {
        'source_filenames': sorted(
            rooted_glob(COMMON_ROOT / 'static/', 'coffee/src/**/*.js') +
            rooted_glob(PROJECT_ROOT / 'static/', 'coffee/src/**/*.js')
-        ) + ['js/hesitate.js', 'js/base.js',
-             'js/models/feedback.js', 'js/views/feedback.js',
+        ) + ['js/hesitate.js', 'js/base.js', 'js/views/feedback.js',
             'js/models/section.js', 'js/views/section.js',
             'js/models/metadata_model.js', 'js/views/metadata_editor_view.js',
             'js/views/assets.js'],
--- a/cms/static/coffee/spec/models/feedback_spec.coffee
+++ b/cms/static/coffee/spec/models/feedback_spec.coffee
@@ -1,34 +0,0 @@
-describe "CMS.Models.SystemFeedback", ->
-    beforeEach ->
-        @model = new CMS.Models.SystemFeedback()
-
-    it "should have an empty message by default", ->
-        expect(@model.get("message")).toEqual("")
-
-    it "should have an empty title by default", ->
-        expect(@model.get("title")).toEqual("")
-
-    it "should not have an intent set by default", ->
-        expect(@model.get("intent")).toBeNull()
-
-
-describe "CMS.Models.WarningMessage", ->
-    beforeEach ->
-        @model = new CMS.Models.WarningMessage()
-
-    it "should have the correct intent", ->
-        expect(@model.get("intent")).toEqual("warning")
-
-describe "CMS.Models.ErrorMessage", ->
-    beforeEach ->
-        @model = new CMS.Models.ErrorMessage()
-
-    it "should have the correct intent", ->
-        expect(@model.get("intent")).toEqual("error")
-
-describe "CMS.Models.ConfirmationMessage", ->
-    beforeEach ->
-        @model = new CMS.Models.ConfirmationMessage()
-
-    it "should have the correct intent", ->
-        expect(@model.get("intent")).toEqual("confirmation")
--- a/cms/static/coffee/spec/views/feedback_spec.coffee
+++ b/cms/static/coffee/spec/views/feedback_spec.coffee
@@ -18,79 +18,105 @@ beforeEach ->
            else
                return trimmedText.indexOf(text) != -1;

-describe "CMS.Views.Alert as base class", ->
+describe "CMS.Views.SystemFeedback", ->
    beforeEach ->
-        @model = new CMS.Models.ConfirmationMessage({
+        @options =
            title: "Portal"
            message: "Welcome to the Aperture Science Computer-Aided Enrichment Center"
-        })
        # it will be interesting to see when this.render is called, so lets spy on it
-        spyOn(CMS.Views.Alert.prototype, 'render').andCallThrough()
+        @renderSpy = spyOn(CMS.Views.Alert.Confirmation.prototype, 'render').andCallThrough()
+        @showSpy = spyOn(CMS.Views.Alert.Confirmation.prototype, 'show').andCallThrough()
+        @hideSpy = spyOn(CMS.Views.Alert.Confirmation.prototype, 'hide').andCallThrough()

-    it "renders on initalize", ->
-        view = new CMS.Views.Alert({model: @model})
-        expect(view.render).toHaveBeenCalled()
+    it "requires a type and an intent", ->
+        neither = =>
+            new CMS.Views.SystemFeedback(@options)
+        noType = =>
+            options = $.extend({}, @options)
+            options.intent = "confirmation"
+            new CMS.Views.SystemFeedback(options)
+        noIntent = =>
+            options = $.extend({}, @options)
+            options.type = "alert"
+            new CMS.Views.SystemFeedback(options)
+        both = =>
+            options = $.extend({}, @options)
+            options.type = "alert"
+            options.intent = "confirmation"
+            new CMS.Views.SystemFeedback(options)
+
+        expect(neither).toThrow()
+        expect(noType).toThrow()
+        expect(noIntent).toThrow()
+        expect(both).not.toThrow()
+
+    # for simplicity, we'll use CMS.Views.Alert.Confirmation from here on,
+    # which extends and proxies to CMS.Views.SystemFeedback
+
+    it "does not show on initalize", ->
+        view = new CMS.Views.Alert.Confirmation(@options)
+        expect(@renderSpy).not.toHaveBeenCalled()
+        expect(@showSpy).not.toHaveBeenCalled()

    it "renders the template", ->
-        view = new CMS.Views.Alert({model: @model})
+        view = new CMS.Views.Alert.Confirmation(@options)
+        view.show()
+
        expect(view.$(".action-close")).toBeDefined()
        expect(view.$('.wrapper')).toBeShown()
-        expect(view.$el).toContainText(@model.get("title"))
-        expect(view.$el).toContainText(@model.get("message"))
+        expect(view.$el).toContainText(@options.title)
+        expect(view.$el).toContainText(@options.message)

    it "close button sends a .hide() message", ->
-        spyOn(CMS.Views.Alert.prototype, 'hide').andCallThrough()
-
-        view = new CMS.Views.Alert({model: @model})
+        view = new CMS.Views.Alert.Confirmation(@options).show()
        view.$(".action-close").click()

-        expect(CMS.Views.Alert.prototype.hide).toHaveBeenCalled()
+        expect(@hideSpy).toHaveBeenCalled()
        expect(view.$('.wrapper')).toBeHiding()

 describe "CMS.Views.Prompt", ->
-    beforeEach ->
-        @model = new CMS.Models.ConfirmationMessage({
-            title: "Portal"
-            message: "Welcome to the Aperture Science Computer-Aided Enrichment Center"
-        })
-
    # for some reason, expect($("body")) blows up the test runner, so this test
    # just exercises the Prompt rather than asserting on anything. Best I can
    # do for now. :(
    it "changes class on body", ->
        # expect($("body")).not.toHaveClass("prompt-is-shown")
-        view = new CMS.Views.Prompt({model: @model})
+        view = new CMS.Views.Prompt.Confirmation({
+            title: "Portal"
+            message: "Welcome to the Aperture Science Computer-Aided Enrichment Center"
+        })
        # expect($("body")).toHaveClass("prompt-is-shown")
        view.hide()
        # expect($("body")).not.toHaveClass("prompt-is-shown")

-describe "CMS.Views.Alert click events", ->
+describe "CMS.Views.SystemFeedback click events", ->
    beforeEach ->
-        @model = new CMS.Models.WarningMessage(
+        @primaryClickSpy = jasmine.createSpy('primaryClick')
+        @secondaryClickSpy = jasmine.createSpy('secondaryClick')
+        @view = new CMS.Views.Notification.Warning(
            title: "Unsaved",
            message: "Your content is currently Unsaved.",
            actions:
                primary:
                    text: "Save",
                    class: "save-button",
-                    click: jasmine.createSpy('primaryClick')
+                    click: @primaryClickSpy
                secondary: [{
                    text: "Revert",
                    class: "cancel-button",
-                    click: jasmine.createSpy('secondaryClick')
+                    click: @secondaryClickSpy
                }]
-
        )
-
-        @view = new CMS.Views.Alert({model: @model})
+        @view.show()

    it "should trigger the primary event on a primary click", ->
-        @view.primaryClick()
-        expect(@model.get('actions').primary.click).toHaveBeenCalled()
+        @view.$(".action-primary").click()
+        expect(@primaryClickSpy).toHaveBeenCalled()
+        expect(@secondaryClickSpy).not.toHaveBeenCalled()

    it "should trigger the secondary event on a secondary click", ->
-        @view.secondaryClick()
-        expect(@model.get('actions').secondary[0].click).toHaveBeenCalled()
+        @view.$(".action-secondary").click()
+        expect(@secondaryClickSpy).toHaveBeenCalled()
+        expect(@primaryClickSpy).not.toHaveBeenCalled()

    it "should apply class to primary action", ->
        expect(@view.$(".action-primary")).toHaveClass("save-button")
@@ -100,20 +126,19 @@ describe "CMS.Views.Alert click events", ->

 describe "CMS.Views.Notification minShown and maxShown", ->
    beforeEach ->
-        @model = new CMS.Models.SystemFeedback(
-            intent: "saving"
-            title: "Saving"
-        )
-        spyOn(CMS.Views.Notification.prototype, 'show').andCallThrough()
-        spyOn(CMS.Views.Notification.prototype, 'hide').andCallThrough()
+        @showSpy = spyOn(CMS.Views.Notification.Saving.prototype, 'show')
+        @showSpy.andCallThrough()
+        @hideSpy = spyOn(CMS.Views.Notification.Saving.prototype, 'hide')
+        @hideSpy.andCallThrough()
        @clock = sinon.useFakeTimers()

    afterEach ->
+        delete CMS.Views.Notification.active;
        @clock.restore()

    it "a minShown view should not hide too quickly", ->
-        view = new CMS.Views.Notification({model: @model, minShown: 1000})
-        expect(CMS.Views.Notification.prototype.show).toHaveBeenCalled()
+        view = new CMS.Views.Notification.Saving({minShown: 1000})
+        view.show()
        expect(view.$('.wrapper')).toBeShown()

        # call hide() on it, but the minShown should prevent it from hiding right away
@@ -125,8 +150,8 @@ describe "CMS.Views.Notification minShown and maxShown", ->
        expect(view.$('.wrapper')).toBeHiding()

    it "a maxShown view should hide by itself", ->
-        view = new CMS.Views.Notification({model: @model, maxShown: 1000})
-        expect(CMS.Views.Notification.prototype.show).toHaveBeenCalled()
+        view = new CMS.Views.Notification.Saving({maxShown: 1000})
+        view.show()
        expect(view.$('.wrapper')).toBeShown()

        # wait for the maxShown timeout to expire, and check again
@@ -134,13 +159,13 @@ describe "CMS.Views.Notification minShown and maxShown", ->
        expect(view.$('.wrapper')).toBeHiding()

    it "a minShown view can stay visible longer", ->
-        view = new CMS.Views.Notification({model: @model, minShown: 1000})
-        expect(CMS.Views.Notification.prototype.show).toHaveBeenCalled()
+        view = new CMS.Views.Notification.Saving({minShown: 1000})
+        view.show()
        expect(view.$('.wrapper')).toBeShown()

        # wait for the minShown timeout to expire, and check again
        @clock.tick(1001)
-        expect(CMS.Views.Notification.prototype.hide).not.toHaveBeenCalled()
+        expect(@hideSpy).not.toHaveBeenCalled()
        expect(view.$('.wrapper')).toBeShown()

        # can now hide immediately
@@ -148,8 +173,8 @@ describe "CMS.Views.Notification minShown and maxShown", ->
        expect(view.$('.wrapper')).toBeHiding()

    it "a maxShown view can hide early", ->
-        view = new CMS.Views.Notification({model: @model, maxShown: 1000})
-        expect(CMS.Views.Notification.prototype.show).toHaveBeenCalled()
+        view = new CMS.Views.Notification.Saving({maxShown: 1000})
+        view.show()
        expect(view.$('.wrapper')).toBeShown()

        # wait 50 milliseconds, and hide it early
@@ -162,7 +187,8 @@ describe "CMS.Views.Notification minShown and maxShown", ->
        expect(view.$('.wrapper')).toBeHiding()

    it "a view can have both maxShown and minShown", ->
-        view = new CMS.Views.Notification({model: @model, minShown: 1000, maxShown: 2000})
+        view = new CMS.Views.Notification.Saving({minShown: 1000, maxShown: 2000})
+        view.show()

        # can't hide early
        @clock.tick(50)
--- a/cms/static/coffee/src/main.coffee
+++ b/cms/static/coffee/src/main.coffee
@@ -18,11 +18,11 @@ $ ->
  $(document).ajaxError (event, jqXHR, ajaxSettings, thrownError) ->
    if ajaxSettings.notifyOnError is false
      return
-    msg = new CMS.Models.ErrorMessage(
+    msg = new CMS.Views.Notification.Error(
        "title": gettext("Studio's having trouble saving your work")
        "message": jqXHR.responseText || gettext("This may be happening because of an error with our server or your internet connection. Try refreshing the page or making sure you are online.")
    )
-    new CMS.Views.Notification({model: msg})
+    msg.show()

  window.onTouchBasedDevice = ->
    navigator.userAgent.match /iPhone|iPod|iPad/i
--- a/cms/static/js/models/feedback.js
+++ b/cms/static/js/models/feedback.js
@@ -1,55 +0,0 @@
-CMS.Models.SystemFeedback = Backbone.Model.extend({
-    defaults: {
-        "intent": null,  // "warning", "confirmation", "error", "announcement", "step-required", etc
-        "title": "",
-        "message": ""
-        /* could also have an "actions" hash: here is an example demonstrating
-           the expected structure
-        "actions": {
-            "primary": {
-                "text": "Save",
-                "class": "action-save",
-                "click": function() {
-                    // do something when Save is clicked
-                    // `this` refers to the model
-                }
-            },
-            "secondary": [
-                {
-                    "text": "Cancel",
-                    "class": "action-cancel",
-                    "click": function() {}
-                }, {
-                    "text": "Discard Changes",
-                    "class": "action-discard",
-                    "click": function() {}
-                }
-            ]
-        }
-        */
-    }
-});
-
-CMS.Models.WarningMessage = CMS.Models.SystemFeedback.extend({
-    defaults: $.extend({}, CMS.Models.SystemFeedback.prototype.defaults, {
-        "intent": "warning"
-    })
-});
-
-CMS.Models.ErrorMessage = CMS.Models.SystemFeedback.extend({
-    defaults: $.extend({}, CMS.Models.SystemFeedback.prototype.defaults, {
-        "intent": "error"
-    })
-});
-
-CMS.Models.ConfirmAssetDeleteMessage = CMS.Models.SystemFeedback.extend({
-    defaults: $.extend({}, CMS.Models.SystemFeedback.prototype.defaults, {
-        "intent": "warning"
-    })
-});
-
-CMS.Models.ConfirmationMessage = CMS.Models.SystemFeedback.extend({
-    defaults: $.extend({}, CMS.Models.SystemFeedback.prototype.defaults, {
-        "intent": "confirmation"
-    })
-});
--- a/cms/static/js/models/section.js
+++ b/cms/static/js/models/section.js
@@ -22,22 +22,16 @@ CMS.Models.Section = Backbone.Model.extend({
    },
    showNotification: function() {
        if(!this.msg) {
-            this.msg = new CMS.Models.SystemFeedback({
-                intent: "saving",
-                title: gettext("Saving&hellip;")
-            });
-        }
-        if(!this.msgView) {
-            this.msgView = new CMS.Views.Notification({
-                model: this.msg,
+            this.msg = new CMS.Views.Notification.Saving({
+                title: gettext("Saving&hellip;"),
                closeIcon: false,
                minShown: 1250
            });
        }
-        this.msgView.show();
+        this.msg.show();
    },
    hideNotification: function() {
-        if(!this.msgView) { return; }
-        this.msgView.hide();
+        if(!this.msg) { return; }
+        this.msg.hide();
    }
 });
--- a/cms/static/js/views/feedback.js
+++ b/cms/static/js/views/feedback.js
@@ -1,39 +1,64 @@
-CMS.Views.Alert = Backbone.View.extend({
+CMS.Views.SystemFeedback = Backbone.View.extend({
    options: {
-        type: "alert",
+        title: "",
+        message: "",
+        intent: null,  // "warning", "confirmation", "error", "announcement", "step-required", etc
+        type: null, // "alert", "notification", or "prompt": set by subclass
        shown: true,  // is this view currently being shown?
        icon: true,  // should we render an icon related to the message intent?
        closeIcon: true,  // should we render a close button in the top right corner?
        minShown: 0,  // length of time after this view has been shown before it can be hidden (milliseconds)
        maxShown: Infinity  // length of time after this view has been shown before it will be automatically hidden (milliseconds)
+
+        /* could also have an "actions" hash: here is an example demonstrating
+           the expected structure
+        actions: {
+            primary: {
+                "text": "Save",
+                "class": "action-save",
+                "click": function(view) {
+                    // do something when Save is clicked
+                }
+            },
+            secondary: [
+                {
+                    "text": "Cancel",
+                    "class": "action-cancel",
+                    "click": function(view) {}
+                }, {
+                    "text": "Discard Changes",
+                    "class": "action-discard",
+                    "click": function(view) {}
+                }
+            ]
+        }
+        */
    },
    initialize: function() {
+        if(!this.options.type) {
+            throw "SystemFeedback: type required (given " +
+                JSON.stringify(this.options) + ")";
+        }
+        if(!this.options.intent) {
+            throw "SystemFeedback: intent required (given " +
+                JSON.stringify(this.options) + ")";
+        }
        var tpl = $("#system-feedback-tpl").text();
        if(!tpl) {
            console.error("Couldn't load system-feedback template");
        }
        this.template = _.template(tpl);
        this.setElement($("#page-"+this.options.type));
-        this.listenTo(this.model, 'change', this.render);
-        return this.show();
-    },
-    render: function() {
-        var attrs = $.extend({}, this.options, this.model.attributes);
-        this.$el.html(this.template(attrs));
        return this;
    },
-    events: {
-        "click .action-close": "hide",
-        "click .action-primary": "primaryClick",
-        "click .action-secondary": "secondaryClick"
-    },
+    // public API: show() and hide()
    show: function() {
        clearTimeout(this.hideTimeout);
        this.options.shown = true;
        this.shownAt = new Date();
        this.render();
        if($.isNumeric(this.options.maxShown)) {
-            this.hideTimeout = setTimeout($.proxy(this.hide, this),
+            this.hideTimeout = setTimeout(_.bind(this.hide, this),
                this.options.maxShown);
        }
        return this;
@@ -43,7 +68,7 @@ CMS.Views.Alert = Backbone.View.extend({
           this.options.minShown > new Date() - this.shownAt)
        {
            clearTimeout(this.hideTimeout);
-            this.hideTimeout = setTimeout($.proxy(this.hide, this),
+            this.hideTimeout = setTimeout(_.bind(this.hide, this),
                this.options.minShown - (new Date() - this.shownAt));
        } else {
            this.options.shown = false;
@@ -52,40 +77,63 @@ CMS.Views.Alert = Backbone.View.extend({
        }
        return this;
    },
-    primaryClick: function() {
-        var actions = this.model.get("actions");
+    // the rest of the API should be considered semi-private
+    events: {
+        "click .action-close": "hide",
+        "click .action-primary": "primaryClick",
+        "click .action-secondary": "secondaryClick"
+    },
+    render: function() {
+        // there can be only one active view of a given type at a time: only
+        // one alert, only one notification, only one prompt. Therefore, we'll
+        // use a singleton approach.
+        var parent = CMS.Views[_.str.capitalize(this.options.type)];
+        if(parent && parent.active && parent.active !== this) {
+            parent.active.stopListening();
+        }
+        this.$el.html(this.template(this.options));
+        parent.active = this;
+        return this;
+    },
+    primaryClick: function(event) {
+        var actions = this.options.actions;
        if(!actions) { return; }
        var primary = actions.primary;
        if(!primary) { return; }
        if(primary.click) {
-            primary.click.call(this.model, this);
+            primary.click.call(event.target, this, event);
        }
    },
-    secondaryClick: function(e) {
-        var actions = this.model.get("actions");
+    secondaryClick: function(event) {
+        var actions = this.options.actions;
        if(!actions) { return; }
        var secondaryList = actions.secondary;
        if(!secondaryList) { return; }
        // which secondary action was clicked?
        var i = 0;  // default to the first secondary action (easier for testing)
-        if(e && e.target) {
-            i = _.indexOf(this.$(".action-secondary"), e.target);
+        if(event && event.target) {
+            i = _.indexOf(this.$(".action-secondary"), event.target);
        }
-        var secondary = this.model.get("actions").secondary[i];
+        var secondary = secondaryList[i];
        if(secondary.click) {
-            secondary.click.call(this.model, this);
+            secondary.click.call(event.target, this, event);
        }
    }
 });

-CMS.Views.Notification = CMS.Views.Alert.extend({
-    options: $.extend({}, CMS.Views.Alert.prototype.options, {
+CMS.Views.Alert = CMS.Views.SystemFeedback.extend({
+    options: $.extend({}, CMS.Views.SystemFeedback.prototype.options, {
+        type: "alert"
+    })
+});
+CMS.Views.Notification = CMS.Views.SystemFeedback.extend({
+    options: $.extend({}, CMS.Views.SystemFeedback.prototype.options, {
        type: "notification",
        closeIcon: false
    })
 });
-CMS.Views.Prompt = CMS.Views.Alert.extend({
-    options: $.extend({}, CMS.Views.Alert.prototype.options, {
+CMS.Views.Prompt = CMS.Views.SystemFeedback.extend({
+    options: $.extend({}, CMS.Views.SystemFeedback.prototype.options, {
        type: "prompt",
        closeIcon: false,
        icon: false
@@ -98,6 +146,27 @@ CMS.Views.Prompt = CMS.Views.Alert.extend({
            $body.removeClass('prompt-is-shown');
        }
        // super() in Javascript has awkward syntax :(
-        return CMS.Views.Alert.prototype.render.apply(this, arguments);
+        return CMS.Views.SystemFeedback.prototype.render.apply(this, arguments);
    }
 });
+
+// create CMS.Views.Alert.Warning, CMS.Views.Notification.Confirmation,
+// CMS.Views.Prompt.StepRequired, etc
+var capitalCamel, types, intents;
+capitalCamel = _.compose(_.str.capitalize, _.str.camelize);
+types = ["alert", "notification", "prompt"];
+intents = ["warning", "error", "confirmation", "announcement", "step-required", "help", "saving"];
+_.each(types, function(type) {
+    _.each(intents, function(intent) {
+        // "class" is a reserved word in Javascript, so use "klass" instead
+        var klass, subklass;
+        klass = CMS.Views[capitalCamel(type)];
+        subklass = klass.extend({
+            options: $.extend({}, klass.prototype.options, {
+                type: type,
+                intent: intent
+            })
+        });
+        klass[capitalCamel(intent)] = subklass;
+    });
+});
--- a/cms/static/js/views/section.js
+++ b/cms/static/js/views/section.js
@@ -67,7 +67,7 @@ CMS.Views.SectionEdit = Backbone.View.extend({
    showInvalidMessage: function(model, error, options) {
        model.set("name", model.previous("name"));
        var that = this;
-        var msg = new CMS.Models.ErrorMessage({
+        var prompt = new CMS.Views.Prompt.Error({
            title: gettext("Your change could not be saved"),
            message: error,
            actions: {
@@ -80,6 +80,6 @@ CMS.Views.SectionEdit = Backbone.View.extend({
                }
            }
        });
-        new CMS.Views.Prompt({model: msg});
+        prompt.show();
    }
 });
--- a/cms/templates/base.html
+++ b/cms/templates/base.html
@@ -54,7 +54,6 @@
    <script type="text/javascript" src="${static.url('js/vendor/CodeMirror/css.js')}"></script>
    <script type="text/javascript" src="//www.youtube.com/player_api"></script>

-    <script src="${static.url('js/models/feedback.js')}"></script>
    <script src="${static.url('js/views/feedback.js')}"></script>

    <!-- view -->