Better performance?

[acacha]

Pull request for issue https://github.com/ludo/jquery-treetable/issues/144

[ludo]

Thanks! I would like to merge this but I notice one test fails with this change (see jquery-treetable/test.html). If you can figure out why then I can accept this pull request.

[acacha]

Thanks for yout time... I don't have to much experience with testing in Javascript. I run the test and I've more than one error as you can see in:

http://acacha.org/~sergi/jquery-treetable/test.html

Note: I've errors with proposed change and without proposed change so I'm not sure how to continue debugging...

[madflow]

@acacha @ludo I fixed the failing tests that where unrelated to this PR here https://github.com/ludo/jquery-treetable/pull/192.

As far as I can tell the existing test "does not show children if the node is hidden" fails - because

<tr data-tt-id="2" data-tt-parent-id="0" class="branch expanded" style="display: none;">

resolves to true when evaluating

if (!this.row[0].hidden) {
   this._showChildren();
}

The hidden attribute is something else and does not correspond to "display:none". Therefore this is always true.

http://www.w3schools.com/tags/att_global_hidden.asp

So I would argue, that there is a good reason for the test to fail. The plugin is still functioning, because this method seems to be a safety-net if the to be expanded node is actually not visible.

This can be resolved by changing the hide() method to:

    Node.prototype.hide = function() {
      this._hideChildren();
      this.row.hide().prop('hidden', true);
      return this;
    };

I can not comment on the issue if this is something we want here.

The jQuery docs sugegst using a dedicated class for this for the best performance (see "Additional Notes").

https://api.jquery.com/visible-selector/

[famaridon]

Hi,

I try with this code var style = window.getComputedStyle(this.row.get(0)); if(style.display !== 'none'){ this._showChildren(); }

and it's works fine and it's realy faster than JQuery.is