Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Batch delete/create nodes in ProjectManager #6448

Merged
merged 11 commits into from Jan 10, 2014

Conversation

jasonsanjose
Copy link
Member

@iwehrman

  • delete/create tree node methods are now synchronous
  • batch delete/create nodes to avoid excessive calls to _redraw

if (added.length > 0) {
var addedJSON = [];

added.forEach(function (addedEntry) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.filter instead of .forEach?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@iwehrman
Copy link
Contributor

Your commits look good to me 👍 Either let me know changes you'd like to see w.r.t. the memoization stuff, or just push the merge button!

* @return {number} Comparator value
*/
var _projectTreeSortComparator = _.memoize(function (a, b) {
var a1 = _getComparableName(a),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you were profiling this where was most of the time spent? Could we optimize more?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's much more room for improvement by way of memoization. Most of the time was spent in FileUtils.compareFilenames.

Using the entry for the name and entry type could be slightly faster than asking for DOM properties. It's probably slight though because it only happens once per node because of the memoization.

@jasonsanjose
Copy link
Member Author

@iwehrman I made some minor changes to sorting that you might want to take a quick look at.

iwehrman added a commit that referenced this pull request Jan 10, 2014
Batch delete/create nodes in ProjectManager
@iwehrman iwehrman merged commit 4adccd9 into master Jan 10, 2014
@iwehrman iwehrman deleted the jasonsanjose/find-tree-node branch January 10, 2014 18:01
@iwehrman
Copy link
Contributor

Everybody hold on to your butts!

@njx
Copy link
Member

njx commented Jan 10, 2014

CODE OF CONDUCT

@jasonsanjose
Copy link
Member Author

I struggled to find the appropriate emoji for that

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants