0
\$\begingroup\$

As part of my effort to learn JavaScript, I have started developing a small library for creating DOM widgets like modals, tooltips and so on from scratch (nothing but plain vanilla JS allowed in other words); much like jQuery UI. So far, I have only developed one widget, namely a modal, but the general structure of the library is there: a collection classes and mixins for keeping the code dry.

As somebody who is very much in the process of learning, I would appreciate general feedback on the code, but more specifically I am interested getting your views on the following points:

  • Is the pattern of classes and mixins a good one for writing scaleable code?
  • What is the best way to deal with user defined parameters and default values?
  • Defensive code. When to throw errors, what to test for and so on. I want debugging the code to be easy for somebody that is not familliar with it.
  • Instinctively, I feel that the Effect mixin is a royal repetitive mess. Any suggestions on how I could improve it? And in regard to the slide-down method, what is the best way to get the height of a hidden DOM element in JavaScript?

A working version of the code below can be found here: https://jsfiddle.net/9sewteLb/1/

var Koalified = {};

if (typeof define === 'function' && define.amd) {

    define('koalified', Koalified);

} else if ('undefined' !== typeof exports && 'undefined' !== typeof module) {

    module.exports = Koalified;

}

Koalified.Effect = function() {

    this.show = function(el) {

    el.style.display = 'block';

};

this.hide = function(el) {

    el.style.display = 'none';

};

this.slideDown = function(el) {

    el.style.display = 'block';
    el.style.height = 'auto';

    function getHeight() {

        var height = el.clientHeight;

        el.style.display = 'none';

        return height;

    }

    var elHeight = getHeight();

    var height = 0;

    el.style.overflow = 'hidden';

    el.style.display = 'block';

    (function incrementHeight() {

        el.style.height = height + 'px';

        if ((height += 20) > elHeight) {

            el.style.height = elHeight;

            el.style.overflow = 'visible';

            return;

        } else {

            requestAnimationFrame(incrementHeight);

        }

    }());

};

this.slideUp = function(el) {

    var height = el.clientHeight;
    el.style.overflow = 'hidden';

    (function decrementHeight() {

        el.style.height = height + 'px';

        if ((height -= 20) < 0) {

            el.style.display = 'none';
            return;

        } else {

            requestAnimationFrame(decrementHeight);

        }

    }());

};

this.fadeIn = function(el) {

    el.style.opacity = 0;
    el.style.display = 'block';

    (function fade() {

        var opacity = parseFloat(el.style.opacity);

        if (!((opacity += 0.07) > 1)) {

            el.style.opacity = opacity;
            requestAnimationFrame(fade);

        }

    }());

};

this.fadeOut = function(el) {

    el.style.opacity = 1;

    (function fade() {

        if ((el.style.opacity -= 0.07) < 0) {

            el.style.display = "none";

        } else {

            requestAnimationFrame(fade);

        }

    }());

};

this.open = function(el, effect) {

    if (el === undefined) throw new Error('An element was not provided as an argument to the close method.');

    switch (effect) {

        case 'display':
            this.show(el);
            break;
        case 'fade':
            this.fadeIn(el);
            break;
        case 'slide':
            this.slideDown(el);
            break;

    }

};

this.close = function(el, effect) {

    if (el === undefined) throw new Error('An element was not provided as an argument to the close method.');

    switch (effect) {

        case 'display':
            this.hide(el);
            break;
        case 'fade':
            this.fadeOut(el);
            break;
        case 'slide':
            this.slideUp(el);
            break;

    }

};

return this;

};

Koalified.MicroComponents = function() {

this.createCloseButton = function(el, closeButtonClassName, closeButtonText) {

    var button = document.createElement('button');
    closeButtonClassName && button.setAttribute('class', closeButtonClassName);
    button.addEventListener('click', this.close.bind(this, this.element, this.animation), false);

    if (closeButtonText !== undefined) button.textContent = closeButtonText;

    return button;

};

return this;

};

Koalified.Modal = function(params) {

this.init(params);

};

Koalified.Effect.call(Koalified.Modal.prototype);

Koalified.MicroComponents.call(Koalified.Modal.prototype);

Koalified.Modal.prototype.constructor = Koalified.Modal;

Koalified.Modal.prototype.init = function(params) {

if (!params["element"]) throw new Error('A dom element was not passed to the Modal constructor.');

this.element = document.getElementById(params["element"]);

this.modalClassName = params["modalClassName"] !== undefined ? params["modalClassName"] : null;

this.trigger = params["trigger"] !== undefined ? document.getElementById(params["trigger"]) : false;

this.closeButton = params["closeButton"] !== undefined ? params["closeButton"] : true;

this.closeButtonClassName = params["closeButtonClassName"] !== undefined ? params["closeButtonClassName"] : null;

this.closeButtonText = params["closeButtonText"] !== undefined ? params["closeButtonText"] : null;

this.animation = params["animation"] !== undefined ? params["animation"] : 'display';

this.setup();

};

Koalified.Modal.prototype.setup = function() {

if (this.closeButton !== false) {

    var closeButton = this.createCloseButton(this.element, this.closeButtonClassName, this.closeButtonText);

    this.element.insertBefore(closeButton, this.element.childNodes[0]);

}

this.trigger !== false && this.trigger.addEventListener('click', this.open.bind(this, this.element, this.animation), false);

};

Koalified.Modal.prototype.openModal = function() {

    this.open(this.element, this.animation);

};

Koalified.Modal.prototype.closeModal = function() {

    this.close(this.element, this.animation);

}; 
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

First off, you need to work on indentation. Braces and indentation indicate which pieces of code go together. If you're indentation is off, then one cannot readily know where a piece of code belongs to. For instance:

Koalified.Effect = function() {

    this.show = function(el) {

    el.style.display = 'block';

};

this.hide = function(el) {

    el.style.display = 'none';

};

Right off the bat, I'd be thinking this is the global window object as it is in line with Koalified, your global namespace. However, it's just that your el.style.display = 'block'; isn't aligned to be inside show, which throws off the closing }.

Your code is also filled with a lot of styling-related code which isn't good in the long run. Suggesting you define the states of your widget (visible, hidden etc.) as CSS classes, and JS just do adding and removing of classes.

// visible modal
<div class="modal">

// hidden modal
<div class="modal modal--hidden">

.modal{ display: block }
.modal--hidden{ display: none }

I also see that you're doing animations. That can also be done in CSS using the transition property. For instance, fade animation can be done by transitioning opacity. Again, you can just add the classes that define the state of the widget in JS. Actual styles live in CSS.

.modal{
  transition: opacity 1s;
  opacity: 1;
}

.modal--hidden{
  opacity: 0;
}

Also note the -- in the class name. I'm using the BEM convention. The block (B) is my base styles for the modal. The modifier (M) modifies the state of the base. In this case .modal--hidden changes the default state of the modal from being shown to being hidden. This makes it easy to keep track of widgets and their states.

With that, your JS will be reduced to something like:

this.hide = function(el){
  el.classList.add('modal--hidden');
}

this.show = function(el)
  el.classList.remove('modal--hidden');
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ 'B' is block, not 'base' \$\endgroup\$ Commented Feb 29, 2016 at 20:23
  • \$\begingroup\$ @JosephtheDreamer Cheerio for the feedback! I've read through a few tutorials on CSS animations and have to say thank you for bringing them to my attention, not to mention the css naming convention, which I shall promply adopt. \$\endgroup\$ Commented Mar 1, 2016 at 7:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.