2
\$\begingroup\$

I'm trying to determine whether an element which has been clicked has a specific tagname. The trick is, that sometimes when you click an element you want any parent to be that specific tag.

My approach:

var ignore = 'a input textarea body'.split(' ');

var isClickable = function(elem) {
    var cancel = false;

    for (i in ignore) {
        if (elem.nodeName.toLowerCase() !== ignore[i]) {
            if (elem.parentNode.nodeName.toLowerCase() !== 'body')
                isClickable(elem.parentNode);
        }
        else {
            cancel = true;
            return cancel;
        }
    }

    return cancel;

};


document.onclick = function(evt) {
    isClickable(evt.target);
}

Is that code right or did I forget something?

SOLUTION: I came up to that tiny script based on Guffas solution (thanks!) I've optimized the code to a shorter and faster version:

var isClickable = function(elem) {
    for (i in ignore)
        if (elem.nodeName.toLowerCase() === ignore[i])
            return true;
    return (elem.parentNode.nodeName.toLowerCase() !== 'body') && isClickable(elem.parentNode);
};
\$\endgroup\$
  • \$\begingroup\$ You shouldn't use for ... in on arrays. It loops over all properties, including length. \$\endgroup\$ – RoToRa Jul 29 '13 at 13:29
2
\$\begingroup\$

I see two big problems with the code:

  • You are doing the recursive call inside the loop, so you will do each level four times. That will quickly escalade to a lot of calls. You should do the call after the loop, where you have determined that the node is not one in the array.

  • You are ignoring the result of the recursive call.

So:

function isClickable(elem) {
  for (i in ignore) {
    if (elem.nodeName.toLowerCase() === ignore[i]) {
      return true;
    }
  }
  if (elem.parentNode.nodeName.toLowerCase() !== 'body') {
    return isClickable(elem.parentNode);
  }
  return false;
};
\$\endgroup\$
  • \$\begingroup\$ for your first advice: how can i optimize that then? and for the second one: can i avoid this by doing return isClickable(elem.parentNode) ?? \$\endgroup\$ – David Fariña Jun 24 '13 at 14:54
  • \$\begingroup\$ @DavidFariña: That's not really optimising, but rather keeping it from doing the same thing an awful lot of times for no reason... Simply put the call after the loop. Yes, using return works for taking care of the result from the recursion. I added some code above. \$\endgroup\$ – Guffa Jun 24 '13 at 15:09
  • \$\begingroup\$ Thank you very much, weve optimized your code that its very short! \$\endgroup\$ – David Fariña Jun 24 '13 at 15:47
1
\$\begingroup\$

As mentioned for (var in obj) shouldn't be used for arrays. Instead, and since you've got the tags in a string, I'd use following method:

var tags = ["a", "input", "textarea", "body"],
    isClickable = function (elem) {
          return ((tags.join(" ") + " ").indexOf(elem.nodeName.toLowerCase() + " ") !== -1) ||
                 ((elem.parentNode.nodeName.toLowerCase() !== 'body') && isClickable(elem.parentNode));
    };

If support for IE8 and earlier isn't required (or with use of polyfills) following would also work:

// Alternatively use "a input textarea body".split(" ")
var tags = ["a", "input", "textarea", "body"],
    isClickable = function (elem) {
          return (tags.indexOf(elem.nodeName.toLowerCase()) !== -1) ||
                 ((elem.parentNode.nodeName.toLowerCase() !== 'body') && isClickable(elem.parentNode));
    };
\$\endgroup\$
  • \$\begingroup\$ This would incorrectly return true for b and i tags. \$\endgroup\$ – VLS Nov 26 '13 at 16:49
  • \$\begingroup\$ True. Hacked the solution a bit to work with partial tag-names, e.g. i and b. It's not elegant anymore though. \$\endgroup\$ – Mathatan Nov 27 '13 at 11:21

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.