0
\$\begingroup\$

How would you write this jQuery code cleaner and better? I'm a beginner.

$.extend({
misTip : function($tipSettings) {
    $tip = $tipSettings.tip ? $tipSettings.tip : '';
    $closeTime = $tipSettings.closeTime ? $tipSettings.closeTime : 1500;
    $refresh = $tipSettings.refresh;

    delete $tipSettings.msg;
    delete $tipSettings.closeTime;
    delete $tipSettings.refresh;

    //dialog ui tip
    var tpl = '';
    tpl += '<div style="padding:5px 20px">';
    tpl += '<p style="font-size:14px;padding-top:10px;"><span class="ui-icon ui-icon-alert" style="float:left; margin:0 7px 20px 0;"></span><span class="ajaxMsg">' + $tip + '</span></p>';
    tpl += '</div>';

    var $defaultTipSettings = {
        title : 'notice',
        slow : 'slide',
        width : 320,
        open : function (event, ui) {
            $(this).bind("keypress",function(event){
                $(this).dialog('close');
            });
            $dialog = $(this);
            setTimeout(function(){
                $dialog.dialog('close');
                if ($refresh) {
                    _refresh();
                }
            }, $closeTime);
        }
    }
    var $tipSettings = $.extend($defaultTipSettings, $tipSettings);
    $(tpl).dialog($tipSettings);
}
});
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

You are using several global variables in the code, which should be local.

You are prefixing variable names with $ for no apparent reason, that only makes the code harder to read.

You can use the || operator instead of the conditional operator to check for missing values.

You are using the variable _refresh in the code, I assume that it should be the variable that you defined instead.

$.extend({
  misTip : function(tipSettings) {
    var tip = tipSettings.tip || '';
    var closeTime = tipSettings.closeTime || 1500;
    var refresh = tipSettings.refresh;

    delete tipSettings.msg;
    delete tipSettings.closeTime;
    delete tipSettings.refresh;

    //dialog ui tip
    var tpl =
      '<div style="padding:5px 20px">' +
      '<p style="font-size:14px;padding-top:10px;"><span class="ui-icon ui-icon-alert" style="float:left; margin:0 7px 20px 0;"></span><span class="ajaxMsg">' + tip + '</span></p>' +
      '</div>';

    var defaultTipSettings = {
      title : 'notice',
      slow : 'slide',
      width : 320,
      open : function (event, ui) {
        $(this).bind("keypress",function(){
          $(this).dialog('close');
        });
        var dialog = $(this);
        setTimeout(function(){
          dialog.dialog('close');
          if (refresh) {
            refresh();
          }
        }, closeTime);
      }
    }
    var settings = $.extend(defaultTipSettings, tipSettings);
    $(tpl).dialog(settings);
  }
});
\$\endgroup\$
0

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.