trackiffer icon indicating copy to clipboard operation
trackiffer copied to clipboard

{{data-attribute}} Not Matching

Open tpitale opened this issue 13 years ago • 10 comments

Because of the change from the previous issue fixing delegation, the $elem in getReplacement() is not what I would expect.

Doing this doesn't work:

trackiffer({
  'body.controller.index' : {
    delegate : '#some_form',
    rule : ['_trackEvent', 'form', 'submit', '{{data-thing-id}}']
  }
});

Doing this does:

trackiffer({
  'body.controller.index' : {
    delegate : '#some_form',
    rule : ['_trackEvent', 'form', 'submit', function($elem){return $elem.find("#some_form").attr('data-thing-id')}]
  }
});

$elem appears to be assigned to the matched element for 'body.controller.index', rather than the delegated match.

tpitale avatar Jan 12 '12 14:01 tpitale

As another example, this in the README:

trackiffer({
  'body' : {
      delay : false,
      delegate : 'a',
      rule : ['_trackEvent', 'Link', 'Social', '{{text}}', 1]
   }
});

Would return all the text in the body.

tpitale avatar Jan 12 '12 14:01 tpitale

This line:

var formatted_event_data = _t.formatData(event_data.rule.slice(0), $elem);

Could be changed to something like this:

var formatted_event_data = _t.formatData(event_data.rule.slice(0), jQuery(this));

tpitale avatar Jan 12 '12 14:01 tpitale

However, there may be greater repercussions in that handler function.

tpitale avatar Jan 12 '12 14:01 tpitale

Curses. Will look at this today.

averyvery avatar Jan 12 '12 15:01 averyvery

I tested my suggested change, it works for me. Do you want a pull request?

tpitale avatar Jan 12 '12 18:01 tpitale

Feel free to keep using your fixed version, but you were right, this goes a little deeper. Will push a broader fix this evening.

averyvery avatar Jan 12 '12 18:01 averyvery

Okay, should be fixed.

There's another bug, at the moment, where you can't do the following:

'.config' : {
    delegate : 'a.delegated',
    rule : [...]
},
'.config' : {
    delegate : 'form',
    type : 'submit',
    rule : [...]
}

...because, of course, you're overwriting the previous .config. Will start my own issue on this, but it will require some other, less pleasant changes.

averyvery avatar Jan 12 '12 18:01 averyvery

Scratch my previous comment, that can be resolved by just calling Trackiffer a second time with the other '.config'.

averyvery avatar Jan 12 '12 18:01 averyvery

It would be cool if it could all be in one block, but if that is a nasty fix, no worries.

tpitale avatar Jan 12 '12 18:01 tpitale

Well, it would be a big syntax change if we wanted to continue using standard JS methods - it might look like...

trackiffer([
  {
      selector : 'a.config'
      rule : [...]
  },
  {
      selector : 'a.config'
      rule : [...]
  }
]);

...or maybe...

trackiffer([
  {
      'a.config' : [...]
  },
  {
      'a.config' : {
        delegate : 'span',
        rule : [...]
      }
  }
]);

...or even...

trackiffer(
  {'a.config' : [...]},
  {'a.config' : {
    delegate : 'span'
    rule : [...]
  }
)

Not thrilled with an of these. An alternative would be a little workaround, which is cute but introduces a new idea (Trackiffer-specific syntax) into a known concept (jQuery selectors):

  {
      'a.config%1' : [...],
      'a.config%2' : {
        rule : [...]
      }
  }

averyvery avatar Jan 12 '12 19:01 averyvery