HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

A class for animating messages as feedback to the user

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
theuseranimatingmessagesforfeedbackclass

Problem

This class provides animation messages to the user. Looking for general feedback.

I've posted the library I use as well if you have time to look that over.

```
/*****
SMessenger - messaging system to provide feedback to the user
*/
var SMessenger = $A.Class.create({
Name: 'SMessenger',

// static constants
A: {
DELAYTRANS: 1000,
DELAY: 2000,
GRANULARITY: 15,
GRANULARITY_POSITION: 1,
OFFSET: 40,
RADIX: 10,
UNITS: 'px'
},

// holds dome elements
E: {

// the dom holds the messages
dynamic: '#dynamic'
},
init: function () {

// pull the elements dynamically at init time
$A.eachChild(this.E.dynamic, function (val) {
this.E[val.id] = $A.el('#' + val.id);
}, this);
},

// s_ denotes server response
populateMessage: function (element, type) {
var period = '';

// messages from the server will have a period
// they are denoted by a preceding s_
if (type.match("s_")) {
type = type.replace('s_', '');
period = '.';
}
element.innerHTML = this.E['dyn_' + type].innerHTML + period;
},

// creates a fading message for the user
fadingMessage: function (obj) {
this.populateMessage(obj.response_element[0], obj.state);
obj.response_element.fade('down', this.A.DELAY);
},

// creates a message that fades up and then fades down.
popMessage : function (obj) {
var self = this;
this.populateMessage(obj.response_element, obj.state);
$A(obj.pane_element).fade('u

Solution

It looks really good! The only bit I don't like is the nested ifs, but I'm really nit-picking.

if (hide) {
            if (obj.input_element) {
                obj.input_element.removeEventListener("keypress", obj.enter);
            }
            obj.cover_element.style.display = 'inline';
        } else {
            if (obj.input_element) {
                obj.input_element.addEventListener("keypress", obj.enter);
            }
            obj.cover_element.style.display = 'none';
        }


Maybe....

obj.cover_element.style.display = hide ? 'inline' : 'none';
if (!obj.input_element) return;

if (hide) {
   obj.input_element.removeEventListener("keypress", obj.enter);
}
else {
   obj.input_element.addEventListener("keypress", obj.enter);
}


Putting the return value at the start, instead of protecting both blocks from it being null, makes it slightly more understandable. But I still don't like that section much.

Code Snippets

if (hide) {
            if (obj.input_element) {
                obj.input_element.removeEventListener("keypress", obj.enter);
            }
            obj.cover_element.style.display = 'inline';
        } else {
            if (obj.input_element) {
                obj.input_element.addEventListener("keypress", obj.enter);
            }
            obj.cover_element.style.display = 'none';
        }
obj.cover_element.style.display = hide ? 'inline' : 'none';
if (!obj.input_element) return;

if (hide) {
   obj.input_element.removeEventListener("keypress", obj.enter);
}
else {
   obj.input_element.addEventListener("keypress", obj.enter);
}

Context

StackExchange Code Review Q#39912, answer score: 3

Revisions (0)

No revisions yet.