khan4019 / tree-grid-directive

Need one or more maintainer for this! comment or email me if you are interested
http://khan4019.github.io/tree-grid-directive/test/treeGrid.html
347 stars 183 forks source link

Several performance improvements (ng-if vs. ng-show, one time binding, ng-repeat with track by) #118

Open anx-ckreuzberger opened 7 years ago

anx-ckreuzberger commented 7 years ago

Hi!

I would like to propose several performance tweaks to this directive!

First (ng-show vs ng-if): I would suggest avoiding ng-if for showing/hiding icons. For instance here: https://github.com/khan4019/tree-grid-directive/blob/master/src/tree-grid-directive.js#L13

<a ng-if=\"col.sortable\" ng-click=\"sortBy(col)\">{{col.displayName || col.field}}</a><span ng-if=\"!col.sortable\">{{col.displayName || col.field}}</span>

Here ng-show/ng-hide would be so much better, because 1) it does not open a new scope (ng-if does) 2) it does not remove/add elements from the DOM (ng-if does)

I believe that the only reason where ng-if is valid in that template would be when you combine it with compile or any other heavy computation.

Second: Use Angular One Time Binding I also think that you should make heavy use of Angulars one time binding syntax, using :: where it makes sense. One time binding will avoid creating a watcher.

For instance, look at this code:

                    "       <td ng-repeat=\"col in colDefinitions\">\n" +
                    "         <div ng-if=\"col.cellTemplate\" compile=\"col.cellTemplate\" cell-template-scope=\"col.cellTemplateScope\"></div>\n" +
                    "         <div ng-if=\"!col.cellTemplate\">{{row.branch[col.field]}}</div>\n" +
                    "       </td>\n" +

This will create a watcher for colDefinitions, and in addition to col.cellTemplate for each column. In addition, this is within an ng-repeat of all rows, so you can multiply the amount of watchers...

As a counter example where you would NOT want one time binding to be active: Showing the sorting icon depending on whether the column is sorted or not:

<i ng-show=\"col.sorted\" class=\"{{col.sortingIcon}} pull-right\"></i>

Third: Use ng-repeat with track by ng-repeat makes heavy computation using a hash function. By using track-by with a unique identifier you can avoid this computation. for instance, ng-repeat on the column definitions could be tracked by their index, using ng-repeat=\"col in ::colDefinitions track by $index\"

I'm waiting a little before I send a Pull Request, in case other people find reasons on why this or that should not be handled the way I proposed it. PLEASE DISCUSS!

For simplicity so people can try it out, here is my modified template:

function ($templateCache) {
                $templateCache.put('template/treeGrid/treeGrid.html',
                    "<div class=\"table-responsive\">\n" +
                    " <table class=\"table tree-grid\">\n" +
                    "   <thead>\n" +
                    "     <tr>\n" +
                    "       <th><a ng-show=\"expandingProperty.sortable\" ng-click=\"sortBy(expandingProperty)\">{{expandingProperty.displayName || expandingProperty.field || expandingProperty}}</a><span ng-hide=\"expandingProperty.sortable\">{{expandingProperty.displayName || expandingProperty.field || expandingProperty}}</span><i ng-show=\"expandingProperty.sorted\" class=\"{{expandingProperty.sortingIcon}} pull-right\"></i></th>\n" +
                    "       <th ng-repeat=\"col in ::colDefinitions track by $index\"><a ng-show=\"::col.sortable\" ng-click=\"sortBy(col)\">{{col.displayName || col.field}}</a><span ng-hide=\"::col.sortable\">{{col.displayName || col.field}}</span><i ng-show=\"col.sorted\" class=\"{{col.sortingIcon}} pull-right\"></i></th>\n" +
                    "     </tr>\n" +
                    "   </thead>\n" +
                    "   <tbody>\n" +
                    "     <tr ng-repeat=\"row in tree_rows | searchFor:$parent.filterString:expandingProperty:colDefinitions track by row.branch.uid\"\n" +
                    "       ng-class=\"'level-' + {{ row.level }} + (row.branch.selected ? ' active':'')\" class=\"tree-grid-row\">\n" +
                    "       <td><a ng-click=\"user_clicks_branch(row.branch)\"><i ng-class=\"row.tree_icon\"\n" +
                    "              ng-click=\"row.branch.expanded = !row.branch.expanded\"\n" +
                    "              class=\"indented tree-icon\"></i></a><span ng-if=\"expandingProperty.cellTemplate\" class=\"indented tree-label\" " +
                    "              ng-click=\"on_user_click(row.branch)\" compile=\"expandingProperty.cellTemplate\"></span>" +
                    "              <span ng-if=\"!expandingProperty.cellTemplate\" class=\"indented tree-label\" ng-click=\"on_user_click(row.branch)\">\n" +
                    "             {{row.branch[expandingProperty.field] || row.branch[expandingProperty]}}</span>\n" +
                    "       </td>\n" +
                    "       <td ng-repeat=\"col in ::colDefinitions track by $index\">\n" +
                    "         <div ng-if=\"::col.cellTemplate\" compile=\"col.cellTemplate\" cell-template-scope=\"col.cellTemplateScope\"></div>\n" +
                    "         <div ng-if=\"!::col.cellTemplate\">{{row.branch[col.field]}}</div>\n" +
                    "       </td>\n" +
                    "     </tr>\n" +
                    "   </tbody>\n" +
                    " </table>\n" +
                    "</div>\n" +
                    "");
            }