Skip to content

Prevents child node scope with no children to be counted in depth.#388

Merged
Voles merged 4 commits into
angular-ui-tree:masterfrom
Whyves:master
May 8, 2015
Merged

Prevents child node scope with no children to be counted in depth.#388
Voles merged 4 commits into
angular-ui-tree:masterfrom
Whyves:master

Conversation

@Whyves

@Whyves Whyves commented Feb 23, 2015

Copy link
Copy Markdown
Contributor

When specifying maxDepth and using a template in ng-include to generate the tree nodes, a template is created for empty children. This causes the logic in maxSubDepth to include the depth of non-existent children thus going over the max depth limit and preventing node moves even when they were valid. I have added a test for this.

@Voles

Voles commented May 2, 2015

Copy link
Copy Markdown
Member

@Whyves nice improvement! Can you bring your PR up-to-date with the master branch?

@Voles Voles added the reviewed label May 2, 2015
@Whyves

Whyves commented May 3, 2015

Copy link
Copy Markdown
Contributor Author

Tried to rebase but you changed the name attribute in package.json to "AngularJS UI Tree" and that prevents me from running "npm install" it complains about "AngularJS UI Tree" to be invalid. Putting "-" instead of spaces fix the problem. May I suggest that you rename "AngularJS UI Tree" to "angular-ui-tree" which seems more standard. I am under Ubuntu. Once this is fixed, I will rebase my changes.

@Voles

Voles commented May 3, 2015

Copy link
Copy Markdown
Member

Fixed, sorry for the inconvenience...

@Whyves

Whyves commented May 4, 2015

Copy link
Copy Markdown
Contributor Author

There you go, PR up-to-date. Sorry for the multiple commits. I had a few issues with jscs :-)

Voles pushed a commit that referenced this pull request May 8, 2015
Prevents child node scope with no children to be counted in depth.
@Voles
Voles merged commit 88aeb84 into angular-ui-tree:master May 8, 2015
@Voles

Voles commented May 8, 2015

Copy link
Copy Markdown
Member

Thanks for this, @Whyves!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants