5
\$\begingroup\$

I'm making some changes to a JavaScript plugin on a site I've been made steward over. This main plugin has it's own sub-plugins and I'm trying to make certain aspects modular.

Currently, I'm attempting to make a reusable modal window that the sub-plugins ferry data to. This main plugin uses extensive use of prototype.

Right now, the main Plugin calls the sub-plugins "callback" method. The callback method then calls the Plugin.prototype.openModal and supplies it with a html template (html), callback function (what should be done after everything is done), validation (function that will validate the inputs of the modal, if present), and prerender (function that is fired before the modal is launched).

My code is pretty pseudo-cody, but this is what I have:

Prototype for reusable modal window, contained in main plugin:

Plugin.prototype.openModal = function(html, callback, validation, prerender) {

  var self = this;

  var headerFragment = html.hasOwnProperty('header') ? html.header : '',
      bodyFragment = html.hasOwnProperty('body') ? html.body : '',
      closeButtonText = html.hasOwnProperty('closeText') ? html.closeText : 'Cancel',
      confirmButtonText = html.hasOwnProperty('confirmText') ? html.confirmText : 'Save changes';

  // Only create the modal when it's called for the first time!
  if(!document.getElementById('confirm-modal')) this.createModal();

  var modal = $('#modal');
  modal.find('.modal-header h3').html(headerFragment);
  modal.find('.modal-body').html(bodyFragment);
  modal.find('.close-btn').html(closeButtonText);
  modal.find('.confirm-btn').html(confirmButtonText);
  modal.find('.modal-validation span').text('');

  if(typeof prerender === 'function') prerender();

  function closeModal(input) {
    if(callback && typeof callback === 'function') {
      input = input ? input : $('#modal').find('input').val();
      callback.call(self, input);
    }
    $('#modal').modal('hide');
  }

  function handleAlert(text) {
    var alert = modal.find('.modal-validation span');
    if(alert.text() != text) {
      alert.fadeOut(function(){
        alert.text(text).fadeIn();
      })
    }
  }

  $('#modal').modal('show');
  $('.confirm-btn').unbind('click').on('click', function(){

    var input;

    if(modal.find('input').length > 1) {
      var inputLength = modal.find('input').length
      input = [];
      modal.find('input').each(function(){
        if($(this).val()) input.push($(this).val());
      });

      if(input.length < inputLength) input = null;

    } else if(modal.find('input').length === 1) {
      input = modal.find('input').val();
    }
    if(input) {
      if(typeof validation === 'function') {
        var isValid = validation(input);
        if(typeof isValid === 'object' && isValid.type === 'valid') {
          isValid.value ? closeModal(isValid.value) : closeModal();
        } else {
          handleAlert(isValid);
        }
      } else {
        if(input) {
          typeof input === 'object' ? closeModal(input) : closeModal();
        } else {
          handleAlert('You must enter something!');
        }
      }
    } else {
      handleAlert('You must enter something!');
    }
  })
}

Function being called by sub plugin which is sending and receiving data to the prototype:

Plugin.Plugins['Link'] = (function () {
  var template = {
        header: 'Add a link, bro',
        body: [
          '<div>What the header said</div>',
          '<input type="text" value="http://" />'
        ].join(''),
        // closeText: 'NOPE',
        confirmText: 'Insert Link'
      },
      validationErrorMessage = 'You did not enter a valid URL';
  return {
    'callback': function (input, change) {
      var self = this;
      this.plugin.openModal(template, function(input){
        document.execCommand('createLink', false, input);
      }, function(input){
        return self.validate(input);
      });
    },
    validate: function(input) {
      var isValid = this.validateUrl(input);
      if(isValid) {
        return {type: 'valid'};
      }
      return validationErrorMessage;
    },
    'validateUrl': function (url) {
      url = url.replace(/(?:(?:^|\n)\s+|\s+(?:$|\n))/g,'').replace(/\s+/g,' ');
      if (url.charAt(0) == "/" ) {
          return true;
      } else {
          var regexp = /(ftp|http|https|gopher|telnet):\/\/(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@!\-\/]))?/;
          return regexp.test(url);
      }
    }
  }
}

This works perfectly fine across all versions of sub-plugins, but I can't help to think that there is a better way to ferry data. The whole:

  var self = this;
  this.parchment.openModal(template, function(input){
    document.execCommand('createLink', false, input);
  }, function(input){
    return self.validate(input);
  });

Bit seems like it could be a lot more elegant.

I'm namely struggling to keep contexts correct across the data being passed to Plugin.prototype. Namely validation and the data returned to the plugins callback.

Can anyone recommend a better structure for this? This is my first attempt to do JavaScript of this sort.

\$\endgroup\$
2
\$\begingroup\$

Interesting question,

your code is fairly easy to follow, JsHint cannot find anything serious to complain about.

Since you have a validate function, I would also create a callback function this way your call to openModal could be

this.parchment.openModal(template, this.createLinkCallback, this.validate );

Because you provide the functions themselves as parameters, you no longer need self, and you have a very readable line of code. ( Personally I might have called validate -> validationCallback ).

Furthermore, there is something smelly about the fact that you sprinkle your code with typeof something === 'function', I am not very fond of it, if you are going to check parameter types, you probably should do it up front in your code. Also this line : typeof input === 'object' ? closeModal(input) : closeModal(); so basically if there is an array of inputs you would not return anything, why ? After going thru the trouble of collection the values into an array etc.

Also, consider caching some of your jQuery calls, especially modal.find('input') is pretty expensive computing wise and you use this one a lot.

Finally, you are checking if(input) twice, so that handleAlert('You must enter something!'); under the first if(input) true block becomes dead code.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the advice. The code is pretty rough and some things needs to be cleaned up. I was mostly worried about passing the values around. I tried to do what you said calling this.createLinkCallback and this.validate but I had an issue getting the input to send between the prototype and back. I'll take a look at it in a little bit and revise my code above. \$\endgroup\$ – tr3online Apr 11 '14 at 19:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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