trentrichardson / Intimidatetime

Intimidatetime is a wicked awesome date time picker for both jQuery and Zepto.
MIT License
17 stars 8 forks source link

unable to pass custom options to timezone #3

Open scien opened 10 years ago

scien commented 10 years ago
    @$el.find("input[data-type='datetime']").intimidatetime {
      format: 'M/d/yyyy hh:mm:ss z'
      units:
        timezone:
          format: 'zzz'
          options: [-240,-300,-360,-420]
          names: TIMEZONE_ABBR_MAP
    }

This doesn't work. Instead you get all 40 default options. The issue is with the extend function.

o1[p] = (o1[p] === undefined || typeof o2[p] !== 'object' || $.isArray(o2[p])) ? o2[p] : mrec(o1[p], o2[p]);

This sometimes sets o1[p] = o2[p] when o2[p] is an object (it sets the reference). This causes data to be mixed from $.intimidatetime.i18n[''], $.intimidatetime.defaults, and my custom options. and the custom options end up getting overwritten by the Array[40] default.

The stack/flow looks like goes...

and then my original options have been overwritten.

As a quick fix, I updated my local extend function to the following...

    extend: function(){
      var o = arguments[0],
        i = 1,
        l = arguments.length,
        val = function(o) {
          if(!o) {
            return o;
          }
          else if(Array.isArray(o)) {
            return $.extend(true, [], o);
          }
          else if(typeof o == 'object') {
            return $.extend(true, {}, o);
          }
          return o;
        },
        mrec = function(o1, o2){
            for (var p in o2) {
              if( o2.hasOwnProperty(p)){
                o1[p] = (o1[p] === undefined || typeof o2[p] !== 'object' || $.isArray(o2[p])) ? val(o2[p]) : mrec(o1[p], o2[p]);
              }
            }
            return o1;
          };

      for(; i<l; i+=1){
        o = mrec(o, arguments[i]);
        //$.extend(true, o, arguments[i]);
      }

I'm going to pull out the Zepto code and just use $.extend and see if that works

scien commented 10 years ago

jquery deep extend can't be used directly.

$.extend(true, [1, 2, 3, 4, 5, 6], [7, 8, 9])
[7, 8, 9, 4, 5, 6]

I was unaware this was how jquery treated arrays in deep merges. very good to know. a shallow merge would replace the array as expected, but then my custom settings would replace the entire units section of the options which causes a ton of other issues. ex: o.units.hour.am[0] - can't read property am of undefined

http://bugs.jquery.com/ticket/9477

trentrichardson commented 10 years ago

Hey @scien .. yeah the issue with $.extend is why I created the built in extend to try and get around that. Seems to be a fine balance to merge objects but replace arrays completely. Were you able to resolve your issue? Thanks for digging in to the code.